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

RESOLVED DUPLICATE of bug 394322

Status

defect
RESOLVED DUPLICATE of bug 394322
10 months ago
10 months ago

People

(Reporter: tdgoodman, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

10 months ago
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.
(Reporter)

Comment 1

10 months ago
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.

Comment 2

10 months ago
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:

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

/9j/4QAYRXhpZgAASUkqAAgAAAAAAAAAAAAAAP/sABFEdWNreQABAAQAAABQAAD/7gAOQWRvYmUA
...
...
(Reporter)

Comment 3

10 months ago
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

Comment 4

10 months ago
also
https://support.mozilla.org/en-US/questions/1222256 (only happens starting 60.0b7)
https://support.mozilla.org/en-US/questions/1222690 (claims to have happened also in 52.8.0)

Comment 5

10 months ago
correction
https://support.mozilla.org/en-US/questions/1222690 (only happens starting 60.0b7)
https://support.mozilla.org/en-US/questions/1222256 (claims to have happened also in 52.8.0)

Updated

10 months ago
Attachment #8986821 - Attachment mime type: message/rfc822 → text/plain

Comment 6

10 months ago
Problem confirmed in TB 60 beta not neither TB TB 52.7 nor TB 52.8. BTW, https://support.mozilla.org/en-US/questions/1222256 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 ;-)
Status: UNCONFIRMED → NEW
Component: Untriaged → Message Compose Window
Ever confirmed: true

Comment 7

10 months ago
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"http://schemas.microsoft.com/office/2004/12/omml" xmlns=3D"http:=
//www.w3.org/TR/REC-html40">

Comment 8

10 months ago
Here's the same test but with that <html> tag reduced to <html>. Now the forward works.

Comment 9

10 months ago
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)

Comment 10

10 months ago
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.

Comment 11

10 months ago
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 =
    mozilla::dom::DOMParser::CreateWithoutGlobal(rv2);
  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(
    "@mozilla.org/layout/documentEncoder;1?type=text/html");
  uint32_t aFlags = nsIDocumentEncoder::OutputRaw |
                    nsIDocumentEncoder::OutputDisallowLineBreaking;
  rv = encoder->Init(document, NS_LITERAL_STRING("text/html"), aFlags);
  NS_ENSURE_SUCCESS(rv, -1);
  rv = encoder->EncodeToString(parsed);
  NS_ENSURE_SUCCESS(rv, -1);
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:
Original
Parsed:
<html><head><meta http-equiv="content-type" content="text/html; charset="></head><body></body></html>
Modified
<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)

Comment 12

10 months ago
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.

Comment 13

10 months ago
Let's dupe it all to bug 394322 which has a proposed fix.
Status: NEW → RESOLVED
Last Resolved: 10 months ago
Flags: needinfo?(ben.bucksch)
Flags: needinfo?(alice0775)
Resolution: --- → DUPLICATE
Duplicate of bug: 394322

Comment 15

10 months ago
Thanks Alice, as I already suspected, bug 1419417.
You need to log in before you can comment on or make changes to this bug.