Closed Bug 1470210 Opened 6 years ago Closed 6 years ago

Forward Inline fails to include original HTML message text when inline image is present


(Thunderbird :: Message Compose Window, defect)

Not set


(Not tracked)



(Reporter: tdgoodman, Unassigned)



(3 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20180605171542

Steps to reproduce:

Display message as Original HTML. Select Message > Forward As > Inline

Actual results:

Compose window appears with Forwarded Message header, but no message text. All attachments intact.

Expected results:

Compose window should also have included the message text.
Note that changing the view to View > Message Body As > Plain Text, then Message > Forward As > Inline *DOES* include the message text in the compose window for the forward message. Both Original HTML and Simple HTML message body views fail to include the message text in the compose window.

Windows 10 Home (64-bit) Version 1803
Thunderbird 60.0b7 (32-bit)

This error *DOES NOT* occur on Thunderbird 52.8.0 (32-bit) on Windows 7 Professional (64-bit) SP1.
I'm able to reproduce this bug with Thunderbird 60.0b7 and 60.0b8 on Windows 7.

It occurs when trying to compose a forwarded message (eg. by clicking the forward button) from a HTML messages containing inline images like this:

Content-Disposition: inline; filename="signatur.jpg"
Content-ID: <signatur.jpg>
Content-Transfer-Encoding: base64
Content-Type: image/jpg; name="signatur.jpg"

I've changed the title based on Dietrich's observation that this bug seems to be tied to forwarding inline images. 

Some other observations:
- Images that are merely attached do not interfere with generating the quoted text in the new compose message.
- Replying to the message works with no problem, only Forwarding the message has this bug.
- Not all messages with in-line images are affected. I have some messages with in-line images that forward without issue (the ones I have found so far were generated in Thunderbird 60 where there is only one image in the signature).
- Forwarding As Attachment works without issue.
Summary: Forward Inline fails to include original HTML message text → Forward Inline fails to include original HTML message text when inline image is present
also (only happens starting 60.0b7) (claims to have happened also in 52.8.0)
correction (only happens starting 60.0b7) (claims to have happened also in 52.8.0)
Attachment #8986821 - Attachment mime type: message/rfc822 → text/plain
Problem confirmed in TB 60 beta not neither TB TB 52.7 nor TB 52.8. BTW, is a little confused writing about a "58 version".

Most likely a fallout from our recent security fixes, the best security is of course not to show anything ;-)
Component: Untriaged → Message Compose Window
Ever confirmed: true
Here's a greatly reduced test case exposing the error.

Note the HTML tag:
<html xmlns:v=3D"urn:schemas-microsoft-com:vml" xmlns:o=3D"urn:schemas-micr=
osoft-com:office:office" xmlns:w=3D"urn:schemas-microsoft-com:office:word" =
xmlns:m=3D"" xmlns=3D"http:=
Here's the same test but with that <html> tag reduced to <html>. Now the forward works.
This maybe Core::Editor regression.

When running this in a local debug build, I see:
[1828, Main Thread] WARNING: HTMLEditRules::BeforeEdit() failed to handle something: 'NS_SUCCEEDED(rv)', file c:/mozilla-source/comm-central/editor/libeditor/HTMLEditor.cpp, line 3518

Alice, can you please find the regression for us:
STR: import "reduced-testcase.eml - showing error" (or "Addition to Shop Drawing.eml"), forward inline.
Flags: needinfo?(alice0775)
Oh, here's a clue. When viewing the HTML as "simple" HTML, both TB 52 and TB 60 fail to forward correctly. So there has always been something wrong with forwarding sanitised HTML. Now something similar to sanitising has moved onto the main "original" HTML code path, and it started failing there as well.

Alice, most likely caused by bug 1419417, landed in bug 1419417 comment #193 on 2018-05-21 19:03 CEST and uplifted to TB 60 beta 7.
Ben, care to fix the regression you caused in bug 1419417 and which you urged us to ship in TB 52 to 25 million users :-(

I've debugged it a bit in mimeTextHTMLParsed.cpp:
  // Parse the HTML source.
printf("Original\n%s", NS_ConvertUTF16toUTF8(rawHTML).get());
  mozilla::ErrorResult rv2;
  RefPtr<mozilla::dom::DOMParser> parser =
  nsCOMPtr<nsIDocument> document = parser->ParseFromString(
    rawHTML, mozilla::dom::SupportedType::Text_html, rv2);
  if (rv2.Failed())
    return -1;

  // Serialize it back to HTML source again.
  nsCOMPtr<nsIDocumentEncoder> encoder = do_CreateInstance(
  uint32_t aFlags = nsIDocumentEncoder::OutputRaw |
  rv = encoder->Init(document, NS_LITERAL_STRING("text/html"), aFlags);
  rv = encoder->EncodeToString(parsed);
printf("Parsed:\n%s", NS_ConvertUTF16toUTF8(parsed).get());

  // Write it out.
  NS_ConvertUTF16toUTF8 resultCStr(parsed);
  MimeInlineTextHTML_insert_lang_div(obj, resultCStr);
  MimeInlineTextHTML_remove_plaintext_tag(obj, resultCStr);
printf("Modified\n%s", resultCStr.get());

When viewing the message, I get *two* sets of Original/Parsed/Modified, the first one with the proper HTML the second one empty:
<html><head><meta http-equiv="content-type" content="text/html; charset="></head><body></body></html>
<html><head><meta http-equiv="content-type" content="text/html; charset="></head><body></body></html>

Looks like for rendering the first one is used, for inline forward that second empty one. As per comment #10, that has always been broken for "simple" HTML and now that bug moved to the main code path :-(

But strangely enough, what I wrote in the paragraph above can't be correct since the behaviour changes depending on the content of the HTML tag, see comment #8.

Alice, I'm leaving the NI, maybe you can just test around the date given in comment #10.

BTW, this will be hard to fix since it's jolly-good MIME.
Flags: needinfo?(ben.bucksch)
Actually, for simple HTML this bug is already known, but we never cared to fix it: Bug 1154105, bug 313401 and bug 394322. In fact, bug 313401 is quite specific, stating "... if <html> tag has attribute". We've just confirmed that.
Let's dupe it all to bug 394322 which has a proposed fix.
Closed: 6 years ago
Flags: needinfo?(ben.bucksch)
Flags: needinfo?(alice0775)
Resolution: --- → DUPLICATE
Thanks Alice, as I already suspected, bug 1419417.
You need to log in before you can comment on or make changes to this bug.