Forwarding test message mail/test/mozmill/composition/multipart-charset.eml as attachment gives deprecation warning

RESOLVED FIXED in Thunderbird 53.0

Status

MailNews Core
MIME
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: Jorg K (GMT+2), Assigned: Jorg K (GMT+2))

Tracking

Trunk
Thunderbird 53.0

Thunderbird Tracking Flags

(thunderbird_esr4551+ fixed, thunderbird51 fixed, thunderbird52 fixed, thunderbird53 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 months ago
Forward test message mail/test/mozmill/composition/multipart-charset.eml as attachment and see this in the error console:

DEPRECATION WARNING: Encoding to non-UTF-8 values is obsolete
You may find more details about this deprecation at: http://bugzilla.mozilla.org/show_bug.cgi?id=790855
resource://gre/components/mimeJSComponents.js 443 encodeMimePartIIStr_UTF8
  Deprecated.jsm:79
	warning resource://gre/modules/Deprecated.jsm:79:5
	encodeMimePartIIStr_UTF8 resource://gre/components/mimeJSComponents.js:443:7
(Assignee)

Comment 1

5 months ago
The warning comes from EncodeMimePartIIStr_UTF8() which has three callers:
https://dxr.mozilla.org/comm-central/search?q=EncodeMimePartIIStr_UTF8&redirect=false

Breaking at the relevant call site shows this stack:
xul.dll!nsMsgI18NEncodeMimePartIIStr() Line 239	C++
xul.dll!LegacyParmFolding() Line 1116	C++
xul.dll!mime_generate_attachment_headers() Line 725	C++
xul.dll!nsMsgComposeAndSend::PreProcessPart() Line 1093	C++
xul.dll!nsMsgComposeAndSend::GatherMimeAttachments() Line 888	C++
xul.dll!nsMsgAttachmentHandler::UrlExit() Line 1332	C++

Since the test message is in EUC-KR, this is what is passed in and then the encoder complains.

The problem is that mime_generate_attachment_headers() tries to encode the attachment file name in the charset of the body.

The warning can be ignored, however, it's ugly, so patch is coming ;-)
(Assignee)

Comment 2

5 months ago
Created attachment 8819935 [details] [diff] [review]
1324443-attachment-file-names-utf-8.patch

Aceman, can you please rs this for me. It's just shutting up a confusing warning.

The test case is very clear:
Forward our test mail mail/test/mozmill/composition/multipart-charset.eml as attachment.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8819935 - Flags: review?(acelists)

Comment 3

5 months ago
Comment on attachment 8819935 [details] [diff] [review]
1324443-attachment-file-names-utf-8.patch

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

Ok, you probably do not make it worse.
But why can we encode the name to utf-8 when the message is in something else? Does some standard allow that? That the mime parser decided to only produce utf-8 does not mean we have to use it for everything.
Attachment #8819935 - Flags: review?(acelists)
Attachment #8819935 - Flags: review+
Attachment #8819935 - Flags: feedback?(Pidgeot18)
(Assignee)

Comment 4

5 months ago
(When will people understand that Joshua doesn't really work here any more?)

I don't know a whole lot about this code, but here we're talking about encoding the attachment name in the attachment headers, nothing else. So if you attach "Jörg'sFílè.txt" you get the name encoded in UTF-8 in the attachment header:
Content-Type: text/plain;
 name="=?UTF-8?B?SsO2cmcnc0bDrWzDqC50eHQ=?="
The message body can of course be in a different encoding. This is purely about the encoding of the file name.

Given that the code that issues the deprecation warning ignores the charset passed in and just proceeds, it's a good idea to fix the caller to suppress this warning.
(Assignee)

Comment 5

5 months ago
Best to land this after the next M-C merge.
Keywords: checkin-needed

Comment 6

5 months ago
(In reply to Jorg K (GMT+1) from comment #4)
>  name="=?UTF-8?B?SsO2cmcnc0bDrWzDqC50eHQ=?="
> The message body can of course be in a different encoding. This is purely
> about the encoding of the file name.

OK, thanks, that's the answer. You can use UTF-8 because you include the charset specification in the name directly.
(Assignee)

Comment 7

5 months ago
https://hg.mozilla.org/comm-central/rev/9c1291d2e1f7521c2558e8c76f37a6b724c9be32
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 53.0
(Assignee)

Updated

5 months ago
Attachment #8819935 - Flags: feedback?(Pidgeot18)
(Assignee)

Updated

5 months ago
Keywords: checkin-needed
(Assignee)

Comment 8

5 months ago
Comment on attachment 8819935 [details] [diff] [review]
1324443-attachment-file-names-utf-8.patch

I'll uplift this simple no-risk patch to get rid of the frightening warning.
Attachment #8819935 - Flags: approval-comm-beta+
Attachment #8819935 - Flags: approval-comm-aurora+
(Assignee)

Comment 9

5 months ago
Aurora (TB 52):
https://hg.mozilla.org/releases/comm-aurora/rev/c2d950d166ab0a562d869c5dbaee79e3f14977a5
status-thunderbird51: --- → affected
status-thunderbird52: --- → fixed
status-thunderbird53: --- → fixed
status-thunderbird_esr45: --- → affected
tracking-thunderbird_esr45: --- → ?
(Assignee)

Comment 10

5 months ago
Comment on attachment 8819935 [details] [diff] [review]
1324443-attachment-file-names-utf-8.patch

[Approval Request Comment]
Regression caused by (bug #): JS Mime and friends, here bug 790855
User impact if declined: Frightening message displayed in error console. 
Testing completed (on c-c, etc.): Manual.
Risk to taking this patch (and alternatives if risky):
No risk. Just passing fixed "UTF-8" charset where the caller ignores the charset anyway but complains about non-UTF-8 being passed in with a terrible message.
Attachment #8819935 - Flags: approval-comm-esr45?
(Assignee)

Comment 11

5 months ago
Beta (TB 51):
https://hg.mozilla.org/releases/comm-beta/rev/3747ae2ff8d3f25fab0528217aefbbe43764f892
status-thunderbird51: affected → fixed

Comment 12

4 months ago
Comment on attachment 8819935 [details] [diff] [review]
1324443-attachment-file-names-utf-8.patch

https://hg.mozilla.org/releases/comm-esr45/rev/45af1574facc
Attachment #8819935 - Flags: approval-comm-esr45? → approval-comm-esr45+

Updated

4 months ago
status-thunderbird_esr45: affected → fixed
tracking-thunderbird_esr45: ? → 51+
You need to log in before you can comment on or make changes to this bug.