1710581 - Fix test_createRFC822Message.js to make sure non-UTF-8 binary strings are transported correctly

Status: RESOLVED FIXED

(MailNews Core :: General, enhancement)

Description

[Tom]

From bug 1710043 comment #12:

test_createRFC822Message.js needs an overhaul, IOUtils.readUTF8 doesn't appear to be correct for reading back arbitrary data.

Comment 1

[Ping Chen [:rnons] (away)]

Can you send a link to IOUtils.readUTF8 you're referring to? I changed to IOUtils.read a few days ago

Comment 2

[Tom]

https://searchfox.org/comm-central/rev/9b70387c49576c721e8a1d7a1945aa18782b6d31/mailnews/compose/test/unit/test_createRFC822Message.js#20

This one didn't get changed, and the message body you're transporting should contain some arbitrary binary data to make sure there is no UTF-8 assumption anywhere in the code path.

Comment 3

[Tom]

Attachment: 1710581.patch

Comment 4

[Ping Chen [:rnons] (away)]

Comment on attachment 9221305 [details] [diff] [review]
1710581.patch

Looks good, thanks. Please add a timestamp to the patch. I often use hg export tip > 1710581.patch to generate one.

Comment 5

[Tom]

Attachment: 1710581.patch

Thanks for the review, now with timestamp.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e1946904f4d2
enhance test_createRFC822Message.js with some binary data. r=rnons

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2441ae8a4ce1
followup lint fix. rs=eslint DONTBUILD

Comment 8

[Rob Lemley [:rjl]]

Comment on attachment 9221516 [details] [diff] [review]
1710581.patch

[Approval Request Comment]
Test-only code, low risk.
This bug updates test code with respect to the changes in bug 1710043 and should be uplifted at the same time.

Comment 9

[Tom]

This bug updates test code with respect to the changes in bug 1710043 and should be uplifted at the same time.

Thanks for requesting the "uplift", but it's really not necessary. Bug 1710043 is rather important, HTML "downgrade" to plain text can have some surprising results. Do you still want it on the beta or should we clear the request?

Comment 10

[Wayne Mery (:wsmwk)]

Comment on attachment 9221516 [details] [diff] [review]
1710581.patch

[Triage Comment]
Sorry, we missed doing the next 89 beta, so this will appear in beta 90