Closed Bug 397021 Opened 14 years ago Closed 9 years ago
Subject line <foo@bar> in the forward inline body not shown in html composition mode
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:126.96.36.199) Gecko/20070911 Fedora/188.8.131.52-8.fc8 Firefox/184.108.40.206 Build Identifier: version 220.127.116.11 (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
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
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
Thanks for fixing this bug so quickly. :-) Patch attached.
(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
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.
(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.
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.
Is there no one willing to meet me halfway here and write a unit test for this bug? I'd really appreciate the help.
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
Here's a mozmill test for it, you'd run it using make SOLO_TEST=composition/test-forwarded-content.js mozmill-one
Thank you Magnus! Here is a single consolidated patch that incorporates my fix and Magnus's mozmill test. Review?
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!
Apparently "hg export -o" appends instead of overwriting. D'oh! Updated patch attached.
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+
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+.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0
Bah, you're supposed to use MsgEscapeHTML, as per line 1435... patch coming up.
Sorry, didn't know. Why do we have two functions that do exactly the same thing?
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 on attachment 610985 [details] [diff] [review] Use MsgEscapeHTML Pushed changeset b1b2d61638f5 to comm-central.
You need to log in before you can comment on or make changes to this bug.