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

RESOLVED FIXED in Thunderbird 23.0

Status

MailNews Core
Attachments
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: standard8, Assigned: emk)

Tracking

({regression})

Trunk
Thunderbird 23.0
regression
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird21 fixed, thunderbird22 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Blocks: 860180
(Assignee)

Comment 1

5 years ago
Created attachment 738459 [details] [diff] [review]
Fix a mailnews test to keep up with the UTF-16 handling update

UTF-16LE and UTF-16BE are the canonical encoding names now.
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #738459 - Flags: review?(mbanner)
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.
(Reporter)

Comment 3

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

Thanks for the patch.
Attachment #738459 - Flags: review?(mbanner) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/comm-central/rev/bd4327b7d487
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 5

5 years ago
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.
(Assignee)

Updated

5 years ago
Depends on: 863025
Looks like bug 860180 was backported to mozilla-aurora and mozilla-beta, so I presume we need to backport this patch as well?
(Assignee)

Comment 7

5 years ago
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.
Attachment #738459 - Flags: approval-mozilla-beta?
Attachment #738459 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 8

5 years ago
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
Attachment #738459 - Flags: approval-mozilla-beta?
Attachment #738459 - Flags: approval-mozilla-aurora?
Attachment #738459 - Flags: approval-comm-beta+
Attachment #738459 - Flags: approval-comm-aurora+
(Reporter)

Comment 9

5 years ago
https://hg.mozilla.org/releases/comm-aurora/rev/8df8bf7b0e71
https://hg.mozilla.org/releases/comm-beta/rev/4c95dfc465de
status-thunderbird21: --- → fixed
status-thunderbird22: --- → fixed
Target Milestone: --- → Thunderbird 23.0
You need to log in before you can comment on or make changes to this bug.