189490 - Mailview should be remembered through exit.

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[laurel]

Using 2003-01-15 commercial trunk

We currently reset the mailview to All upon exit/restart.  I think we should
remember the last view so the user doesn't have to always reset.  

A good example is that you want to view Not Junk messages only. Upon restart,
you see Junk mail then have to reset the view.

Steps:
1.  Open mail, set Views dropdown to anything but All.
2.  Exit and restart, go to mail again. 

Result: View is reset to All
Expected: view remembered (global)

(Note: current implementation decision is that the view is global, not
remembered  on a per account or per folder basis.)

Comment 1

[Ninoschka Baca]

The same problem occurs when closing/reopening the mail window in the same
session. The mailveiws always reverts back to All when it should remember the
last view.

Comment 2

[Samir Gehani]

Mail triage team: need info.  Jennifer, could you elaborate on the expected
behavior based on your spec?  Thanks.

Comment 3

[Ninoschka Baca]

Another reason to justify the expected behavior:

- The View dropdown selection should match the View|Messages menu. When using
the menu item to select a view (i.e. Unread), after an exist/restart, this menu
item is still set to Unread. Therefore the View drop down should still have
Unread selected. This should also be true when closing the Mail window in the
same session.

Comment 4

[jglick]

We didn't cover this specifically when we reviewed the spec, but I agree with
laurel and nbaca.

Comment 5

[Samir Gehani]

Mail triage team: nsbeta1+/adt3

Comment 6

[(not reading, please use [e-mail] instead)]

ok, will work on storing the view index in prefs.

Comment 7

[(not reading, please use [e-mail] instead)]

Attachment: patch

Comment 8

[David :Bienvenu]

Comment on attachment 122527 [details] [diff] [review]
patch

sr=bienvenu

Comment 9

[(not reading, please use [e-mail] instead)]

fixed.

this is designed for people who really want to stick with a "no junk" view or a
"people I know view".

for the normal case (if you don't use views, so you are in the "all" view), this
should be a no-op.

Comment 10

[parish]

I started getting crashes (in gklayout.dll) whenever I clicked the View dropdown
from around the time this was checked in. Since no-one else has reported it in
the NGs I figured it must be my local tree. I had Neil's patch for bug 189007 so
backed it out which seemed to fix it - but I still get a crash occassionally.
Deleting localstore.rdf and XUL.mfl fixes it for a while.

See http://bugzilla.mozilla.org/show_bug.cgi?id=189007#c15

Could this fix be the cause?

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030510

Comment 11

[[e-mail]]

Comment on attachment 122527 [details] [diff] [review]
patch

>@@ -528,3 +528,5 @@
> // to reduce forking in the js / C++
> // overridden by stand alone mail
> pref("mail.standalone", false);
>+
>+pref("mailnews.view.last",0); // 0 == "all" view
/me idly wonders whether persistence could have been used instead
(not that it matters)

>+  // store this, to persist across sessions
>+  gPrefs.setIntPref("mailnews.view.last", parseInt(val));
Bah, don't need parseInt, XPConnect is your friend :-)

>+function ViewChangeByValue(aValue)
>+{
>     var viewPicker = document.getElementById('viewPicker');
>-    viewPicker.selectedItem = document.getElementsByAttribute("value", kLastDefaultViewIndex + aDefaultSelectedIndex)[0];
>+  viewPicker.selectedItem = document.getElementsByAttribute("value", aValue)[0];
>     viewChange(viewPicker);
>-  }
> }
Parish: try changing this to viewPicker.value = aValue;

Comment 12

[parish]

Neil: Change *what* to

viewPicker.value = aValue;

the whole function body?

BTW, I've spun a debug build and when you click the drop down it ASSERTs:

###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().:
'mRawPtr != 0', file ../../../../dist/include/xpcom\nsCOMPtr.h, line 691

and then crashes. If you break to the debugger from the MS "phone home" screen
the current line pointer is at

    mCloseTimer->Cancel();

near the end of nsMenuPopupFrame::Notify() in nsMenuPopupFrame.cpp

What's really puzzling me is that no-one else appears to see this.

Comment 13

[parish]

Turns out it was bug 205728 that caused the crash (I hadn't found it 'coz it was
filed under Browser, not Mail/News).

Comment 14

[Ninoschka Baca]

Trunk build 2003-05-18: WinXP
Trunk build 2003-05-20: Mac 10.1.5, Linux RH 8
Verified Fixed. It remembers the view after closing the Mail window and also
after exiting the entire application.

Comment 15

[(not reading, please use [e-mail] instead)]

this checkin caused the regression bug #206504, which is now fixed.

Comment 16

[[e-mail]]

Parish: no, I only meant one line. Seth has since checked in a different
workaround. What was happening was the menulist was trying to display as
selected an item that wasn't actually in the list :-(

Comment 17

[parish]

I realized that when I saw your comment in bug 206504.

I filed bug 206512 for the same thing but in between me searching for a bug for
the problem and actually filing, someone had filed bug 206504.

Can you, or someone else, close bug 206512 as a DUP?