Skip to content

Report issues with parsing inline images#3350

Merged
tomholub merged 4 commits intomasterfrom
issue-3301-report-issues-with-parsing-inline-images
Jan 29, 2021
Merged

Report issues with parsing inline images#3350
tomholub merged 4 commits intomasterfrom
issue-3301-report-issues-with-parsing-inline-images

Conversation

@limonte
Copy link

@limonte limonte commented Jan 27, 2021

This PR reports issues with parsing inline images to track issues with unexpected src values of inline images (#3301 (comment))

close #3301

@limonte limonte requested a review from seisvelas January 27, 2021 14:34
@limonte
Copy link
Author

limonte commented Jan 27, 2021

The better review experience would be with ?w=1: https://github.com/FlowCrypt/flowcrypt-browser/pull/3350/files?w=1

img.setAttribute('src', `cid:${imgAttachment.cid}`);
imgAttachments.push(imgAttachment);
} else {
Catch.report(`Unable to parse an inline image with src="${src}"`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reports it to us - not good - a privacy violation. You can instead do a toast to tell the user.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, will do that 👍

Copy link
Author

@limonte limonte Jan 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of toast I decided to go with the error modal which was already implemented there.

CleanShot 2021-01-28 at 15 14 10@2x

CleanShot 2021-01-28 at 15 14 22@2x

The current behavior is: we will NOT send a message if there were issues with parsing inline images, IMO it's better to prevent sending a message which is broken because of unexpected issues. @tomholub Let me know if you want some other behavior here, e.g. remove broken images and send a message without them.

@limonte limonte changed the title Report issues with parsing imline images Report issues with parsing inline images Jan 28, 2021
@limonte limonte force-pushed the issue-3301-report-issues-with-parsing-inline-images branch from 327b8c6 to 802f9a0 Compare January 29, 2021 17:32
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you both 👍

@tomholub tomholub merged commit 4014d3f into master Jan 29, 2021
@tomholub tomholub deleted the issue-3301-report-issues-with-parsing-inline-images branch January 29, 2021 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

check security of cid replacements PR

3 participants