Skip to content

[FIX] Outlook: fix crash when adding inline images #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[FIX] Outlook: fix crash when adding inline images #49

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions outlook/src/taskpane/components/Log/Logger.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

previous code seems fishy, can't it be rework ?
(yours with 2 indexes look even more fishy)

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

// an id that enables us to match an img with an attachment.
let j = 0;
const imageElements = doc.getElementsByTagName('img');
// iterate in reverse order since some attachments may be from the deleted history
const imageElements = Array.from(doc.getElementsByTagName('img')).filter((img) =>
img.getAttribute('src')?.startsWith('cid:'),
);

inlineAttachments.forEach((inlineAttachment) => {
if (inlineAttachment != null && inlineAttachment.error == undefined) {
if (inlineAttachment.oversize) {
let j = imageElements.length - 1;
let k = inlineAttachments.length - 1;
while (j >= 0 && k >= 0) {
if (inlineAttachments[k] != null && inlineAttachments[k].error == undefined) {
if (inlineAttachments[k].oversize) {
imageElements[j].setAttribute(
'alt',
_t('Could not display image %(attachmentName)s, size is over limit', {
attachmentName: inlineAttachment.name,
attachmentName: inlineAttachments[k].name,
}),
);
} else {
const fileExtension = inlineAttachment.name.split('.')[1];
const fileExtension = inlineAttachments[k].name.split('.')[1];
imageElements[j].setAttribute(
'src',
`data:image/${fileExtension};base64, ${inlineAttachment.content}`,
`data:image/${fileExtension};base64, ${inlineAttachments[k].content}`,
);
}
j++;
}
});
j--;
k--;
}

if (oversizeAttachments.length > 0) {
const attachmentNames = oversizeAttachments.map((attachment) => `"${attachment.name}"`).join(', ');
Expand Down