Closed
Bug 397021
Opened 16 years ago
Closed 12 years ago
Subject line <foo@bar> in the forward inline body not shown in html composition mode
Categories
(MailNews Core :: MIME, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 14.0
People
(Reporter: jik, Assigned: jik)
Details
Attachments
(2 files, 4 obsolete files)
8.10 KB,
patch
|
jik
:
review+
|
Details | Diff | Splinter Review |
1.37 KB,
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.6) Gecko/20070911 Fedora/2.0.0.6-8.fc8 Firefox/2.0.0.6 Build Identifier: version 2.0.0.6 (20070911) from Fedora Rawhide If I create a message containing subject"<foo@bar>" in the subject, send it to myself, and then try to forward it, the "<foo@bar>" is stripped from the subject. It's just wrong for thunderbird to be mucking with subjects like that. Reproducible: Always
Comment 1•16 years ago
|
||
I assume you mean in the body where the --- Original Message --- stuff is. Confirming on linux/trunk.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: PC → All
Summary: Subject line with <foo@bar> in it is mangled on forward → Subject line <foo@bar> in the forward inline body mangled
Comment 2•16 years ago
|
||
Or rather, it's just not shown (in html mode) as the < > are not encoded as < and > -- or so I assume as the plain text composition works fine.
Summary: Subject line <foo@bar> in the forward inline body mangled → Subject line <foo@bar> in the forward inline body not shown in html composition mode
Assignee | ||
Comment 3•13 years ago
|
||
Thanks for fixing this bug so quickly. :-) Patch attached.
Comment 4•13 years ago
|
||
(In reply to comment #3) > Created attachment 460147 [details] [diff] [review] > proposed fix > > Thanks for fixing this bug so quickly. :-) > Patch attached. You need to request reviews if you want you code to get in see https://developer.mozilla.org/en/Mailnews_and_Mail_code_review_requirements.
Component: Mail Window Front End → MIME
Product: Thunderbird → MailNews Core
QA Contact: front-end → mime
Updated•13 years ago
|
Attachment #460147 -
Flags: superreview?(bugmail)
Attachment #460147 -
Flags: review?(bienvenu)
Comment 5•13 years ago
|
||
Comment on attachment 460147 [details] [diff] [review] proposed fix Ludo, I'm not a super-reviewer although I've heard talk of trying to make that happen. But I can chime in here as a reviewer and say that this definitely needs at least a unit test confirming the change in behaviour. If there are not already unit tests covering the cases where the removed/modified logic was supposed to be doing something helpful, test cases should probably be added for that too. (Please call out the existing tests in a buzilla comment if they already exist.) Sorta helpful testing link: https://developer.mozilla.org/en/MailNews_xpcshell-tests bienvenu is likely the right person to ask for review from once tests are created or if help is required in creating tests. I don't think super-review is required for such a patch, but if it is, bienvenu can give it.
Attachment #460147 -
Flags: superreview?(bugmail)
Attachment #460147 -
Flags: review?(bienvenu)
Attachment #460147 -
Flags: review-
Comment 6•13 years ago
|
||
(In reply to comment #3) > Created attachment 460147 [details] [diff] [review] > proposed fix > > Thanks for fixing this bug so quickly. :-) > Patch attached. Jonathan , you'll need to add some unit test (I forgot to look for these). If you need help with these please ask either here or on our mailing-lists or in #maildev on irc.mozilla.org.
Assignee | ||
Comment 7•13 years ago
|
||
The learning curve for figuring out how to debug and fix Thunderbird issues was steep enough. Unfortunately, I do not have the additional time that would be necessary to get over the undoubtedly steep learning curve that would also be necessary to write a unit test for something like this. One would hope that if a user of the software gave the time and energy to submit a fix, the developers of the software would meet the user halfway and write the unit test. Especially given that this bug has sat unfixed for almost three years. In short, can I please get a little help here? I've owned free software projects for much longer than Mozilla has existed, and I've never asked a user who contributed a fix to write a unit test for it.
Assignee | ||
Comment 8•13 years ago
|
||
Is there no one willing to meet me halfway here and write a unit test for this bug? I'd really appreciate the help.
Updated•13 years ago
|
Flags: in-litmus?
Assignee | ||
Comment 9•12 years ago
|
||
Since posting comment #7 and comment #8, I've learned how to build and patch TB and FF and how to work with the unit tests for both, so I'll write my own unit test for this bug when I get a chance. That may be a while, though -- too many all-nighters fixing other TB bugs have left me with a work deficit I need to address -- so if anyone else stumbles across this bug and wants to pitch in by writing the unit test, I wouldn't say no.
Assignee: nobody → jik
Status: NEW → ASSIGNED
Comment 10•12 years ago
|
||
Here's a mozmill test for it, you'd run it using make SOLO_TEST=composition/test-forwarded-content.js mozmill-one
Assignee | ||
Comment 11•12 years ago
|
||
Thank you Magnus! Here is a single consolidated patch that incorporates my fix and Magnus's mozmill test. Review?
Attachment #460147 -
Attachment is obsolete: true
Attachment #606859 -
Attachment is obsolete: true
Attachment #609755 -
Flags: review?(dbienvenu)
Comment 12•12 years ago
|
||
Comment on attachment 609755 [details] [diff] [review] Consolidated patch thx for the patch! The mime parts of the patch seem to have bit-rotted - can you attach a de-bitrotted patch and re-request review? thx!
Attachment #609755 -
Flags: review?(dbienvenu)
Assignee | ||
Comment 13•12 years ago
|
||
Apparently "hg export -o" appends instead of overwriting. D'oh! Updated patch attached.
Attachment #609755 -
Attachment is obsolete: true
Attachment #610139 -
Flags: review?(dbienvenu)
Comment 14•12 years ago
|
||
Comment on attachment 610139 [details] [diff] [review] Consolidated patch without repetition thx for the patch, looks reasonable - can you use nsnull instead of NULL?
Attachment #610139 -
Flags: review?(dbienvenu) → review+
Assignee | ||
Comment 15•12 years ago
|
||
Same as attachment 610139 [details] [diff] [review] which is already review+. The only change is to replace NULL with nsnull as requested by the reviewer. I've confirmed that it builds with the nsnull change, so I'm carrying forward the review+.
Attachment #610139 -
Attachment is obsolete: true
Attachment #610180 -
Flags: review+
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 16•12 years ago
|
||
http://hg.mozilla.org/comm-central/rev/30c2149ba887
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0
Comment 17•12 years ago
|
||
Bah, you're supposed to use MsgEscapeHTML, as per line 1435... patch coming up.
Comment 18•12 years ago
|
||
Attachment #610985 -
Flags: review?(dbienvenu)
Assignee | ||
Comment 19•12 years ago
|
||
Sorry, didn't know. Why do we have two functions that do exactly the same thing?
Comment 20•12 years ago
|
||
Comment on attachment 610985 [details] [diff] [review] Use MsgEscapeHTML Neil's talking about supporting frozen linkage, which is a different way of building where only some interfaces are supported.
Attachment #610985 -
Flags: review?(dbienvenu) → review+
Comment 21•12 years ago
|
||
Comment on attachment 610985 [details] [diff] [review] Use MsgEscapeHTML Pushed changeset b1b2d61638f5 to comm-central.
Updated•8 years ago
|
Flags: in-litmus?
You need to log in
before you can comment on or make changes to this bug.
Description
•