496046 - Message headers are processed in reverse order

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Alan Wotiz]

Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090602 Shredder/3.0b3pre

The headers are processed in the reverse order that they appear in the message (last to first). This looks like it may be caused by the patch in bug 474759 which changed the header field enumerator (nsMimeStringEnumerator::GetNext in nsMimeHtmlEmitter.cpp) to pick off the fields from the end of the array rather than the beginning. I find this problem first appears somewhere between the 20090120 and 20090124 nightlies, which is when that patch was checked in.

This bug causes two different problems, depending on the header display mode:

A. Normal Headers mode
If a particular header appears more than once in a message, the one that appears last will be displayed, instead of the one that appears first.

Steps to Reproduce:
1. Set the mailnews.headers.extraExpandedHeaders pref to include a header that appears more than once in a particular message
2. Exit and restart shredder (necessary to pick up the changed pref)
3. Display the message selected above
4. Display expanded headers for the message

Actual Results:
Last-appearing header matching the extraExpandedHeaders pref is displayed.

Expected Results:
First-appearing header should be displayed.


B. View All Headers mode
All headers are displayed in reverse order.

Steps to Reproduce:
1. Select View All Headers mode
2. Display a message
3. Select View Message Source

Actual Results:
Headers in message window are in reverse order from those in the source window.

Expected Results:
Headers should be in the same order.

Comment 1

[[e-mail]]

Attachment: Proposed patch

I decided to spend 4 bytes on not shifting elements in the array.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

Should the patch add some documentation somewhere to define what our behavior is?  When reading this bug, I thought it's not entirely clear why we should traverse in one direction over the other.  Or if not documentation, a unit test?

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 381362 [details] [diff] [review]
Proposed patch

r/sr=Standard8 as long as you add documentation as to why we should process them in a specific order.

A unit test would also be nice.

Comment 4

[[e-mail]]

Pushed changeset 7075e80889c6 to comm-central.

Comment 5

[WADA:World Anti-bad-Duping Agency]

Neil, can this bug explain phenomenon of bug 435286 Comment #1?
Will problem of bug 435286 be fixed by fix of this bug?

Comment 6

[[e-mail]]

(In reply to comment #5)
> Neil, can this bug explain phenomenon of bug 435286 Comment #1?
> Will problem of bug 435286 be fixed by fix of this bug?
I doubt it, but I'll CC someone who might know to that bug.