473903 - View All Headers mode displays some headers in wrong order

Status: NEW

(Thunderbird :: Mail Window Front End, defect)

Description

[Alan Wotiz]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090115 Shredder/3.0b2pre

When first selecting View All Headers mode, the order that additional headers are shown is correct only for the first message displayed. When selecting another message, headers are shown in the same order as they were in the first message. Exiting then re-entering View All Headers mode re-establishes the correct order for the displayed message.

Reproducible: Always

Steps to Reproduce:
1. Display an incoming message in Normal Headers mode
2. Select View All Headers mode
3. Headers are shown correctly
4. Display a different incoming message
5. Some headers are not shown in the actual order that they are in the message
6. Set View Normal Headers, then back to View All Headers
7. Headers are now shown in the correct order
Actual Results:  
(see above)

Expected Results:  
Headers should always be shown in the same order as they appear in View Message Source.

This bug comes from the re-use of the DOM nodes that are created when entering View All Headers mode. The only way to correct this would be to remove and recreate the header DOM nodes under 'variousHeadersBox' for each message displayed while in that mode. This will cause a slight but noticeable delay when displaying the headers, so it's really an issue of accuracy vs. performance.

Comment 1

[Alan Wotiz]

Attachment: Proposed fix

This is one possible fix that I've tested. I'm afraid it's just a manual cut-and-paste since I don't have any proper diff tools.

Comment 2

[Michael]

I am unable to reproduce this in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b5pre) Gecko/20090508 Lightning/1.0pre Shredder/3.0b3pre ID:20090508040335 on Windows XP SP3. Can you please check to see if it is still an issue in one of the latest nightly builds?

Comment 3

[Alan Wotiz]

The code that causes this bug hasn't changed since I last saw it occur, but I'll take another look and see if it's still reproducible.

Comment 4

[Alan Wotiz]

I'm still seeing this in the current nightly - Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090602 Shredder/3.0b3pre, but it's a bit difficult to see due to bug 496046 which also affects the header order.

Comment 5

[Ludovic Hirlimann [:Usul]]

Alan can you save one of those message that display the issue in the form of an .eml file and attach it to this bug please ?

Comment 6

[Alan Wotiz]

Attachment: mailbox file with two test messages

The bug won't show up with an .eml file, since it only happens when you switch between two (or more) messages without reloading the message window. Instead, here's a mailbox file you can drop into the Local Folders directory.

To reproduce with this file:

1. select Normal Headers mode
2. display the first message in the file
3. select All Headers mode
4. display the second message in the file

You'll see two Received headers near the top, followed by a MIME-Version and a Content-Type, then later on a bunch more Received headers. (This doesn't match the original message.)

5. select Normal Headers mode
6. select All Headers mode again

Now you'll see the Content-Type and MIME-Version headers at the end, after all the Received headers. (This matches the original message, can use View Source to check.)

Comment 7

[Wayne Mery (:wsmwk)]

Alan, does your draft patch still fix the issue?

Comment 8

[Alan Wotiz]

Attachment: Updated patch

I've updated the patch to work with the current trunk. But beware: I'm not currently running TB3, so I've only been able to test the patch with an older TB2 that's using an equivalent version of the surrounding code. Someone would need to make sure this works OK with TB3, though I'm 99% sure it will.

Also, there is a noticeable added delay when displaying messages in All Headers mode, so this should probably get some type of before-and-after comparison UI review before deciding whether to approve it.

The delay is from having to delete then recreate all the DOM nodes when displaying each message. The "right" way to fix this would be to have some number of nonspecific fixed nodes that are reused each time (similar to the cache of address nodes used for the "To" field), but doing that is way beyond my level of expertise.

Comment 9

[Ludovic Hirlimann [:Usul]]

Comment on attachment 506971 [details] [diff] [review]
Updated patch

so this a patch against trunk right ?

We would probably need to add some test. The documentation is available at : https://developer.mozilla.org/en/Thunderbird/Thunderbird_MozMill_Testing

Comment 10

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 506971 [details] [diff] [review]
Updated patch

I have no idea what to do with this patch.  I mean, I can see how it fixes the bug, but the added delay isn't good and the patch needs tests.  I think you're right about the correct fix, Alan, and if you want to try to write up a patch that does that, I would be more than happy to lend you a hand either here or on IRC.

So I'm going to give this an r-, in the hopes that we can get you to write a better patch.  ;)

Thanks,
Blake.

Comment 11

[Alan Wotiz]

Unfortunately, it doesn't look like I'll have time to look at this any further. I'm setting the bug to unassigned, hopefully someone else might take this up.

Blake - I think you made the right decision; I wasn't entirely comfortable with the patch (due to the performance issue) even though it did fix the problem.