862757 - Permanent Orange: TEST-UNEXPECTED-FAIL | test_detectAttachmentCharset.js | UTF-16 == UTF-16LE - See following stack:

Status: RESOLVED FIXED

(MailNews Core :: Attachments, defect)

Description

[Mark Banner (:standard8)]

This is a regression from bug 860180 - since that landed, our xpcshell tests are now failing with:

https://tbpl.mozilla.org/php/getParsedLog.php?id=21908868&tree=Thunderbird-Trunk#error0

TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/mailnews/compose/test/unit/test_detectAttachmentCharset.js | UTF-16 == UTF-16LE - See following stack:
JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/head.js :: do_throw :: line 472
JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/head.js :: do_report_result :: line 574
JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/head.js :: _do_check_eq :: line 584
JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/head.js :: do_check_eq :: line 591
JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/tests/mailnews/compose/test/unit/test_detectAttachmentCharset.js :: checkAttachmentCharset :: line 16
JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/tests/mailnews/compose/test/unit/test_detectAttachmentCharset.js :: testUTF16 :: line 34
JS frame :: ../../../resources/asyncTestUtils.js :: _async_driver :: line 156
JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/head.js :: .run :: line 439
native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

Whilst we could change the content type, I'm not sure that's the right thing, as supposedly bug 860180 was to fix a regression, but appears to have introduced a different regression.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Fix a mailnews test to keep up with the UTF-16 handling update

UTF-16LE and UTF-16BE are the canonical encoding names now.

Comment 2

[Henri Sivonen (:hsivonen)]

Solving this with attachment 738459 [details] [diff] [review] is my preference, too.

The feature being tested here is rather questionable in my opinion, since it elevates a guess into authoritative metadata instead of letting the recipient to guess.

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 738459 [details] [diff] [review]
Fix a mailnews test to keep up with the UTF-16 handling update

Thanks for the patch.

Comment 4

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/comm-central/rev/bd4327b7d487

Comment 5

[Masatoshi Kimura [:emk]]

Followup to disable UTF-16BE testcase.
https://hg.mozilla.org/comm-central/rev/85a59368547c

nsMsgAttachmentHandler::PickCharset uses nsILineInputStream which nsFileInputStream implements and nsUniversalDetector.
nsFileInputStream::ReadLine calls NS_ReadLine which will truncate the input at the first NUL character.
https://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsFileStreams.cpp#498
https://mxr.mozilla.org/mozilla-central/source/netwerk/base/public/nsReadLine.h#72
nsUniversalDetector will overlook the UTF-16 BOM if the input contains only the BOM.
https://mxr.mozilla.org/comm-central/source/mozilla/extensions/universalchardet/src/base/nsUniversalDetector.cpp
So if the first character (excluding BOM) is in the Latin-1 range (<= 0xFF), nsMsgAttachmentHandler::PickCharset will misdetect the charset.

I'll file a followup bug to fix that.

Comment 6

[Joshua Cranmer [:jcranmer]]

Looks like bug 860180 was backported to mozilla-aurora and mozilla-beta, so I presume we need to backport this patch as well?

Comment 7

[Masatoshi Kimura [:emk]]

Comment on attachment 738459 [details] [diff] [review]
Fix a mailnews test to keep up with the UTF-16 handling update

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 860180
User impact if declined: None. This patch will only fix the permaorange on Thunderbird tinderboxes.
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): Minimal, test only change.
String or IDL/UUID changes made by this patch: None.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 738459 [details] [diff] [review]
Fix a mailnews test to keep up with the UTF-16 handling update

[Triage Comment]
Yep, looks like we do. a=Standard8

Comment 9

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-aurora/rev/8df8bf7b0e71
https://hg.mozilla.org/releases/comm-beta/rev/4c95dfc465de