610603 - mailnews.headers.showMessageId doesn't show the Message-ID unless you click on '+'

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Eric Moore]

User-Agent:       Mozilla/5.0 (Windows NT 6.0; rv:2.0b6) Gecko/20100101 Firefox/4.0b6
Build Identifier: 

If you set mailnews.headers.showMessageId true it displays "message-id + 1" (without the quotes) in the message view pane. Same problem if you view the message in a tab or window. If you click on the plus sign it replaces "+ 1" with a dash and the contents of the Message-ID: header. It is not a sticky setting, you have to click on the plus sign for every message. It is supposed to show the full header without you having to click on anything (there is only one value, and its length shouldn't be a problem). 

mailnews.headers.showUserAgent works correctly so I assume this problem is specific to mailnews.headers.showMessageId. I also tested using a Thunderbird 3.3a1pre ID:20101104032801 nightly build and it had the same problem.



Reproducible: Always

Comment 1

[Eric Moore]

The problem occurs with 3.1.6 but doesn't occur with 3.0.10. I don't have an earlier version of 3.1 handy but somebody in the MozillaZine forums complained that it worked fine until they upgraded to 3.1.6.

Comment 2

[rsx11m]

I can reproduce on trunk builds, and the problem appears to be that the Message-ID heading is treated as an array similar to the In-reply-to and References headers, thus only the indices are listed by default. In detail:

msgHdrViewOverlay.js#209 reads pref and defines OutputMessageIds to handle it;
that functions fills the messageIdArray, where i=0 for the Message-ID header;
processed in mailWidgets.xml#773, where updateMessageIdNode uses full or index notation depending on showFullMessageIds in the mail-messageids-headerfield binding, which in turn is initialized as "false" in the constructor.

Consequently, Message-ID: 1 is always shown in analogy to References: 1, 2, ...
Since there is always just a single Message-ID it should always be expanded.

Two options: (1) somehow realize in the fillMessageIdNodes method that this is a Message-ID header and expand by default if that's the case, or (2) whenever just a single ID is available for the header expand it (including references).

Comment 3

[rsx11m]

Attachment: Show a single ID in full

This implements variant (2) as explained in comment #2: the current "1,2,3" style is retained for any message-ID list which contains multiple identifiers; headers with a single identifier show it in full with the twisty disabled.

Comment 4

[rsx11m]

Attachment: Show always last ID in full

This is a suggestion for a variant (3) where always the last message-ID in a header is shown in full. Thus, it's the same as (2) for Message-ID, but with the additional benefit of also expanding the last ID in References headers. This allows to directly identify the last replied-to address (as it is fully expanded) and makes it easier to click on it.

Have both mailnews.headers.showMessageId and mailnews.headers.showReferences set for testing either of these patches.

Comment 5

[rsx11m]

Attachment: Show always last ID in full (v2)

... and <parameter>s are supposed to start with an 'a' in their name.
Other than that this patch corresponds to attachment 489939 [details] [diff] [review].

Comment 6

[rsx11m]

Attachment: Show always last ID in full (v3)

This is based on bug 611738 attachment 490496 [details] [diff] [review], awaiting check-in for
SeaMonkey. It is functionally identical to attachment 489946 [details] [diff] [review] here,
but with the review changes requested by Neil and IanN incorporated.

Personally, I'd prefer this patch over attachment 489923 [details] [diff] [review] as it also
makes the References header much better accessible.

Comment 7

[rsx11m]

Comment on attachment 491010 [details] [diff] [review]
Show always last ID in full (v3)

Neil, if this patch looks like the one you've just reviewed, that's correct.
I've talked to Blake (sub-module owner for Thunderbird), and since you have already reviewed the same patch for SeaMonkey (and this is still part of the mailWidgets.xml code before it was branched), he is fine if you sign off on
the identical one for Thunderbird as well.

I've double-checked that the same fix applies to the Thunderbird code and verified that it's fully functional in TB 3.3a1pre builds.

Comment 8

[Ludovic Hirlimann [:Usul]]

Bryan the patches have been r+ and are waiting for your UI review.

Comment 9

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 491010 [details] [diff] [review]
Show always last ID in full (v3)

lets go with this one

Comment 10

[rsx11m]

Thanks for the reviews, push on trunk please.

Comment 11

[rsx11m]

I didn't find any litmus/mozmill tests for either Message-ID or References headers, and I'm not even sure if this needs one, but just in case:

Steps to perform:
 1. Set mailnews.headers.showMessageId & mailnews.headers.showReferences "true"
 2. Select a message in a thread which is a reply to a reply (or deeper)
 3. Click twisty next to References to open the list
 4. Click twisty next to References to close the list

Expected results:
 2a. no twisty seen for Message ID
 2b. Message ID displayed in full <some@thing>
 2c. References shows 1, [2, 3], <expanded>
 3.  References shows all IDs expanded
 4.  References shows all as index number

I don't have time or resources right now to come up with a mozmill test, especially since it likely would have to cover bug 62033 in its entirety.
This may be stuff for a follow-up bug if you feel that it's needed.

Comment 12

[Ludovic Hirlimann [:Usul]]

(In reply to comment #11)
 
> I don't have time or resources right now to come up with a mozmill test,
> especially since it likely would have to cover bug 62033 in its entirety.
> This may be stuff for a follow-up bug if you feel that it's needed.

yes would be nice.

Comment 13

[rsx11m]

Ok, if Mark agrees I'll clone this bug for the tests once the patch is checked in. In the interim, feel free to create a litmus test from comment #11 until some more comprehensive testing is established (as necessary).

Comment 14

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/a93fd1e3b81e

Comment 15

[rsx11m]

Thanks, I've filed bug 613469 for the MozMill tests.

Comment 16

[Ludovic Hirlimann [:Usul]]

https://litmus.mozilla.org/show_test.cgi?id=13815

Comment 17

[rsx11m]

Looks good.