Open Bug 468076 Opened 16 years ago Updated 15 years ago

Twisties for message-ID and references don't remember their status when switching to another message

Categories

(SeaMonkey :: MailNews: Message Display, enhancement)

Product:
SeaMonkey
Component:
MailNews: Message Display
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

People

(Reporter: prof, Unassigned)

Details

Attachments

(1 file)

Add hidden pref
1.06 KB, patch
mnyromyr
: review-
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20081202 Lightning/1.0pre SeaMonkey/2.0a2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20081202 Lightning/1.0pre SeaMonkey/2.0a2

The twisties in the Mail/News-Component used to have a "memory" of their state throughout all messages. 
e.g.: References/Message-ID was opened (twisty indicating down) and a new message being selected did not affect the status of the twisty, it was opened for the new message too. 

This behaviour was changed but no option (not even a "hidden" one) to select old or new behaviour seems to be available. (therefore this request for the ability to change behaviour on basis of a pref)

Reproducible: Always

Steps to Reproduce:
1. Open a message in mail/news
2. Open a twisty for message-ID or references
3. Select another message
Actual Results:  
twisty "forgot" it has been opened/activated

Expected Results:  
twisty should still have been acivated (if requested pref is set accordingly)

location of setting seems to be mailWidgets.xml (method name="clearHeaderValues")

draft of possible solution (untested code for MIDs supplied by Jens Hatlak, thx for contribution):

var pref = Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefBranch);
var expandMID = pref.getBoolPref("mailnews.headers.always_expand_message-id");
if (!expandMID && this.showFullMessageIds)
{
  this.showFullMessageIds = false;
  this.toggleIcon.removeAttribute("open");
}
Summary: Twisties for message-ID and references with(out) memory whn switching to another message → Twisties for message-ID and references with(out) memory when switching to another message
Version: unspecified → Trunk
Summary: Twisties for message-ID and references with(out) memory when switching to another message → Twisties for message-ID and references don't remember their status when switching to another message
Attached patch Add hidden pref — — Splinter Review 
I think it doesn't hurt to make this configurable. This patch adds choice without UI bloat (hidden pref) and doesn't change the default.
Assignee: nobody → jh
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #351602 - Flags: review?(neil)
Attachment #351602 - Flags: review?(neil) → review?(mnyromyr)
Comment on attachment 351602 [details] [diff] [review]
Add hidden pref

I'm all for making it configurable, but it doesn't work out this way:
- the state is only remembered during the current session
- the solution isn't extensible/basic enough (eg. it doesn't work with From or other headers without patching them up the same way)
- the pref name is an atrocity ;-)

Minussing basically for the first to points.


Finally some general nits on the code itself, although meaningless: 

>       <method name="clearHeaderValues">
>         <body>
>           <![CDATA[
>             // clear out our local state
>             this.mMessageIds = new Array;

This should be 
  this.mMessageIds = [];
(Although using "new Array" is not wrong.)

>-            if (this.showFullMessageIds)

You could return early if showFullMessageIds is false.

>+            var persistState = false;
>+            try
>+            {
>+              var pref = Components.classes["@mozilla.org/preferences-service;1"]
>+                                   .getService(Components.interfaces.nsIPrefBranch);
>+              persistState = pref.getBoolPref("mailnews.headers.persist_messageids_expand_state");

You could return early if persistState is true.

>+            }
>+            catch (ex)
>+            {
>+            }

We usually allow empty catches to be written as 
  catch (ex) {}
for brevity even in brace-align style.

>+            if (!persistState && this.showFullMessageIds)

Given you used the early returns above, the entire if statement (but not its content) would now be superfluous... ;-)
Attachment #351602 - Flags: review?(mnyromyr) → review-
(In reply to comment #2)
> (From update of attachment 351602 [details] [diff] [review])
> I'm all for making it configurable, but it doesn't work out this way:
> - the state is only remembered during the current session
> - the solution isn't extensible/basic enough (eg. it doesn't work with From or
> other headers without patching them up the same way)
> - the pref name is an atrocity ;-)
> 
> Minussing basically for the first to points.

Seems someone needs to come up with a better approach then.
Status: ASSIGNED → NEW
Assignee: jh → nobody
This is quite similar to bug 381491, which is about configuration and/or remembering the twisty status for the address headers. Those are reverting to closed-twisty/show-one-line-only defaults when leaving the message as well.

The apparent options are either to introduce a set of preferences, which would work but is not necessarily nice as mentioned in comment #2, or to have a more comprehensive solution. Most intuitive may be to let the user customize the header view as desired, and then have some sort of "use as default" option to store the user's choices when switching messages and between sessions. In the best case, this could be accomplished by just a single preference, but imply a UI element to collect twisty and other states to build a value for that pref from the current selections.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: