Last Comment Bug 594646 - If order of http-equiv and content in <meta> is reversed(<meta content="...;charset=..." http-equiv="Content-Type">), charset in <meta> is applied to mail data coverted to utf-8. Thus, non-ascii characters are shown in garbled
: If order of http-equiv and content in <meta> is reversed(<meta content="...;c...
Status: RESOLVED FIXED
[needs a mozmill test] [View/Message ...
: html5
Product: MailNews Core
Classification: Components
Component: MIME (show other bugs)
: Trunk
: All All
: -- major with 5 votes (vote)
: Thunderbird 24.0
Assigned To: Jonathan Protzenko [:protz]
:
Mentors:
: 571704 593894 607071 615183 820335 822641 843600 843990 845421 845833 845963 851488 852984 857858 857974 857992 861129 870301 884973 892914 (view as bug list)
Depends on: 572886
Blocks: 889656 571704
  Show dependency treegraph
 
Reported: 2010-09-08 19:30 PDT by WADA
Modified: 2013-07-13 02:23 PDT (History)
38 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
-


Attachments
Test mails in Shift_JIS (2.15 KB, text/plain)
2010-09-08 19:30 PDT, WADA
no flags Details
Tentative patch (505 bytes, patch)
2010-11-29 13:16 PST, Jonathan Protzenko [:protz]
bugmail: review+
Details | Diff | Review
testcase, contains two mails, one with display problems and one correct (2.95 KB, application/octet-stream)
2013-03-22 08:32 PDT, j.heinz
no flags Details
tests v1 (5.56 KB, patch)
2013-04-04 21:06 PDT, Szabolcs Hubai (:xabolcs)
mconley: feedback+
Details | Diff | Review
tests v2 (7.34 KB, patch)
2013-04-14 13:58 PDT, Szabolcs Hubai (:xabolcs)
mconley: feedback+
Details | Diff | Review
8bit and qp encoded messages (3.47 KB, application/zip)
2013-04-16 00:35 PDT, Szabolcs Hubai (:xabolcs)
no flags Details
tests v3 (10.60 KB, patch)
2013-04-17 17:28 PDT, Szabolcs Hubai (:xabolcs)
no flags Details | Diff | Review
tests v4 (13.46 KB, patch)
2013-05-23 03:34 PDT, Szabolcs Hubai (:xabolcs)
mconley: review+
Details | Diff | Review

Description WADA 2010-09-08 19:30:32 PDT
Created attachment 473359 [details]
Test mails in Shift_JIS

[Build ID]
> Mozilla/5.0 (Windows NT 5.1; rv:2.0b6pre) Gecko/20100906 Thunderbird/3.2a1pre

With html5.enable=true, <meta http-equiv="Content-Type" content="..."> is not properly processed and non-ascii characters are not shown correctly, if order of attributes in <meta> is reversed.

It was found during addtional testing for bug 572886 with some variants of test cases such as different charset, non-ascii data case. In testing, similar phenomenon to Bug 505072, Bug 508946, Bug 593894 was observed with some test cases, but such problem could not be observed with some other cases even though mail data is very similar(nearly same as mail produces problem). So I checked difference of test mails.   
Attached mail folder file contains three mails written in Shift_JIS(Content-Type: text/html; charset="Shift_JIS").
  mail-01 :
   <meta http-equiv="Content-Type" content="text/html; charset=Shift_JIS">
  mail-02 :
   <meta content="text/html; charset=Shift_JIS" http-equiv="Content-Type">
  mail-03 :
   <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
Expected display of text is;
  ここは日本語です。 Here is Japanese.
Observed phenomenon.
  mail-01 : no problem
  mail-02 : garbage is shown for non-ascii characters.
            (probably phenomenon reported to Bug 593894)
  mail-03 : non-ascii character is shown as expected,
            if charset=utf-8 is specified in mail-02.

I don't know "internal reload by <meta> charset"(problem of bug 572886) happens or not in above cases.
Comment 1 WADA 2010-09-08 20:45:38 PDT
Note:
If http-equiv and content attributes are written in normal order in <meta>, charset in <meta> is properly ignored as expected, when charset is specified in Content-Type: header.
> <meta http-equiv="Content-Type" content="text/html; charset=xxxxx">
Comment 2 WADA 2010-09-15 01:55:05 PDT
CCing to Henri Sivonen.
No HTML5 parser side issue? MIME wrongly passes "charset in meta" to HTML5 parser if reversed order?
Comment 3 Henri Sivonen (:hsivonen) 2010-09-15 04:20:31 PDT
All the test emails have
Content-Type: text/html; charset="Shift_JIS"
on the MIME container. Thunderbird needs to pass Shift_JIS (without quotes) to the HTML parser with charset source set to kCharsetFromChannel.

If you can verify that Thunderbird does this and it *still* doesn't work, then there's a parser bug. However, considering that the problem hasn't been reported in the HTTP context, I bet Thunderbird isn't passing Shift_JIS with kCharsetFromChannel to the HTML parser.

Moreover, the MIME code shouldn't be looking at <meta> *at all*. That's the HTML parser's job--but only when the caller passes kCharsetFromUserDefault (which it should do when the charset parameter in the MIME container is absent.).
Comment 4 WADA 2010-09-15 07:08:22 PDT
Henri Sivonen, thanks for your comment.
CCing to Zane U. Ji.
Is there any fault of MIME or inconsistency between MIME and HTML5 parser?
Why can "order of http-equiv and content in <meta>" be relevant to problem?
Comment 5 WADA 2010-09-15 19:07:06 PDT
FYI.
If multiple <meta>'s is put in <head> section, following occurs.
(A) Normal order of http-equiv and content in all <meta>
Content-Type: text/html; charset="Shift_JIS"
1-st <meta>: http-equiv -> content, charset=charset-01
2-st <meta>: http-equiv -> content, charset=charset-02
3-rd <meta>: http-equiv -> content, charset=charset-03
=> charset-02 is always applied to data converted to utf-8
(B) Order of http-equiv and content is reversed in first <meta> only
Content-Type: text/html; charset="Shift_JIS"
1-st <meta>: content -> http-equiv, charset=charset-01
2-st <meta>: http-equiv -> content, charset=charset-02
3-rd <meta>: http-equiv -> content, charset=charset-03
=> charset-01 is always applied to data converted to utf-8.
   Same phenomenon as this bug.
I'll open separate bug if it's required, for ease of diagnosis.
Comment 6 WADA 2010-10-01 10:00:38 PDT
FYI.
Above phenomenon/problem is with View/Message Body As/Original HTML.
If "Simple HTML", garbled disply is shown even for next simple/quoted-printable HTML other than utf-8.
> Content-Type: text/html; charset=Shift_JIS
> Content-Transfer-Encoding: quoted-printable
> 
> <html><head></head>
> <body>
> <p>=93=FA=96{=8C=EA</p>
> </body></html>
It's similar phenomenon but different problem of "Simple HTML" with quoted-printable text/html or quoted-printable(text/plain or text/html) in multipart.
Comment 7 WADA 2010-10-01 21:00:11 PDT
FYI. Bug for "Simple HTML" case is currently bug 598740.
Comment 8 WADA 2010-10-06 00:08:15 PDT
As for Thunderbird release builds, simplest solution of this kind of bug is;
  Default of html5.enable=false, until this kind of bugs will be resolved.
Comment 9 WADA 2010-10-08 00:37:17 PDT
After fix of Bug 582788, phenomenon/problem of Bug 572886 was morphed to this bug. Setting dependency to Bug 572886 for ease of tracking and iagnosis.
Comment 10 WADA 2010-10-26 01:35:47 PDT
*** Bug 607071 has been marked as a duplicate of this bug. ***
Comment 11 WADA 2010-10-27 18:48:11 PDT
*** Bug 593894 has been marked as a duplicate of this bug. ***
Comment 12 WADA 2010-10-27 19:16:40 PDT
*** Bug 571704 has been marked as a duplicate of this bug. ***
Comment 13 Jonathan Protzenko [:protz] 2010-11-29 13:16:58 PST
Created attachment 493779 [details] [diff] [review]
Tentative patch

Wada-San, could you please tell me if this patch fixes the issue for you? It does for me in bug 615183.
Comment 14 Jonathan Protzenko [:protz] 2010-11-29 13:17:59 PST
*** Bug 615183 has been marked as a duplicate of this bug. ***
Comment 15 Jonathan Protzenko [:protz] 2010-11-30 00:20:55 PST
David you might want to have a look into this as well.
Comment 16 WADA 2010-11-30 00:59:35 PST
(In reply to comment #13)
> Tentative patch

It looks for me right thing, because of next in comment #3 by Henri Sivonen.
> I bet Thunderbird isn't passing Shift_JIS with kCharsetFromChannel to the HTML parser.
> Moreover, the MIME code shouldn't be looking at <meta> *at all*.
> That's the HTML parser's job

(Q1) Is it correctly done for next?
> That's the HTML parser's job
> --but only when the caller passes kCharsetFromUserDefault.
> (which it should do when the charset parameter in the MIME container is
absent.).

(Q2) Can you explain why no problem with (A) and problem occurs only with (B)? (A) <meta http-equiv="Content-Type" content="text/html; charset=...">
(B) <meta content="text/html; charset=..." http-equiv="Content-Type">
As current code is next, I guess Tb misses charset in meta tag when (B) then passes null or garbage.
> muDV->SetHintCharacterSetSource(kCharsetFromMetaTag);

Jonathan Protzenko, where can I obtain build in which your patch landed? I don't have environment to build. (before it, I do't know how to build :-) )
Comment 17 Jonathan Protzenko [:protz] 2010-11-30 01:49:18 PST
I've pushed this patch to a try server, which means I should be able to point you to a build in a couple hours.

My theory is that libmime actually takes care of all the encoding issues and already outputs UTF8, so the HTML parser actually must ignore the <meta> tag at all costs. What supports my theory is the following line:

http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMessenger.cpp#394

but I'm definitely no expert.
Comment 18 WADA 2010-11-30 03:52:40 PST
(In reply to comment #17)
> and already outputs UTF8,
> so the HTML parser actually must ignore the <meta> tag at all costs.

Questions.

Why Tb should pass to HTML(5) parser converte-to-utf-8 version of HTML by Tb?
- non base64/quoted-printable, charset specified in Content-Type: header
  HTML in "charset in Content-Type" exists in mail source.
- non base64/quoted-printable, no charset in Content-Type: header
  HTML of unknown charset exists in mail source.
- base64/quoted-printable, charset specified in Content-Type: header
  Before covert to utf-8 by Tb, Tb has to decode data by base64 or QP.
  Decoded data exists and is data in "charset in Content-Type:",
- base64/quoted-printable, charset is not specified in Content-Type: header
  Before covert to utf-8 by Tb, Tb has to decode data by base64 or QP.
  Decoded data exists and is data in unknow charset.
If "charset in Content-Type:" and data of above instead of converted-to-utf-8 version is passed to HTML(5) parser, I think HTML(5) parser and rendering engine will do every thing including "charset in <meta>" handling.
Is it right?

Correct "charset of Content-Type: header" handling and correct "charset of <meta>" handling by Tb is mandatory for resolving <img src="cid:...">, so utf-8-version should be passed to HTML(5) parser and rendering engine?
If so, "charset in <meta>" should be passed when case (B) to fake HTML(5) parser, reversed order of attributes in <meta>, as done when case (A) to fake HTML(5) parser, normal order of attributes in <meta>, which won't produce problem.
Is it right?
Comment 20 Jonathan Protzenko [:protz] 2010-11-30 04:52:13 PST
I think the bottom line is libmime already does handle the conversion to UTF8, and many other parts of Thunderbird are relying on this feature. Plus, no one really knows libmime, and it's been working fine so far, so I don't see any reason for changing this.

But once again, I'm no expert.

The OSX64 build seems to be taking longer than I thought, I hope you're not running OSX. There were no test failures with that patch activated.
Comment 21 Joe Sabash [:JoeS1] 2010-11-30 06:53:14 PST
Testing with the tryserver build in winxp, this seems to fix my testcase here:
https://bugzilla.mozilla.org/attachment.cgi?id=450875
Comment 22 Henri Sivonen (:hsivonen) 2010-11-30 08:03:05 PST
Comment on attachment 493779 [details] [diff] [review]
Tentative patch

I'm not really the right person to review mailnews code.

Today, I discovered that there was an actual bug in this area in the HTML5 parser: kCharsetFromMetaTag was treated incorrectly. (kCharsetFromChannel was treated correctly, though.) This will be fixed in bug 594730. This probably explains why the patch here works (if it does; I didn't test it).

Anyway, if the MIME container has the 
Content-Type: text/html; charset=foo
header, the MIME code should pass the base64-decoded or quoted-printable-decoded bytes to the parser with foo as the charset and kCharsetFromChannel as the source.

If the MIME container doesn't have charset=foo, the MIME code shouldn't be sniffing for <meta>. It should pass the base64-decoded or quoted-printable-decoded bytes to the parser with the user's default charset and kCharsetFromUserDefault as the source.
Comment 23 WADA 2010-11-30 15:21:52 PST
(In reply to comment #22)
> Today, I discovered that there was an actual bug in this area in the HTML5
> parser: kCharsetFromMetaTag was treated incorrectly. (kCharsetFromChannel was
> treated correctly, though.) This will be fixed in bug 594730.

Questions to Henri Sivonen.

As for this bug's problem, problem only when reversed order of attributes in <meta> for http-equiv="Content-Type", it sounds for me that cause of the problem is "html5 parser ignored charset specified in <meta>".
Henri Sivonen, is it right?

If so, the tentative patch by Jonathan Protzenko is not needed and this bug is fixed by your patch for html5 parser only.
Is it right?

Even if this bug is fixed by your patch for html5 parser only, the tentative patch by Jonathan Protzenko should be landed to avoid other unwanted problems?
Comment 24 WADA 2010-11-30 20:48:04 PST
(In reply to comment #21)
> Testing with the tryserver build in winxp, this seems to fix my testcase here:
> https://bugzilla.mozilla.org/attachment.cgi?id=450875

I couldn't reproduce this bug's problem(both Original HTML and Simple HTML) with your tryserver build. I could't reproduce problem of bug 598740(no <meta>, quoted-printable, Simple HTML only issue) too.
Comment 25 Henri Sivonen (:hsivonen) 2010-12-01 00:33:14 PST
(In reply to comment #23)
> As for this bug's problem, problem only when reversed order of attributes in
> <meta> for http-equiv="Content-Type", it sounds for me that cause of the
> problem is "html5 parser ignored charset specified in <meta>".
> Henri Sivonen, is it right?

Probably not exactly right, but that doesn't really matter for the answers below.
 
> If so, the tentative patch by Jonathan Protzenko is not needed and this bug is
> fixed by your patch for html5 parser only.
> Is it right?

Most likely.

> Even if this bug is fixed by your patch for html5 parser only, the tentative
> patch by Jonathan Protzenko should be landed to avoid other unwanted problems?

Yes, assuming that no one want to make the MIME library have the right layering. Comment 20 suggests that there's no interest in making the MIME library have the right layering.

So I'm OK with attachment 493779 [details] [diff] [review] landing, but I'm not a mailnews reviewer. Someone who is should review.
Comment 26 WADA 2010-12-01 02:01:28 PST
(In reply to comment #21)
> Testing with the tryserver build in winxp, this seems to fix my testcase here:
> https://bugzilla.mozilla.org/attachment.cgi?id=450875

I couldn't reproduce problem with multiple <meta> tags in comment #5 too with your tryserver build.

(In reply to comment #25)
> Yes, assuming that no one want to make the MIME library have the right layering.

Henri, correct assumption is perhaps next :-)
  No one here can make the MIME library have the right layering.
Do we need to check with your patch for html5 parser before land of patch by Jonathan Protzenko?
Comment 27 Henri Sivonen (:hsivonen) 2010-12-01 03:17:57 PST
(In reply to comment #26)
> Do we need to check with your patch for html5 parser before land of patch by
> Jonathan Protzenko?

No, attachment 493779 [details] [diff] [review] can land first.
Comment 28 Jonathan Protzenko [:protz] 2010-12-07 13:19:55 PST
Comment on attachment 493779 [details] [diff] [review]
Tentative patch

Andrew, following our discussion on IRC, I'm requesting approval for this patch.

:hsivonen, it does appear that my theory is correct and that libmime already handles the character decoding and outputs UTF8. This means any charset notification that comes from meta tags was until recently ignored, and should continue to be ignored. I can understand that you should regard this as pretty scandalous ;-), but until we actually perform the big libmime rewrite we all dream of, we'll have to stick with that...
Comment 29 Andrew Sutherland [:asuth] 2010-12-07 13:29:02 PST
Comment on attachment 493779 [details] [diff] [review]
Tentative patch

I suggest a lot of baking on trunk will be in order for this...
Comment 30 Andrew Sutherland [:asuth] 2010-12-07 13:29:53 PST
Standard8, for your general release driver scary breakage awareness, may I present this bug...
Comment 31 Jonathan Protzenko [:protz] 2010-12-07 13:31:49 PST
Hey, I pushed to try and it didn't break anything, so it should all be fine, right? Right? ...
Comment 32 Andrew Sutherland [:asuth] 2010-12-07 13:40:59 PST
Sorry, I may have made that sound more concerning that I really feel that it is.  This just seems like one of those problem spaces where I could see there existing many ridiculously illegal messages and that any change in behaviour may result in things that did work but should not have worked no longer working and resulting in a lot of new bug reports or what not.
Comment 33 Joe Sabash [:JoeS1] 2010-12-07 14:23:56 PST
FWIW I ran on the tryserver build for quite some time for a large variety of multi-part related newsgroups postings with no bad effects.
Might I suggest though that ignoring the meta tag data be made subject to a hidden pref. Without that any future changes in the html5 parser would be difficult to evaluate.
Comment 34 Jonathan Protzenko [:protz] 2010-12-08 09:33:31 PST
Joe, I quite didn't get what you mean in your last comment. Since this is the right thing to do anyway, we'll never want the html5 parser to take into account the meta tags, so I'm afraid I don't see the point of having a pref. Could you elaborate? :)
Comment 35 Joe Sabash [:JoeS1] 2010-12-08 14:27:28 PST
(In reply to comment #34)
> Joe, I quite didn't get what you mean in your last comment. Since this is the
> right thing to do anyway, we'll never want the html5 parser to take into
> account the meta tags, so I'm afraid I don't see the point of having a pref.
> Could you elaborate? :)

I'm probably missing something...but might not there be meta-data (new for html5, or older) that would be handled better in the new parser, rather than our libemime. "<meta http-equiv="refresh" content="x" />" for remote content in an iframe comes to mind.
And for the future don't web-apps embed info in meta-data.

Granted, not many folks try these things in TB.
Comment 36 Henri Sivonen (:hsivonen) 2010-12-10 01:47:00 PST
FWIW, bug 594730 landed, so it would be worthwhile to test if it fixed the problem already.
Comment 37 Joe Sabash [:JoeS1] 2010-12-10 10:46:58 PST
Testing with Mozilla/5.0 (Windows NT 5.1; rv:2.0b8pre) Gecko/20101210 Thunderbird/3.3a2pre
My testcase in comment 21 seems to work fine now in the stock nightly build.
Comment 38 WADA 2010-12-10 19:32:59 PST
(In reply to comment #36)
> bug 594730 landed, (snip)

Tested with next build.
> Mozilla/5.0 (Windows NT 5.1; rv:2.0b8pre) Gecko/20101210 Thunderbird/3.3a2pre
Problems with html5.enable=true of all next types was not reproduced.
(a) reversed attributes order in meta tag for charset
(b) multiple meta tags for charset
(c) View/Message Body As/Simple HTML and meta tag(s) for charset
Comment 39 Jonathan Protzenko [:protz] 2010-12-11 14:01:14 PST
:asuth, :bienvenu: :Standard8, the fix in the html5 parser seems to be enough to restore the previous behavior. Shall we consider that my patch is not needed anymore or do you feel like it might be worth applying?
Comment 40 Andrew Sutherland [:asuth] 2010-12-11 15:11:18 PST
I would suggest that we add a mozmill test that streams some messages that had issues before that most recent core patch.  We do not land the new patch, but make sure the mozmill test references this bug.  At such time as we can produce a mozmill test case that requires the patch to pass, we land the patch.
Comment 41 Jonathan Protzenko [:protz] 2010-12-12 03:25:10 PST
Ok, I'll take care of it, adding this on my to-do list. I recall :bienvenu saying that streaming messages through a docshell was not entirely scriptable, so I might end up doing this through mozmill, but I all cases, I don't think it should be too hard to write.
Comment 42 Mark Banner (:standard8) 2011-05-06 05:04:06 PDT
Based on comments 36 - 39 it looks like the core change fixed this bug.

I do agree with Andrew that we should get a testcase for this, either in this bug, or in a follow-up bug (with this one marked as wontfix).
Comment 43 Rimas Kudelis 2012-12-08 00:29:54 PST
Hi folks,
FYI, this bug is manifesting again, and you don't even need html5.enable=true, as that setting doesn't even seem to exist in Thunderbird 20:
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20100101 Thunderbird/20.0a1
I tried adding the html5.enable setting in about config, and tried it at both true and false, and that doesn't affect anything.
I can attach a few more test messages, but WADA's test messages work fine as well.

Should I report this as a separate bug, or can it be dealt with here?
Comment 44 Masatoshi Kimura [:emk] 2012-12-08 03:25:59 PST
HTML5 parser was always enabled far back.
Comment 45 WADA 2012-12-10 18:53:49 PST
(In reply to Rimas Kudelis from comment #43)
> but WADA's test messages work fine as well.

As clearly written in bug summary, and as seen in very very simple test mails, this bug is for problem, with additional condition written at Whiteboard: field, in test mails which I attached to this bug. This bug is never for generic "garbled display" phenomenon.
It indicates that your case is different from my case(==this bug's case).  

> Should I report this as a separate bug, or can it be dealt with here?

Open separate bug for your case, with attaching mail, with clear description about specific conditions in your case, please.
Comment 46 Rimas Kudelis 2012-12-10 23:16:12 PST
I'm not exactly following you.
The problem I referred to also depends on same conditions ("View → Message body as → Original HTML" and the order of attributes in Content-Type meta header reversed). The only difference from your case is that html5.enable doesn't matter anymore (presumably because that pref is now non-functional and html5 is the default).

I can report it as a separate bug, no problem, but I strongly suspect that it's still the same problem.
Comment 47 WADA 2012-12-11 00:18:04 PST
(In reply to Rimas Kudelis from comment #46)
Bug summry implies "problem never occurs if order is not reversed".
See simplest test case I attached, please.
Do you mean "no problem occurs in your case too if order of parameter in meta tag is changed to normai"?
Comment 48 Rimas Kudelis 2012-12-11 00:31:40 PST
(In reply to WADA from comment #47)
> Bug summry implies "problem never occurs if order is not reversed".
> See simplest test case I attached, please.
> Do you mean "no problem occurs in your case too if order of parameter in
> meta tag is changed to normai"?

Yes, that's what I mean.

Just test your own simple messages with html5.enable unset in latest nightly, and you should see the problem manifest itself.
Comment 49 WADA 2012-12-11 02:20:38 PST
I also could observe garbled display with test mail-02 in my test case what I attached to this bug in following trunk nightly.
> Application Build ID=20121210030329 
> Mozilla/5.0 (Windows NT 5.1; rv:20.0) Gecko/20100101 Thunderbird/20.0a1.
You said "no problem with my test case, with Tb 20, in your test", didn't you?
Did your "work fine" mean "successfully reproduce problem/regression"?

I couldn't see problem in trunk of Application Build ID=20121009030549.
> Mozilla/5.0 (Windows NT 5.1; rv:19.0) Gecko/19.0 Thunderbird/19.0a1
No html5.enabled is shown by Config Editor too. 

Apparently regression in Tb 20. Change like proposed patch by Jonathan Protzenko may be needed in Tb 20's code. 
Rimas Kudelis, open new bug with referring to this bug and test case of this bug, please, because original problem of this bug with html5.enabled=true itself was resolved in Tb 3.0beta and problem of this bug didn't occur till Tb 19.
Comment 50 WADA 2012-12-11 02:28:46 PST
Rimas Kudelis, if possible, find regression window by binary search of trunk nightly build, please.
Comment 51 Rimas Kudelis 2012-12-11 03:15:50 PST
(In reply to WADA from comment #49)
> I also could observe garbled display with test mail-02 in my test case what
> I attached to this bug in following trunk nightly.
> > Application Build ID=20121210030329 
> > Mozilla/5.0 (Windows NT 5.1; rv:20.0) Gecko/20100101 Thunderbird/20.0a1.
> You said "no problem with my test case, with Tb 20, in your test", didn't
> you?
> Did your "work fine" mean "successfully reproduce problem/regression"?

Yes, that's what I meant: they work fine as reproducable testcases. Sorry for my vagueness.

> 
> I couldn't see problem in trunk of Application Build ID=20121009030549.
> > Mozilla/5.0 (Windows NT 5.1; rv:19.0) Gecko/19.0 Thunderbird/19.0a1
> No html5.enabled is shown by Config Editor too. 
> 
> Apparently regression in Tb 20. Change like proposed patch by Jonathan
> Protzenko may be needed in Tb 20's code. 
> Rimas Kudelis, open new bug with referring to this bug and test case of this
> bug, please, because original problem of this bug with html5.enabled=true
> itself was resolved in Tb 3.0beta and problem of this bug didn't occur till
> Tb 19.

OK. Not sure about narrowing the window, but I'll report the bug in a few minutes.

By the way, if this bug is fixed, then why is it still open?
Comment 52 WADA 2012-12-11 03:53:52 PST
(In reply to Rimas Kudelis from comment #51)
> By the way, if this bug is fixed, then why is it still open?

(a) Original problem of this bug incidentally disappeared by change of bug 594730 in HTML parset/HTML5 parser, but no one knows correct solution in Tb side to perfectly resolve issues in Tb.
(b) "Patch attached by Jonathan Protzenko is needed in Tb or not" is still unclear.
(c) Automated test for this bug is not available yet.
Comment 53 WADA 2012-12-11 03:55:38 PST
I'll close this bug after you open new bug for regression in Tb 20, in order to avoid confusion.
Comment 54 Rimas Kudelis 2012-12-11 04:36:30 PST
Reported as bug 820335
Comment 55 WADA 2012-12-11 04:55:30 PST
Changing to FIXED.
Comment 56 Jonathan Protzenko [:protz] 2013-01-09 08:59:34 PST
*** Bug 820335 has been marked as a duplicate of this bug. ***
Comment 57 Jonathan Protzenko [:protz] 2013-01-09 09:00:54 PST
So apparently bug 820335 has a test case that shows that the patch from this bug is actually needed, so I'm reopening.

What remains to be done:
- write a mozmill test or an xpcshell test that exercises the bug,
- commit something in the tree.
Comment 58 Jonathan Protzenko [:protz] 2013-01-09 09:02:00 PST
WADA, if you feel like this should remain in bug 820335, feel free to undo my changes :), but please post a copy of the patch over there then.
Comment 59 Joshua Cranmer [:jcranmer] 2013-01-11 11:59:47 PST
*** Bug 822641 has been marked as a duplicate of this bug. ***
Comment 60 Rimas Kudelis 2013-01-13 12:52:24 PST
Gee, if noone is going to prepare any tests, can we please have the patch committed into the repo? It would absolutely suck if we'd keep this bug manifesting just because nobody has time or is willing to prepare tests. Tests should be there to prevent breakage, not to prevent bugfixes from happening...

please!
Comment 61 Henri Sivonen (:hsivonen) 2013-01-14 04:22:03 PST
When looking at nsIMarkupDocumentViewer for an unrelated reason, I noticed that there are some sad code in comm-central for attempting to make the HTML parser use UTF-8. See bug 829543.

If you have a component in mailnews that converts messages to UTF-8 before passing them to the HTML parser, the best way to make sure that the HTML parser consumes the data as UTF-8 is to start the stream with the UTF-8 BOM.
Comment 62 Rimas Kudelis 2013-01-23 03:34:12 PST
*bump*
Jonathan, is there anything that has to be done prior to checking your patch in?
Comment 63 Jonathan Protzenko [:protz] 2013-01-23 06:40:08 PST
In order to checkin my patch, there need to be a test for it first.

However, since I read bug 829543, I believe that fixing bug 829543 is the right way to fix our problems here, and once bug 829543 is fixed, the present bug will be fixed as a side-effect.

So fixing 829543 is the right thing to do now.
Comment 64 Rimas Kudelis 2013-01-23 07:13:04 PST
Are you sure that actually fixing this (even if temporarily) by your patch is a no-no? It would be sad to have this bug pushed on users just because a patch that works is not the Right Way. I wonder if there's any timeframe for Bug 829543. Does your patch even prevent Bug 829543 from being fixed anyhow?
Comment 65 Vladimir Morozov 2013-02-22 09:05:35 PST
*** Bug 843990 has been marked as a duplicate of this bug. ***
Comment 66 Philip Chee 2013-02-23 07:30:34 PST
*** Bug 843600 has been marked as a duplicate of this bug. ***
Comment 67 Philip Chee 2013-02-28 03:43:11 PST
*** Bug 845963 has been marked as a duplicate of this bug. ***
Comment 68 Philip Chee 2013-02-28 03:43:39 PST
*** Bug 845833 has been marked as a duplicate of this bug. ***
Comment 69 Philip Chee 2013-02-28 03:44:12 PST
*** Bug 845421 has been marked as a duplicate of this bug. ***
Comment 70 Henri Sivonen (:hsivonen) 2013-02-28 03:52:08 PST
(In reply to Rimas Kudelis from comment #64)
> Does your patch even prevent Bug 829543 from being fixed anyhow?

Not technically. In terms of sense of urgency, maybe.

Given how the duplicates keep flowing in, I think it would make sense to land the patch here.
Comment 71 Rimas Kudelis 2013-02-28 12:53:48 PST
Henri, well landing the patch would be appreciated, at least on my side...

What surprises me is how this issue has been handled up until now. It's rather obvious that bug 829543 isn't being fixed as quickly as desired, and now that the sh*t hit the fan, those dupes flowing are anything but unexpected.

I would encourage to commit this fix to comm-aurora and comm-beta as well, cause it seems really lame to let a regression like that slip freely into release just because some tests are missing, that it seems nobody is willing to write anyway.

Please forgive my grumpiness. I know it's not exactly the best tone I could use in Bugzilla, but I'm a Thunderbird user myself, and I'm really annoyed by what has happened.
Comment 72 Phoenix 2013-03-20 11:13:40 PDT
*** Bug 852984 has been marked as a duplicate of this bug. ***
Comment 73 Magnus Melin 2013-03-20 11:35:06 PDT
So, seems we should land this, even if it turns out to be just a temp fix.
Comment 74 Jonathan Protzenko [:protz] 2013-03-20 13:51:31 PDT
Yes, I think we should land this, even though I don't even have time to write tests for this. I didn't land it because I'd like to have confirmation from someone else (e.g. standard8) that landing this without tests is the right call.
Comment 75 Joshua Cranmer [:jcranmer] 2013-03-20 18:19:53 PDT
There is a student who has expressed interest in writing tests for this.
Comment 76 Mark Banner (:standard8) 2013-03-22 08:05:36 PDT
Joshua said over irc he'll look at moving this forward.
Comment 77 j.heinz 2013-03-22 08:32:19 PDT
Created attachment 728241 [details]
testcase, contains two mails, one with display problems and one correct

Hi,

here is a testcase (mailfolder called "Test" containing 2 mails which show the difference).

Best regards
J.
Comment 78 Tony Mechelynck [:tonymec] 2013-03-23 23:29:29 PDT
*** Bug 851488 has been marked as a duplicate of this bug. ***
Comment 79 Tony Mechelynck [:tonymec] 2013-04-04 08:01:06 PDT
*** Bug 857992 has been marked as a duplicate of this bug. ***
Comment 80 Szabolcs Hubai (:xabolcs) 2013-04-04 21:06:23 PDT
Created attachment 733720 [details] [diff] [review]
tests v1

The test compares the textContent of body tag for the .eml files.
It may be not the best solution. :)

Two tests included
- one for new line delimited "charset" attribute (see Bug 822641 Comment #2)
- and the other one for reverse ordered attributes

Interestingly the reverse ordered test passes.

Forgot to test with the fix applied. :/
Comment 81 Joshua Cranmer [:jcranmer] 2013-04-04 22:09:59 PDT
(In reply to Szabolcs Hubai (:xabolcs) from comment #80)
> Created attachment 733720 [details] [diff] [review]
> tests v1

/me smells the start of a test suite for comparing email rendering... exactly what I'll need down the line for the jsmime stuff. :-)

Running this test with and without the patch to see what happens.
Comment 82 Szabolcs Hubai (:xabolcs) 2013-04-06 07:57:02 PDT
Comment on attachment 733720 [details] [diff] [review]
tests v1

One more question about those fancy characters.
Should I, could I check in those *.eml files as a real (with bom) utf-8 file?
Does mercrurial (and diff, and bugzilla) understand "bommed" files?

BTW I would be even more happy if I could add the japanese test messages too from WADA's attachement [1].


[1] https://bugzilla.mozilla.org/attachment.cgi?id=473359
Comment 83 Joshua Cranmer [:jcranmer] 2013-04-06 09:03:17 PDT
(In reply to Szabolcs Hubai (:xabolcs) from comment #82)
> Comment on attachment 733720 [details] [diff] [review]
> tests v1
> 
> One more question about those fancy characters.
> Should I, could I check in those *.eml files as a real (with bom) utf-8 file?
> Does mercrurial (and diff, and bugzilla) understand "bommed" files?

The BOM doesn't go over well with email (and it's also frowned upon by Unicode people). A safer alternative is to encode the problematic characters with, say, quoted printable (a test case I use locally to verify the patch was using =91, for example).
Comment 84 Phoenix 2013-04-07 04:33:58 PDT
*** Bug 857974 has been marked as a duplicate of this bug. ***
Comment 85 Woody Suwalski 2013-04-07 12:25:55 PDT
*** Bug 858982 has been marked as a duplicate of this bug. ***
Comment 86 Szabolcs Hubai (:xabolcs) 2013-04-07 13:36:47 PDT
Comment on attachment 733720 [details] [diff] [review]
tests v1

Building with c-c qparent: 1d584cce7616, m-c:97cfc16ba5dc (due to bug 858014) ...
... without tentative patch [1] applied, tests fails;
... with tentative patch applied, tests passes. :)


BTW I'd to do some experiments with messages containing quoted printable characters.



[1] https://bugzilla.mozilla.org/attachment.cgi?id=493779
Comment 87 Mike Conley (:mconley) - (needinfo me!) 2013-04-09 23:40:39 PDT
Comment on attachment 733720 [details] [diff] [review]
tests v1

Review of attachment 733720 [details] [diff] [review]:
-----------------------------------------------------------------

Hey xabolcs, these are *fantastic* looking tests! Thanks so much for doing this!

Just a few nits below. I haven't had a chance to run these tests yet, but once you update the patch, I'll happily push to try to see how they perform.

Great work,

-Mike

::: mail/test/mozmill/message-reader/test-bug594646.js
@@ +1,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> +  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> +  * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +/**

Format block comments like:

/**
 * Blah blah..
 * blah blah blah..
 */

@@ +12,5 @@
> +const RELATIVE_ROOT = "../shared-modules";
> +const MODULE_REQUIRES = ["folder-display-helpers"];
> +
> +function setupModule(module) {
> +   collector.getModule("folder-display-helpers").installInto(module);

It looks like you're using 3 space indentation. Please use 2 instead.

@@ +13,5 @@
> +const MODULE_REQUIRES = ["folder-display-helpers"];
> +
> +function setupModule(module) {
> +   collector.getModule("folder-display-helpers").installInto(module);
> +   collector.getModule("window-helpers").installInto(module);

If this module uses window-helpers, then window-helpers should be added to MODULE_REQUIRES.

@@ +23,5 @@
> +
> +   // Be sure to view message body as Original HTML
> +   msgc.window.MsgBodyAllowHTML();
> +
> +   let textContent = msgc.window.msgWindow.messageWindowDocShell

For long chains like this, we tend to do something like:

let textContent = msgc.window
                      .msgWindow
                      .messageWindowDocShell
                      .contentViewer
                      .DOMDocument
                      .documentElement
                      .textContent;

@@ +30,5 @@
> +   close_window(msgc);
> +   return textContent;
> +}
> +
> +/**

Indent one more space.

@@ +39,5 @@
> +   let textContentB = extract_eml_body_textcontent(emlB);
> +   assert_equals(textContentA, textContentB);
> +}
> +
> +/**

Indent one more space.

@@ +49,5 @@
> +function test_original_html_characters_head_meta_content_charset_httpEq() {
> +   check_eml_textcontent("./bug594646_reference.eml", "./bug594646_reversed_order.eml");
> +}
> +
> +/**

Indent one more space.
Comment 88 Szabolcs Hubai (:xabolcs) 2013-04-10 01:18:10 PDT
(In reply to Mike Conley (:mconley) from comment #87)
> Comment on attachment 733720 [details] [diff] [review]
> tests v1
> 
> Review of attachment 733720 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hey xabolcs, these are *fantastic* looking tests! Thanks so much for doing
> this!

Feedback like this makes me happy! :)

> 
> Just a few nits below. I haven't had a chance to run these tests yet, but
> once you update the patch, I'll happily push to try to see how they perform.
> 

I manually tested QP messages, and they are also garbled in original HTML mode.

To increase compatibility and portablility I'd like to "convert" the already
included *.emls into quoted printable format.
Should I keep the 8-bit encoded messages, and extend the test coverage with QP encoding,
or may I include QP encoded messages only?

> Great work,
> 
> -Mike
> 


Thanks!
Comment 89 Mike Conley (:mconley) - (needinfo me!) 2013-04-11 00:46:17 PDT
(In reply to Szabolcs Hubai (:xabolcs) from comment #88)
> (In reply to Mike Conley (:mconley) from comment #87)
> > Comment on attachment 733720 [details] [diff] [review]
> > tests v1
> > 
> > Review of attachment 733720 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Hey xabolcs, these are *fantastic* looking tests! Thanks so much for doing
> > this!
> 
> Feedback like this makes me happy! :)
> 
> > 
> > Just a few nits below. I haven't had a chance to run these tests yet, but
> > once you update the patch, I'll happily push to try to see how they perform.
> > 
> 
> I manually tested QP messages, and they are also garbled in original HTML
> mode.
> 
> To increase compatibility and portablility I'd like to "convert" the already
> included *.emls into quoted printable format.
> Should I keep the 8-bit encoded messages, and extend the test coverage with
> QP encoding,
> or may I include QP encoded messages only?

I don't think I know enough about the problem domain to really say with any certainty. I just know how to write Mozmill tests. :)

Maybe jcranmer has a position on this?
Comment 90 Phoenix 2013-04-12 10:24:55 PDT
*** Bug 861129 has been marked as a duplicate of this bug. ***
Comment 91 Joshua Cranmer [:jcranmer] 2013-04-12 14:51:01 PDT
I tend to prefer QP-encoded/B64-encoded messages in tests to 8-bit messages, simply because they interact better with software. Our mime subsystem will anyways decode quoted-printable/base64 to Unicode before passing it off anyways, so QP encoding really oughtn't have an effect.
Comment 92 Szabolcs Hubai (:xabolcs) 2013-04-14 13:58:51 PDT
Created attachment 737305 [details] [diff] [review]
tests v2

Addressed nits:
- 2 space indent
- block comments
- MODULE_REQUIRES
- textContext long chain

"one more space indent" doesn't applied yet.
Are you sure that is what you want?

Text context of the reference message was promoted to a global variable,
and function check_eml_textcontent() was modified to take only one eml,
and compare that one to the reference.

One more test was added - http-equiv attribute is in new line.

Messages was converted to QP encoding.
Interestingly messages in QP encoding shows different results from 8 bit messages.
Therefore I ask only for feedback.

jcranmer! It seems like there is a little effect due to encoding.
I'll include the 8 bit encoded messages in the next version in the next days.
Are there any bug about this difference?
Comment 93 Mike Conley (:mconley) - (needinfo me!) 2013-04-15 08:09:43 PDT
Comment on attachment 737305 [details] [diff] [review]
tests v2

Review of attachment 737305 [details] [diff] [review]:
-----------------------------------------------------------------

Looking good!

::: mail/test/mozmill/message-reader/test-bug594646.js
@@ +13,5 @@
> +
> +const RELATIVE_ROOT = "../shared-modules";
> +const MODULE_REQUIRES = ["folder-display-helpers", "window-helpers"];
> +
> +let referenceTextContent;

Globals should be prefixed with a g, so let gReferenceTextContent.
Comment 94 Joshua Cranmer [:jcranmer] 2013-04-15 08:10:57 PDT
(In reply to Szabolcs Hubai (:xabolcs) from comment #92)
> jcranmer! It seems like there is a little effect due to encoding.
> I'll include the 8 bit encoded messages in the next version in the next days.
> Are there any bug about this difference?

Can you be more specific about what the difference is?
Comment 95 Szabolcs Hubai (:xabolcs) 2013-04-16 00:35:31 PDT
Created attachment 737850 [details]
8bit and qp encoded messages

(In reply to Joshua Cranmer [:jcranmer] from comment #94)
> (In reply to Szabolcs Hubai (:xabolcs) from comment #92)
> > jcranmer! It seems like there is a little effect due to encoding.
> > I'll include the 8 bit encoded messages in the next version in the next days.
> > Are there any bug about this difference?
> 
> Can you be more specific about what the difference is?

Check zipped emls with affected builds! (I hope my QP messages are valid.)
* test_newline_charset_8bit.eml - mojibaked
* test_newline_charset_qp.eml - OK

* test_newline_httpequiv_8bit.eml - OK
* test_newline_httpequiv_qp.eml - mojibaked

* test_reverse_order_8bit.eml - mojibaked
* test_reverse_order_qp.eml - mojibaked


I tought you say, 8bit and QP encodings should provide the same test results,
but with the attached emls they provide different results.

Are there any bug about this difference?
Or this is a feature? ;)
Comment 96 Joshua Cranmer [:jcranmer] 2013-04-17 05:44:34 PDT
(In reply to Szabolcs Hubai (:xabolcs) from comment #95)
> Are there any bug about this difference?
> Or this is a feature? ;)

I'm only mildly surprised at there being a difference, but I don't think it's worth filing a bug. If you have tests to see if any of them are mojibaked they can be fixed all at once.
Comment 97 Szabolcs Hubai (:xabolcs) 2013-04-17 17:28:25 PDT
Created attachment 738825 [details] [diff] [review]
tests v3

- added back the 8 bit encoded messages
- addressed gReferenceTextContent nit


And the testrun. :)

> INFO Passed: 5
> INFO Failed: 0
> INFO Skipped: 0
> SUMMARY-PASS | test-bug594646.js::setupModule
> SUMMARY-PASS | test-bug594646.js::test_original_html_characters_head_meta_content_charset_httpEq
> SUMMARY-PASS | test-bug594646.js::test_original_html_characters_head_meta_httpEq_content_newline_charset
> SUMMARY-PASS | test-bug594646.js::test_original_html_characters_head_meta_content_charset_newline_httpEq
> SUMMARY-PASS | test-bug594646.js::teardownModule
Comment 98 WADA 2013-04-17 20:44:42 PDT
(In reply to Szabolcs Hubai (:xabolcs) from comment #95)
> * test_newline_charset_8bit.eml - mojibaked
> * test_newline_charset_qp.eml - OK
> * test_newline_httpequiv_8bit.eml - OK
> * test_newline_httpequiv_qp.eml - mojibaked
> * test_reverse_order_8bit.eml - mojibaked
> * test_reverse_order_qp.eml - mojibaked
> Are there any bug about this difference?
> Or this is a feature? ;)

Quoted-printable case is bug 571704 and bug 598740.
- bug 571704 : Orginal HTML, quoted-printable, meta : reversed order
               I closed as dup of this bug.
- bug 598740 : Simple HTML, any of quoted-printable and base64,
               meta : any order
Comment 99 WADA 2013-04-17 21:05:26 PDT
FYI. Relevant conditions:
(1) View/Message Body As : (1-1) Original HTML, (1-2) Simple HTML
(2) Content-Type charset : (2-1) UTF-8,         (2-2) Non UTF-8
(3) C-T-Encoding : (3-1) 8/7bits, (3-2) quoted-printable, (3-3) base64
(4) <meta> tag           : (4-1) Normal order,  (4-2) Reversed order
(5) <meta> charset       : (5-1) UTF-8,         (5-2) Non UTF-8
(6) Number of <meta>     : (6-1) one <meta>,    (6-2) multiple <meta>
    if multi-meta, many variations in <meta> order and <meta> charset.
(7) Tb version :
    (7-1) Bug 594646 occured initially,
    (7-2) Bug 594646 disappeared once, after change in HTML parser
    (7-3) Bug 594646 came back
    (7-4) Patch for Bug 594646 by Jonathan Protzenko is applied
Comment 100 Tony Mechelynck [:tonymec] 2013-04-23 14:11:26 PDT
*** Bug 857858 has been marked as a duplicate of this bug. ***
Comment 101 Szabolcs Hubai (:xabolcs) 2013-05-06 13:13:32 PDT
Comment on attachment 738825 [details] [diff] [review]
tests v3

FTR.

[21:20] xabolcs mconley: wada in comment #99 wrote a lot about combinations about buggy meta tags
[21:20] xabolcs mconley: how much of them should i add into the testcase?
[21:21] mconley xabolcs: oh my
[21:21] mconley xabolcs: that's a lot of cases
[21:21] mconley xabolcs: we probably shouldn't block landing bug 594646 for all of those
[21:21] xabolcs mconley: yeah
[21:21] mconley xabolcs: I'd rather we deal with them in follow-ups.

<snip>

[22:11] xabolcs mconley: well, i try to add one or two more test email to the testcase and update the patch
[22:11] mconley xabolcs: sorry - didn't notice you'd responded (include my name next time in case I'm unresponsive, as I may have moved channels)
[22:11] xabolcs mconley: and attach to the bug
[22:11] mconley xabolcs: I think that's a fine strategy
[22:11] xabolcs mconley: ok, will do
Comment 102 Phoenix 2013-05-09 10:43:43 PDT
*** Bug 870301 has been marked as a duplicate of this bug. ***
Comment 103 Szabolcs Hubai (:xabolcs) 2013-05-12 12:32:55 PDT
(In reply to WADA from comment #99)
> <snip>
> FYI. Relevant conditions:
> (6) Number of <meta>     : (6-1) one <meta>,    (6-2) multiple <meta>
>     if multi-meta, many variations in <meta> order and <meta> charset.
> <snip>

FYI this fix doesn't fix emails with multiple meta tags (case (6-2), "see also" bug 855329).

protz, jcranmer: should it fix?
Comment 104 Joshua Cranmer [:jcranmer] 2013-05-22 16:36:34 PDT
(In reply to Szabolcs Hubai (:xabolcs) from comment #103)
> (In reply to WADA from comment #99)
> > <snip>
> > FYI. Relevant conditions:
> > (6) Number of <meta>     : (6-1) one <meta>,    (6-2) multiple <meta>
> >     if multi-meta, many variations in <meta> order and <meta> charset.
> > <snip>
> 
> FYI this fix doesn't fix emails with multiple meta tags (case (6-2), "see
> also" bug 855329).
> 
> protz, jcranmer: should it fix?

It's best to keep that to a separate bug, especially since it's a less common issue.
Comment 105 Szabolcs Hubai (:xabolcs) 2013-05-23 01:05:52 PDT
(In reply to Joshua Cranmer [:jcranmer] from comment #104)
> (In reply to Szabolcs Hubai (:xabolcs) from comment #103)
> > 
> > FYI this fix doesn't fix emails with multiple meta tags (case (6-2), "see
> > also" bug 855329).
> > 
> > protz, jcranmer: should it fix?
> 
> It's best to keep that to a separate bug, especially since it's a less
> common issue.

Thanks!
Will hack some base64 encoded mail and update the test patch today.
Comment 106 Szabolcs Hubai (:xabolcs) 2013-05-23 03:34:34 PDT
Created attachment 753208 [details] [diff] [review]
tests v4

Added base64 encoded messages.


Tested as
> make SOLO_TEST=message-reader/test-bug594646.js  mozmill-one

Interestingly I got 1 TEST-UNEXPECTED-FAIL with c-c: fd449e55a4d3 m-c: c21ef3664c67.
See below!

> INFO Passed: 5
> INFO Failed: 1
> INFO Skipped: 0
> SUMMARY-UNEXPECTED-FAIL | test-nntp-helpers.js | test-nntp-helpers.js::<TOP_LEVEL>
>   EXCEPTION: MailServices is not defined
>     at: nonesuch line 129
> SUMMARY-PASS | test-bug594646.js::setupModule
> SUMMARY-PASS | test-bug594646.js::test_original_html_characters_head_meta_content_charset_httpEq
> SUMMARY-PASS | test-bug594646.js::test_original_html_characters_head_meta_httpEq_content_newline_charset
> SUMMARY-PASS | test-bug594646.js::test_original_html_characters_head_meta_content_charset_newline_httpEq
> SUMMARY-PASS | test-bug594646.js::teardownModule
> make: *** [mozmill-one] Error 1
Comment 107 Magnus Melin 2013-05-23 03:38:03 PDT
That's also discussed in bug 99019.
Comment 108 Magnus Melin 2013-05-23 03:39:51 PDT
Err, bug 874690.
Comment 109 Szabolcs Hubai (:xabolcs) 2013-05-27 23:29:29 PDT
mconley: ping.

(In reply to Szabolcs Hubai (:xabolcs) from comment #106)
> Created attachment 753208 [details] [diff] [review]
> tests v4
> 
> Added base64 encoded messages.
> 
>
Comment 110 WADA 2013-05-28 00:10:03 PDT
Question to developers:
What is reason why "Tentative patch by Jonathan Protzenko"(one line change) is still not landed even on trunk, even though status = review++ ?
Automated test is mandatory to land and ship the one-line patch?
"not land the patch" is needed to complete auto-test implmentation?
Comment 111 Jonathan Protzenko [:protz] 2013-05-28 00:20:30 PDT
Last time I checked, a test was mandatory to check that in, and I don't have any time to write such a test. I suggest, with the upcoming release of Thunderbird 24, that we check the patch in without tests.

Mark, would you be ok with giving me a green light for committing this as an interim fix? I feel like the issue stings users pretty hard, and we wouldn't want this to be in the next stable release...
Comment 112 Jonathan Protzenko [:protz] 2013-05-28 00:22:36 PDT
WADA: re-reading the bug, I think we just need to ping mconley for the test review. The test is already written, we just need it to be reviewed.

Mark, could we set this bug as blocking-thunderbird24?
Comment 113 Joshua Cranmer [:jcranmer] 2013-05-28 05:34:29 PDT
I had the patch in my queue ready for imminent landing, I was just waiting for the test to get reviewed, which has gone through far more iterations than I was expecting.
Comment 114 Mike Conley (:mconley) - (needinfo me!) 2013-05-28 06:41:32 PDT
Comment on attachment 753208 [details] [diff] [review]
tests v4

Review of attachment 753208 [details] [diff] [review]:
-----------------------------------------------------------------

An excellently written test. Thanks Szabolcs!
Comment 116 WADA 2013-06-22 00:47:49 PDT
*** Bug 884973 has been marked as a duplicate of this bug. ***
Comment 117 Tony Mechelynck [:tonymec] 2013-06-25 07:58:24 PDT
Mozilla/5.0 (X11; Linux i686 on x86_64; rv:25.0) Gecko/20100101 Firefox/25.0 SeaMonkey/2.22a1 ID:20130625003001 c-c:dd35e67c6af4 m-c:bc569033125a

I VERIFY that HTML emails where accented Latin letters were corrupt before because of this bug, appear now all right, even with View → Message Body As → Original HTML.

If someone from the Thunderbird QA team can do the same check on his/her side, the Status of this bug can be changed to VERIFIED.
Comment 118 Tony Mechelynck [:tonymec] 2013-06-25 08:00:41 PDT
... and I didn't request a Version or Milestone change: :-?
Comment 119 Tony Mechelynck [:tonymec] 2013-07-12 19:15:12 PDT
*** Bug 892914 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.