The default bug view has changed. See this FAQ.

(editablefrom) Make From header editable: "edit draft" and "edit as new message" not working for RFC 2047 encoded From.

RESOLVED FIXED in Thunderbird 46.0

Status

MailNews Core
Composition
RESOLVED FIXED
a year ago
a year ago

People

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

Tracking

(Depends on: 1 bug)

Thunderbird 46.0

Thunderbird Tracking Flags

(thunderbird44 unaffected, thunderbird45+ fixed, thunderbird46 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

a year ago
Fix problems described in bug 87987 comment #202:

Edit As New Message and edit draft showing a raw header:
=?UTF-8?Q?J=c3=b6rg_Knobloch?= <jorgk@jorgk.com>
and the user is placed into "Customize From Address" mode as a consequence.

There are CSS issues: Text cut off at baseline.

See attachment 8698483 [details] for both issues.
(Assignee)

Updated

a year ago
Assignee: nobody → neil
Filed bug 1232753 for the CSS fix as a followup of bug 1159316.

With this split this bug can be used for the encoding issue.
(Assignee)

Updated

a year ago
Summary: (editablefrom) Make From header editable: Part 2: Fix CSS, fix "edit draft" and "edit as new message" → (editablefrom) Make From header editable: Part 2: Fix "edit draft" and "edit as new message"
(Assignee)

Updated

a year ago
tracking-thunderbird45: --- → ?
What's the decoded header in this case?
(Assignee)

Comment 3

a year ago
As I said in bug 87987 comment #202:
I see this in the From field:
=?UTF-8?Q?J=c3=b6rg_Knobloch?= <jorgk@jorgk.com>

if I "edit as new message" a message that contained any of these headers:
From: =?ISO-8859-1?Q?J=F6rg_Knobloch?= <jorgk@jorgk.com>
From: =?UTF-8?Q?J=c3=b6rg_Knobloch?= <jorgk@jorgk.com>
which are two different encodings of "Jörg Knobloch".

You can also test with this header:
From: "=?UTF-8?Q?Maria_del_Mar_Sol=C3=A9?= mariadelmarsole@yahoo.com [Barcelona-Freecycle]" <Barcelona-Freecycle-noreply@yahoogroups.com>

which shows as
=?UTF-8?Q?Maria_del_Mar_Sol=c3=a9_mariadelmarsole@yahoo.com_ [Barcelona?= =?UTF-8?Q?-Freecycle] ?= <Barcelona-Freecycle-noreply@yahoogroups.com>

if you edit the message as new, but should show as:
Maria del Mar Solé mariadelmarsole@yahoo.com [Barcelona-Freecycle] <Barcelona-Freecycle-noreply@yahoogroups.com>

Another one:
Header:
From: "=?UTF-8?Q?=22Bettina_R=C3=B6hricht=22?= Bettina_Roehricht@web.de [freecycle-berlin]" <freecycle-berlin-noreply@yahoogroups.de>

Shows as:
=?UTF-8?Q?=22Bettina_R=c3=b6hricht=22_Bettina=5fRoehricht@web.de_ [free?= =?UTF-8?Q?cycle-berlin] ?= <freecycle-berlin-noreply@yahoogroups.de>

Should be:
"Bettina Röhricht" Bettina_Roehricht@web.de [freecycle-berlin] <freecycle-berlin-noreply@yahoogroups.de>

(I have a whole bag of these one, since I worked on JSMime regressions.)
(Assignee)

Comment 4

a year ago
Created attachment 8698896 [details] [diff] [review]
Proposed fix (v1).

You're using the wrong function, see:
https://dxr.mozilla.org/comm-central/source/mailnews/mime/public/nsIMsgHeaderParser.idl#106

Not sure what the join() was for.
Attachment #8698896 - Flags: review?(neil)
(Assignee)

Updated

a year ago
Summary: (editablefrom) Make From header editable: Part 2: Fix "edit draft" and "edit as new message" → (editablefrom) Make From header editable: "edit draft" and "edit as new message" not working for RFC 2047 encoded From.
Comment on attachment 8698896 [details] [diff] [review]
Proposed fix (v1).

>+    let from = MailServices.headerParser.parseEncodedHeader(params.composeFields.from, undefined, false);
undefined is not a string. Did you mean null?

false is the default, was there a reason to explicitly specify it?

(In reply to Jorg K (GMT+1) from comment #4)
> Not sure what the join() was for.
Because both functions return arrays.
Attachment #8698896 - Flags: review?(neil) → review-
(Assignee)

Comment 6

a year ago
Created attachment 8698977 [details] [diff] [review]
Proposed fix (v2).

OK, I stuck a [0] at the end. Not sure why join(", ") was a good idea. Do we expect more than one sender?

Otherwise I copied it form here:
https://dxr.mozilla.org/comm-central/source/mailnews/mime/src/mimeJSComponents.js#390

Let's not argue about a one line change, if you have a better line, please submit it.
Attachment #8698977 - Flags: review?(neil)
(Assignee)

Updated

a year ago
Attachment #8698896 - Attachment is obsolete: true
(In reply to Jorg K from comment #6)
> Not sure why join(", ") was a good idea. Do we expect more than one sender?
I'm sure I originally used [0] too, but e.g. in bug 87978 comment 171, Magnus expected it, so I had to add it in...
(Assignee)

Comment 8

a year ago
Created attachment 8699020 [details] [diff] [review]
Proposed fix (v3). [checked in - comment 12, TB 46][checked in - comment 13, TB 45 Aurora]

Third time lucky? ;-)
Attachment #8699020 - Flags: review?(neil)
(Assignee)

Updated

a year ago
Attachment #8698977 - Attachment is obsolete: true
Attachment #8698977 - Flags: review?(neil)

Updated

a year ago
Attachment #8699020 - Flags: review?(neil) → review+
(Assignee)

Comment 9

a year ago
Comment on attachment 8699020 [details] [diff] [review]
Proposed fix (v3). [checked in - comment 12, TB 46][checked in - comment 13, TB 45 Aurora]

[Approval Request Comment]
Regression caused by (bug #): 87987
User impact if declined: High, since the function implemented in bug 87987 doesn't work for RFC 2047 encoded names, so an estimated 50% of the user base.
Testing completed (on c-c, etc.): Aceman was going to add tests in bug 87987.
Risk to taking this patch (and alternatives if risky): N/A.

Aceman, are these tests coming?
Flags: needinfo?(acelists)
Attachment #8699020 - Flags: approval-comm-aurora?
(Assignee)

Updated

a year ago
Assignee: neil → mozilla
Status: NEW → ASSIGNED
Keywords: checkin-needed

Updated

a year ago
status-thunderbird44: --- → unaffected
status-thunderbird45: --- → affected
tracking-thunderbird45: ? → +

Comment 10

a year ago
(In reply to Jorg K (GMT+1) from comment #9)
> Aceman, are these tests coming?

Yes, when the tests in bug 87987 land, I can add a test for the bug here. Is it needed to open a draft with the special From name, or does the bug also show when just setting up an identity with special characters and then composing a message?

> Testing completed (on c-c, etc.): Aceman was going to add tests in bug 87987.

I don't think this question from bugzilla really requires there to be automated tests. Only asks where/how it was tested.
Flags: needinfo?(acelists)
(Assignee)

Comment 11

a year ago
(In reply to :aceman from comment #10)
> Yes, when the tests in bug 87987 land, I can add a test for the bug here. Is
> it needed to open a draft with the special From name, or does the bug also
> show when just setting up an identity with special characters and then
> composing a message?

Well, my name contains an "ö" and all was well until I opened a saved draft or used a sent message and "edited message as new". Since the From header parsing used the wrong call (see patch), the From field was populated with a string that didn't match any identity.

So yes, I'm afraid you need to open/edit a saved draft. Thanks for looking into it!

Comment 12

a year ago
https://hg.mozilla.org/comm-central/rev/814a1eab9cffc0853c8bdb868ed998f5bf88b2ae
Bug 1232735 - Use parseEncodedHeader instead of parseDecodedHeader. r=neil

Updated

a year ago
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 46.0

Updated

a year ago
Attachment #8699020 - Flags: approval-comm-aurora? → approval-comm-aurora+
(Assignee)

Comment 13

a year ago
https://hg.mozilla.org/releases/comm-aurora/rev/55fad8a3414e
(Assignee)

Updated

a year ago
Attachment #8699020 - Attachment description: Proposed fix (v3). → Proposed fix (v3). [checked in - comment 12, TB 46][checked in - comment 13, TB 45 Aurora]
(Assignee)

Updated

a year ago
status-thunderbird45: affected → fixed
status-thunderbird46: --- → fixed

Updated

a year ago
Depends on: 1238264
You need to log in before you can comment on or make changes to this bug.