Expand single or last message ID in headers display by default

RESOLVED FIXED in seamonkey2.1b2

Status

SeaMonkey
MailNews: Message Display
--
minor
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: rsx11m, Assigned: rsx11m)

Tracking

Trunk
seamonkey2.1b2

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
This is the SeaMonkey equivalent of Thunderbird bug 610603, which was initially filed for the Message-ID header only. Apparently as a consequence of bug 62033, not just the References but also the Message-ID header are opened by default "closed" with only the index numbers:

> Message-ID: 1
> References: 1, 2, 3

with mailnews.headers.showMessageId and mailnews.headers.showReferences set. The Message-ID header isn't of much use in this way (that's the motivation for bug 610603), while it makes sense for the References header, especially in long threads with many replies. Since those are effectively links, it is possible to trace back the replies within a thread. However, it may not be obvious which one is the  message replied to (i.e., the last one in the referenced chain), and it's hard to hit the tiny number to follow up to the replied-to message.

The proposal is to always expand a single message ID (there is no ambiguity here and sufficient space in a single line), and to expand the last (i.e., the most recent) message ID in a referenced reply chain:

> Message-ID: <tests@example.com>
> References: 1, 2, <replied-to@example.com>

Expanded:

> References: <first@example.com>, <follow-up@example.com>, <replied-to@example.com>
(Assignee)

Comment 1

7 years ago
Created attachment 490147 [details] [diff] [review]
Proposed patch

This patch expands a single or the last message ID when opening a message,
thus resolving the issues mentioned in the bug description. In addition, the open/close twisty is hidden when just a single message ID is present.

Note that "toggleWrap" remains unpatched. Thus, upon closing after opening,
only the index numbers (1, 2, 3) are shown, assuming that this reflects the intention of the user by this action.
Assignee: nobody → rsx11m.pub
Status: NEW → ASSIGNED
Attachment #490147 - Flags: superreview?(neil)
Attachment #490147 - Flags: review?(iann_bugzilla)

Comment 2

7 years ago
Comment on attachment 490147 [details] [diff] [review]
Proposed patch

>+            if (numMessageIds > 1)
>+              this.toggleIcon.removeAttribute("hidden");
>+            else
>+              this.toggleIcon.setAttribute("hidden", "true");
Nit: this.toggleIcon.hidden = numMessageIds <= 1;

>+                this.updateMessageIdNode(messageIdNodes[index * 2], index + 1,
>+                  this.mMessageIds[index], index == numMessageIds - 1);
...
>+                this.updateMessageIdNode(itemInDocument, index + 1,
>+                  this.mMessageIds[index], index == numMessageIds - 1);
[Annoying, but not easily fixed without a rewrite.]
Attachment #490147 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 3

7 years ago
Created attachment 490460 [details] [diff] [review]
Proposed patch (v2)

Updated with Neil's comment addressed (yes, that's shorter = saves 3 lines).
Attachment #490147 - Attachment is obsolete: true
Attachment #490460 - Flags: superreview+
Attachment #490460 - Flags: review?(iann_bugzilla)
Attachment #490147 - Flags: review?(iann_bugzilla)

Comment 4

7 years ago
(In reply to comment #2)
> Comment on attachment 490147 [details] [diff] [review]
> Proposed patch
> 
> >+                this.updateMessageIdNode(messageIdNodes[index * 2], index + 1,
> >+                  this.mMessageIds[index], index == numMessageIds - 1);
> ...
> >+                this.updateMessageIdNode(itemInDocument, index + 1,
> >+                  this.mMessageIds[index], index == numMessageIds - 1);
> [Annoying, but not easily fixed without a rewrite.]
One way round is to pass in numMessageIds instead and then do (aIndex == aNumIds) instead of aIsLastId in the if statement in the private method.

Comment 5

7 years ago
Comment on attachment 490460 [details] [diff] [review]
Proposed patch (v2)

r=me with comment 4 examined.
Attachment #490460 - Flags: review?(iann_bugzilla) → review+

Comment 6

7 years ago
(In reply to comment #4)
>(In reply to comment #2)
>> (From update of attachment 490147 [details] [diff] [review])
>>>+                this.updateMessageIdNode(messageIdNodes[index * 2], index + 1,
>>>+                  this.mMessageIds[index], index == numMessageIds - 1);
>>...
>>>+                this.updateMessageIdNode(itemInDocument, index + 1,
>>>+                  this.mMessageIds[index], index == numMessageIds - 1);
>>[Annoying, but not easily fixed without a rewrite.]
>One way round is to pass in numMessageIds instead and then do (aIndex ==
>aNumIds) instead of aIsLastId in the if statement in the private method.
Good point, but I was actually thinking about the first parameter!
(Assignee)

Comment 7

7 years ago
Created attachment 490496 [details] [diff] [review]
Final patch for checkin [Checkin: comment 8]

(In reply to comment #4)
> One way round is to pass in numMessageIds instead and then do (aIndex ==
> aNumIds) instead of aIsLastId in the if statement in the private method.

So done, I renamed the parameter "aLastId" though, thus it's more obvious that any index can be passed as such.

Push on trunk, please (r=IanN, sr=Neil)
Attachment #490460 - Attachment is obsolete: true
Attachment #490496 - Flags: superreview+
Attachment #490496 - Flags: review+
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
Whiteboard: [c-n: comm-central]
Comment on attachment 490496 [details] [diff] [review]
Final patch for checkin [Checkin: comment 8]

http://hg.mozilla.org/comm-central/rev/49d830521725
Attachment #490496 - Attachment description: Final patch for checkin → Final patch for checkin [Checkin: comment 8]
Leaving the status change decision to you.
Keywords: checkin-needed
Whiteboard: [c-n: comm-central]
Target Milestone: --- → seamonkey2.1b2
(Assignee)

Comment 10

7 years ago
Thanks to all, I'm closing this bug as "Fixed" (if someone wants to address Neil's comment #6, this should be better handled in a follow-up bug).
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.