Closed
Bug 1232735
Opened 9 years ago
Closed 8 years ago
(editablefrom) Make From header editable: "edit draft" and "edit as new message" not working for RFC 2047 encoded From.
Categories
(MailNews Core :: Composition, defect)
Tracking
(thunderbird44 unaffected, thunderbird45+ fixed, thunderbird46 fixed)
RESOLVED
FIXED
Thunderbird 46.0
Tracking | Status | |
---|---|---|
thunderbird44 | --- | unaffected |
thunderbird45 | + | fixed |
thunderbird46 | --- | fixed |
People
(Reporter: jorgk-bmo, Assigned: jorgk-bmo)
References
(Depends on 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
1.11 KB,
patch
|
neil
:
review+
mkmelin
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
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•9 years ago
|
Assignee: nobody → neil
Comment 1•9 years ago
|
||
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•9 years 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•9 years ago
|
tracking-thunderbird45:
--- → ?
Comment 2•9 years ago
|
||
What's the decoded header in this case?
Assignee | ||
Comment 3•9 years 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•9 years ago
|
||
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•9 years 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 5•9 years ago
|
||
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•9 years ago
|
||
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•9 years ago
|
Attachment #8698896 -
Attachment is obsolete: true
Comment 7•9 years ago
|
||
(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 | ||
Updated•9 years ago
|
Attachment #8698977 -
Attachment is obsolete: true
Attachment #8698977 -
Flags: review?(neil)
Updated•9 years ago
|
Attachment #8699020 -
Flags: review?(neil) → review+
Assignee | ||
Comment 9•9 years 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•9 years ago
|
Updated•8 years ago
|
status-thunderbird44:
--- → unaffected
status-thunderbird45:
--- → affected
Comment 10•8 years 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•8 years 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•8 years ago
|
||
https://hg.mozilla.org/comm-central/rev/814a1eab9cffc0853c8bdb868ed998f5bf88b2ae Bug 1232735 - Use parseEncodedHeader instead of parseDecodedHeader. r=neil
Updated•8 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 46.0
Updated•8 years ago
|
Attachment #8699020 -
Flags: approval-comm-aurora? → approval-comm-aurora+
Assignee | ||
Comment 13•8 years ago
|
||
https://hg.mozilla.org/releases/comm-aurora/rev/55fad8a3414e
Assignee | ||
Updated•8 years 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•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•