Closed
Bug 999269
Opened 10 years ago
Closed 10 years ago
Forwarding a message without CC header inline gets an empty "CC:" line in header table
Categories
(MailNews Core :: Composition, defect)
MailNews Core
Composition
Tracking
(thunderbird29 wontfix, thunderbird30 fixed, thunderbird31 fixed, seamonkey2.25 unaffected, seamonkey2.26 fixed, seamonkey2.27 fixed, seamonkey2.28 fixed, thunderbird_esr24 unaffected)
RESOLVED
FIXED
Thunderbird 31.0
Tracking | Status | |
---|---|---|
thunderbird29 | --- | wontfix |
thunderbird30 | --- | fixed |
thunderbird31 | --- | fixed |
seamonkey2.25 | --- | unaffected |
seamonkey2.26 | --- | fixed |
seamonkey2.27 | --- | fixed |
seamonkey2.28 | --- | fixed |
thunderbird_esr24 | --- | unaffected |
People
(Reporter: rsx11m.pub, Assigned: rsx11m.pub)
References
(Blocks 1 open bug)
Details
(Keywords: regression)
Attachments
(1 file)
2.30 KB,
patch
|
jcranmer
:
review+
standard8
:
approval-comm-aurora+
standard8
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
Steps to reproduce: 1. Take a message in your Inbox or folder (IMAP or local, doesn't matter) 2. Verify that it doesn't have any "CC:" header set 3. Forward inline (as plain text or HTML, doesn't matter either) Expected result: 4. No "CC:" line in the header table of the message to be composed Actual result: 4. An empty "CC:" line in the header table of the message to be composed Observed with both today's TB 31.0a1 and SM 2.28a1 nightly builds, as well as a March SM 2.27a2 build on Windows 7.
Regression range (SM trunk on Windows): - last good build: 2.26a1 (Gecko 29.0a1) 20131211003521 0697d16f37df - first bad build: 2.26a1 (Gecko 29.0a1) 20131213003001 fb99ca2bed04 http://hg.mozilla.org/comm-central/graph/13377?revcount=15
Severity: normal → minor
status-seamonkey2.25:
--- → unaffected
status-seamonkey2.26:
--- → affected
status-seamonkey2.27:
--- → affected
status-seamonkey2.28:
--- → affected
status-thunderbird31:
--- → affected
Joshua, these are mostly patches from bug 842632 in the regression range. Does this ring any bell?
Flags: needinfo?(Pidgeot18)
Comment 3•10 years ago
|
||
I'm not seeing the issue locally, which makes it hard for me to say what's going on. Off-hand, from my recollection of relevant code, existence of the header is usually treated as a non-empty version of the field. I know my new jsmime code tends to be extremely intent on deleting whitespace wherever it can, although I also realized that I didn't introduce its use yet here (since the actual use-jsmime patches have yet to land). It's possible that the old code is not quite so whitespace-abhorent and you have something somewhere generating a " " for the Cc value instead of a null/empty string.
Flags: needinfo?(Pidgeot18)
Weird, I could reproduce this with two different profiles with no other STR than the ones specified. Note that only the "CC" heading is affected. Forwarding a message inline which doesn't have a "To" header still results only in an empty "CC:" but not an empty "To:" line being added. Thus, it doesn't appear to be systemic to address headers but specific to the "CC" one.
Thanks to Bozz who pointed me to a message which has a Cc heading set, and forwarding that one resulted in no "CC:" line added to the forward table. Thus, the logic is reversed. The problem lies in mimedrft.cpp line #890, which comm-central changeset fbac0bdddf0b changed 8.358 - if (to) 8.359 + if (!to.IsEmpty()) but then 8.366 - if (cc) 8.367 + if (cc.IsEmpty()) which is a reversed logic for the test in mime_insert_normal_headers(). Adding the '!' to the cc test corrects the logic.
This fixes the occurrence for the normal-header case I've noticed this bug for. There are also several instances in mime_insert_micro_headers() where I don't know what micro headers are, but I've corrected them as well while I'm there. Joshua, please double-check that this doesn't break anything else.
Attachment #8410364 -
Flags: review?(neil)
Attachment #8410364 -
Flags: feedback?(Pidgeot18)
Comment 7•10 years ago
|
||
Comment on attachment 8410364 [details] [diff] [review] Re-reverse reversed logic Review of attachment 8410364 [details] [diff] [review]: ----------------------------------------------------------------- Derpy edit fail is derpy.
Attachment #8410364 -
Flags: review?(neil)
Attachment #8410364 -
Flags: review+
Attachment #8410364 -
Flags: feedback?(Pidgeot18)
Thanks for the ultra-quick review, push for comm-central please.
Assignee: nobody → rsx11m.pub
Status: NEW → ASSIGNED
tracking-seamonkey2.26:
--- → ?
Keywords: checkin-needed
Comment on attachment 8410364 [details] [diff] [review] Re-reverse reversed logic [Approval Request Comment] Regression caused by (bug #): bug 842632 User impact if declined: erroneous behavior for headers when forwarding inline Testing completed (on c-c, etc.): I was planning to wait until this lands on c-c and passes all tests, but since the tree is currently "Approval required" and we should take this for the SM 2.26 release, I'll flag it this ahead of the push. Risk to taking this patch (and alternatives if risky): low IanN, Callek: You'll probably have to approve for comm-beta as Thunderbird's 29.0 beta was released already.
Attachment #8410364 -
Flags: approval-comm-beta?
Attachment #8410364 -
Flags: approval-comm-aurora?
Flags: needinfo?(iann_bugzilla)
Flags: needinfo?(bugspam.Callek)
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to rsx11m from comment #9) > Testing completed (on c-c, etc.): I was planning to wait until this lands on > c-c and passes all tests, Ok, I see that this thinking is fundamentally flawed - if there /were/ tests for the forward header table already, they should have caught this regression in the first place... ;-) Filed bug 1000147 on expanding the mozmill test respectively.
Comment 11•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/94cf07d93040
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [c-n: comm-central]
Target Milestone: --- → Thunderbird 31.0
Comment 12•10 years ago
|
||
Comment on attachment 8410364 [details] [diff] [review] Re-reverse reversed logic Note: this is landing post TB 29b1 (there won't be another 29b1 landing).
Attachment #8410364 -
Flags: approval-comm-beta?
Attachment #8410364 -
Flags: approval-comm-beta+
Attachment #8410364 -
Flags: approval-comm-aurora?
Attachment #8410364 -
Flags: approval-comm-aurora+
Comment 13•10 years ago
|
||
https://hg.mozilla.org/releases/comm-aurora/rev/24bc3bbd5c5b https://hg.mozilla.org/releases/comm-beta/rev/4fa1c7021190
Assignee | ||
Comment 14•10 years ago
|
||
Thanks Mark. Cancelling the SM-specific requests now that this has landed on the branches.
You need to log in
before you can comment on or make changes to this bug.
Description
•