1297761 - TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-charset-edit.js

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-charset-edit.js | test-charset-edit.js::test_wrong_reply_charset
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-charset-edit.js | test-charset-edit.js::test_no_mojibake 

First seen Wed Aug 24, 2016, 16:01:00
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=44a362f04d8d444b7e84ff772a28fed383a58ab0

Most likely caused by bug 1191841.

Comment 1

[Jorg K (CEST = GMT+2)]

mozmake SOLO_TEST=composition/test-drafts.js mozmill-one

Failures are:

SUMMARY-UNEXPECTED-FAIL | c:\mozilla-source\comm-central\mail\test\mozmill\composition\test-charset-edit.js | test-charset-edit.js::test_wrong_reply_charset
  EXCEPTION: a != b: 'UTF-8' != 'windows-1252'.
    at: test-folder-display-helpers.js line 2858
       assert_true test-folder-display-helpers.js:2858 11
       assert_equals test-folder-display-helpers.js:2845 3
       test_wrong_reply_charset test-charset-edit.js:113 3
       Runner.prototype.wrapper frame.js:585 9
       Runner.prototype._runTestModule frame.js:655 9
       Runner.prototype.runTestModule frame.js:701 3
       Runner.prototype.runTestFile frame.js:534 3
       runTestFile frame.js:713 3
       Bridge.prototype._execFunction server.js:179 10
       Bridge.prototype.execFunction server.js:183 16
       Session.prototype.receive server.js:283 3
       AsyncRead.prototype.onDataAvailable server.js:88 3
SUMMARY-UNEXPECTED-FAIL | c:\mozilla-source\comm-central\mail\test\mozmill\composition\test-charset-edit.js | test-charset-edit.js::test_no_mojibake
  EXCEPTION: a != b: 'x-mac-croatian' != 'utf-7'.
    at: test-folder-display-helpers.js line 2858
       assert_true test-folder-display-helpers.js:2858 11
       assert_equals test-folder-display-helpers.js:2845 3
       test_no_mojibake test-charset-edit.js:146 3
       Runner.prototype.wrapper frame.js:585 9
       Runner.prototype._runTestModule frame.js:655 9
       Runner.prototype.runTestModule frame.js:701 3
       Runner.prototype.runTestFile frame.js:534 3
       runTestFile frame.js:713 3
       Bridge.prototype._execFunction server.js:179 10
       Bridge.prototype.execFunction server.js:183 16
       Session.prototype.receive server.js:283 3
       AsyncRead.prototype.onDataAvailable server.js:88 3

We also note
[332] WARNING: NS_ENSURE_SUCCESS(childCV->SetForceCharacterSet(msgCharSet), NS_ERROR_FAILURE) failed with result 0x80070057: file c:/mozilla-source/comm-central/mailnews/compose/src/nsMsgCompose.cpp, line 1618

This was introduced in bug 1191841. Local debugging shows that the charset that fails to be set is x-mac-croatian.

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: Proposed solution (v1a).

Not a nice patch. But M-C broke us and we need to react.

Clean-up bug 1297118 already filed.

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 8784533 [details] [diff] [review]
Proposed solution (v1a).

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

Isn't this just hiding the error? I don't think we should hide it if it's real.

Comment 4

[Jorg K (CEST = GMT+2)]

No, the test that fails deliberately sets an invalid charset to check that this gets corrected somehow. With the changed behaviour of M-C this is not happening, since SetForceCharacterSet() now complains about bad input. So we are busted.

As the checkin comment says:
Ignore error returned by SetForceCharacterSet() to maintain *status quo*.

The whole thing should be reviewed in bug 1297118.

Comment 5

[Jorg K (CEST = GMT+2)]

Perhaps I should say it more clearly.

Before bug 1191841, SetForceCharacterSet() would accept any input and the HTML5 parser would crash later.
They changed that, and now it returns an error instead of crashing later.

Before we checked for an error that never came even with bad input. Ignoring that error now restores the exact previous state (status quo), but without the crash.

So I'd land this now to fix the test failure and the fact that TB actually now doesn't work and review the whole area later.

I hope the purists can wait for bug 1297118 ;-)

Comment 6

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+2, PTO during summer) from comment #5)
> ... and the fact that TB actually now doesn't work ...
Right now, you can't reply to a message with a bad character set since nsMsgCompose::InitEditor() returns with a bad status. I'm not sure what the final solution will look like, but not setting a bad charset, that is, ignoring the error for now, will get the default and that's what we want. That's the status quo.

Comment 7

[Jorg K (CEST = GMT+2)]

Attachment: UTF-7 test.eml

Right now, you can't answer this e-mail. With the patch, which restores the previous state, you can. Once again, this is meant as a bustage fix for the bustage caused by bug 1191841.

Comment 8

[Henri Sivonen (:hsivonen)]

FWIW, I intend to remove the x-mac-* encodings that aren't in the Encoding Standard in due course. Intent to Remove coming up after I've dealt with more pressing issues.

Comment 9

[Magnus Melin [:mkmelin]]

Comment on attachment 8784533 [details] [diff] [review]
Proposed solution (v1a).

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

Ok, yeah I guess this makes sense then. r=mkmelin

Comment 10

[Jorg K (CEST = GMT+2)]

Aleth, can you land this with your fix for the build oranges so we get a "completely" green tree, ha, ha.

Comment 11

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/d1ab8c5568261fa49914c23022f76806a73b6767
Bug 1297761 - Ignore error returned by SetForceCharacterSet() to maintain status quo. r=mkmelin

Comment 12

[Intermittent Failures Robot]

7 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* comm-central: 7

Platform breakdown:
* windowsxp: 2
* windows7-32: 2
* linux64: 2
* osx-10-10: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1297761&startday=2016-08-22&endday=2016-08-28&tree=all

Comment 13

[Kent James (:rkent)]

Comment on attachment 8784533 [details] [diff] [review]
Proposed solution (v1a).

[Approval Request Comment]
If we land the patch for bug 1191841 in m-esr45 THUNDERBIRD_45_VERBRANCH then presumably this issue will appear in TB 45, so we should land this too.

Comment 14

[Jorg K (CEST = GMT+2)]

Right!

Comment 15

[Kent James (:rkent)]

https://hg.mozilla.org/releases/comm-esr45/rev/af893d6214a0