[FIX] Outlook: fix crash when adding inline images #49
Conversation
Currently logging emails can crash if there are more inline image attachments than img tags. Steps to reproduce ----- 1. Reply to an email that has an inline image 2. From the reply, select "Log Email Into Contact" 3. No log is created and the button is stuck in the loading state Cause ----- Currently, we loop through `inlineAttachments` to match them one-to-one with the document's `img` tags. But the counter does not consider the case where there's a different number of `inlineAttachments` and `imageElements`. This usually happens when there's an img in the email history which is removed at the top of the function, but the attachment still exists. Some emails can also have embeded inline attachments that are unused. Fix ----- Use two counters so that we don't hit an index out of bounds error. Also filter by `img` elements using `cid:` in src, so we don't overwrite `img` elements using URL images. opw-4855422
| @@ -120,30 +120,35 @@ class Logger extends React.Component<LoggerProps, LoggerState> { | |||
| } | |||
| } | |||
| }); | |||
| // a counter is needed to map img tags with attachments, as outlook does not provide | |||
| // attempt to map img tags with attachments by index, as outlook does not provide | |||
There was a problem hiding this comment.
previous code seems fishy, can't it be rework ?
(yours with 2 indexes look even more fishy)
There was a problem hiding this comment.
I have a task to rework the addon, but not the time atm (in the oxp rush)
I can have a look when I rework them if you don't find
There was a problem hiding this comment.
I'm mapping by indices since the Office API does not expose any way to map via contentId. This seems to be a known limitation: OfficeDev/office-js#4987 which is probably the same issue the original code was working around. Two indices are needed since the arrays could be of different lengths. There is still a rare case if the attachments are in a different order than the body, then they will be inserted in the wrong order.
The most robust way to do this would be to fetch the entire .eml file and process the attachments through that since the contentId will be in the .eml data (the linked issue describes this). However I think this method could be saved for the addon rework.
Currently logging emails can crash if there are more inline image attachments than img tags.
Steps to reproduce
Cause
Currently, we loop through
inlineAttachmentsto match them one-to-one with the document'simgtags. But the counter does not consider the case where there's a different number ofinlineAttachmentsandimageElements. This usually happens when there's an img in the email history which is removed at the top of the function, but the attachment still exists. Some emails can also have embeded inline attachments that are unused.Fix
Use two counters so that we don't hit an index out of bounds error. Also filter by
imgelements usingcid:in src, so we don't overwriteimgelements using URL images.opw-4855422