Closed Bug 10872 Opened 25 years ago Closed 22 years ago

[FEATURE] Remember last selected message in a folder

Categories

(SeaMonkey :: MailNews: Message Display, defect, P3)

Product:
SeaMonkey
Component:
MailNews: Message Display
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla1.3beta

People

(Reporter: scottputterman, Assigned: sspitzer)

References

Details

Attachments

(1 file, 4 obsolete files)

update patch, with bienvenu suggestions rolled in
16.48 KB, patch
sspitzer
: superreview+
Details | Diff | Splinter Review 
bug tracking feature in Summary.
Blocks: 10791
Target Milestone: M11
Target Milestone: M11 → M14
setting M14
Status: NEW → ASSIGNED
Target Milestone: M14 → M15
Sol: remember sort/thread mode is more important for B1 than remember last
selected message.
Target Milestone: M15 → M18
Disagree this is beta.  How do I disagree?  Do I just change the target and see
if anyone disagrees back? OK, I will try it.  Setting M18.  I do agree with
remember sort and thread however.
*** Bug 22895 has been marked as a duplicate of this bug. ***
moving to future milestone.
Target Milestone: M18 → Future
Guess we need to log a bug to remove the pref from the prefs ui main mail and
newsgroups panel, huh?  I'll do that...
QA Contact: lchiang → laurel
bug #46342 logged to remove pref.

Not much left on the panel, eh?
nominating for mail3
Keywords: mail3
marking nsbeta1+
Keywords: nsbeta1
Whiteboard: [nsbeta1+]
moving to mozilla0.8 milestone.
Target Milestone: Future → mozilla0.8
reassigning to chuang
Assignee: putterman → chuang
Status: ASSIGNED → NEW
moving to mozilla0.9
Target Milestone: mozilla0.8 → mozilla0.9
marking nsbeta1-
Keywords: nsbeta1 → nsbeta1-
Whiteboard: [nsbeta1+] → [nsbeta1+ 1/25]
Target Milestone: mozilla0.9 → Future
bringing back into nsbeta1+
Keywords: nsbeta1- → nsbeta1
Whiteboard: [nsbeta1+ 1/25] → [nsbeta1+]
Target Milestone: Future → mozilla0.9
moving to mozilla0.9.1
Target Milestone: mozilla0.9 → mozilla0.9.1
*** Bug 74872 has been marked as a duplicate of this bug. ***
Let me give this a try.
Taking
Assignee: chuang → prass
In 4.x at least we didn't remember this across sessions, just within the current
session.  From looking at the code it seems that this remembers it after you
quit and shut down.  I don't think it should do that since I still think users
will want to scroll to the first new message when starting up.  I can't tell for
sure if this is the case so ignore me if it isn't.
putterman is right, the current patch will persist the last msg across sessions.

4.x had this?  are we using the same pref that we used in 4.x?

suresh was going to persist the msg as an attribute in nsIMsgFolder, which makes
sense to me.

I've got a few other comments, I'll follow up soon.
3 things:

1) wrong pref, it looks like 4.x used "mailnews.remember_selected_message"
(can you confirm that using http://lxr.mcom.com)

if use the same pref, we do the right thing when the user migrates from 4.x

2) "var LastMsg", do "var lastMsg".  for code readability, LastMsg looks like 
function and not a variable.

3)  instead of persisting the attribute in localStore.rdf (by persisting it as 
an attribute on the folder content), add a string attribute to nsIMsgFolder for 
the last message.

I need to think about the rest of the patch, but that should get you started.
do you plan on conditionally, or unconditionally setting the "last msg" 
attribute on the nsIMsgFolder?

consider this case:

the user reads a message, changes the pref to "remember the last selected 
message", then switches folder.  if we conditionally set, we won't have 
remembered the last message.  we'd have to load another message before it 
started to work.

you have three choices:

1) unconditionally remember the last message, but conditionally use it.  
(simpler, but inefficient for users who aren't remember the last selected 
message)

2) add a pref callback in our 3 pane JS, that gets fired 
if "mailnews.remember_selected_message" gets changed.  if it gets set to true,
set the current displayed message attribute on the nsIMsgFolder
(more complicated, but correct)

3) conditionally set and conditionally get, and log a bug on this issue and 
specify the pref callback solution.

I'd suggest doing #3.
two more things

1) we shouldn't be persisting (within  the session) the index or the uri.  I 
think we need to using the message id.  index can change if messages are 
deleted.  we want to re select the last message, not the message in the same 
place as the last message.

2) 

we need to make sure we do the right thing for cross folder navigation (loading 
the next unread) and provide the functionality that gayatrib needs to finish 
the "Go To Folder" button in search.  your patch will break cross folder 
navigation.

to write the "FolderLoaded" handler correctly, you will have to clean up and 
fix some code.

we should be doing this:

if the special global for message id is set, scroll to and select the exact 
message.  (search uses this).  (null it out after using it.)
if that didn't work and the special global for type of navigation is set, 
scroll and select that message.  (null it out after using it.)
if that didn't work and the user has set the pref to remember the selected 
message, scroll to and select the persisted message.
if that didn't work, scroll to, but don't select the first new message
if that didn't work, scroll to the top.
<end>

it's getting complicated, you should move logic out of the "FolderLoaded" 
handler.

do you have what you need to turn message-id into index?  you might have to 
extend nsIMsgDBView.idl

search will have message uri but persistence will use a message id.

we can turn the message uri into a nsIMsgDBHdr and get the message id and then
use the same code to turn message id into index.
moving to 0.9.2
Target Milestone: mozilla0.9.1 → mozilla0.9.2
moving to 0.9.3
Target Milestone: mozilla0.9.2 → mozilla0.9.3
*** Bug 83472 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Adding myself to the list, I want to listen :)
*** Bug 96155 has been marked as a duplicate of this bug. ***
Seth - Is this worhty of a nsbranch+, and/or the 0.9.4 milestone?
Whiteboard: [nsbeta1+]
nope, not worthy.
Assignee: prass → sspitzer
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Comment on attachment 32497 [details] [diff] [review]
initial patch - store the message index on folder. The new one to store the URI and load folder coming up

bad patch, out of date and needs work.
Attachment #32497 - Attachment is obsolete: true
Attachment #32497 - Flags: needs-work+
moving out.
Target Milestone: mozilla0.9.5 → mozilla0.9.7
Blocks: 104166
*** Bug 107880 has been marked as a duplicate of this bug. ***
*** Bug 108442 has been marked as a duplicate of this bug. ***
Keywords: mail3 → nsbeta1+
Target Milestone: mozilla0.9.7 → mozilla0.9.9
*** Bug 109947 has been marked as a duplicate of this bug. ***
*** Bug 112717 has been marked as a duplicate of this bug. ***
*** Bug 100141 has been marked as a duplicate of this bug. ***
moving to 1.0.1
Target Milestone: mozilla0.9.9 → mozilla1.0.1
*** Bug 121044 has been marked as a duplicate of this bug. ***
Blocks: 122274
Status: NEW → ASSIGNED
Keywords: nsbeta1+ → nsbeta1-
Target Milestone: mozilla1.0.1 → mozilla1.2
*** Bug 123471 has been marked as a duplicate of this bug. ***
*** Bug 48120 has been marked as a duplicate of this bug. ***
*** Bug 126167 has been marked as a duplicate of this bug. ***
*** Bug 140260 has been marked as a duplicate of this bug. ***
*** Bug 142857 has been marked as a duplicate of this bug. ***
*** Bug 145643 has been marked as a duplicate of this bug. ***
*** Bug 146594 has been marked as a duplicate of this bug. ***
*** Bug 147921 has been marked as a duplicate of this bug. ***
*** Bug 153068 has been marked as a duplicate of this bug. ***
*** Bug 153966 has been marked as a duplicate of this bug. ***
*** Bug 157148 has been marked as a duplicate of this bug. ***
*** Bug 158611 has been marked as a duplicate of this bug. ***
Adding 4xp and nscatfood nomination.
Keywords: 4xp, nsCatFood
*** Bug 160284 has been marked as a duplicate of this bug. ***
*** Bug 164726 has been marked as a duplicate of this bug. ***
*** Bug 164987 has been marked as a duplicate of this bug. ***
moving to 1.2beta.
Target Milestone: mozilla1.2alpha → mozilla1.2beta
Keywords: nsbeta1- → nsbeta1
I think either varada or ssu were going to help on this one.


varada's working on it.
Assignee: sspitzer → varada
Status: ASSIGNED → NEW
Commenting out UI for pref till the feature is implemented.
Comment on attachment 102262 [details] [diff] [review]
Changes to pref-mailnews.xul and pref-mailnews.dtd.

there is no need to comment out the .dtd items.  the will jut be unused.
Attachment #102262 - Flags: needs-work+
Commented the lines in pref-mailnews.xul.
Status: NEW → ASSIGNED
Keywords: nsbeta1 → nsbeta1+
taking.

let's hope nsDBFolderInfo::GetLastMessageLoaded() and
nsDBFolderInfo::SetLastMessageLoaded() work, and do the hard parts for me.
Assignee: varada → sspitzer
Status: ASSIGNED → NEW
Target Milestone: mozilla1.2beta → mozilla1.3beta
Attached patch initial patch (obsolete) — — Splinter Review 
1)  only seems to work for the inbox (maybe other folders get closed, and the
dbinfo isn't getting written out, read back in?)

2)  probably not setting last selected in the right place (this should be for 3
pane only, will stand alone msg view affect the last selected message?)

3)  will dbinfo changes persist across sessions?

4)  this patch selects and scrolls, need to verify that is desired (what did
4.x do?) and make sure I didn't break any existing functionality (open message
folder, next unread, etc)
Attachment #102262 - Attachment is obsolete: true
I think I see why this only works for inbox.

we are missing

GetInt32PropertyWithToken(m_lastMessageLoadedColumnToken, m_lastMessageLoaded);

from nsDBFolderInfo::LoadMemberVariables()

we probably keep inbox folder open, but close the others (for footprint reasons)

so we keep creating dbfolderinfo, but never set m_lastMessageLoaded from what we
saved.


a fix would be
Status: NEW → ASSIGNED
ok, that fixes it, but now the last selected message persists across sessions,
which we don't want.

I think I'm going to have to remove that from dbinfo, and put it on nsIMsgFolder
and not persist on disk.

while the db can be unloaded, the folder won't be.

working on that fix now.
Attached patch patch (obsolete) — — Splinter Review 
I still need to test for regressions, but the basics of this works.

the stand alone msg window mucks with the last selected message, because of
where I'm setting it.  (I doubt we want that.)

if not, I'll find a new place to set the last selected message, that is 3 pane
only.
Attachment #109209 - Attachment is obsolete: true
talked it over with bienvenu, he's given me some suggestions on how to improve
this patch.
Comment on attachment 109210 [details] [diff] [review]
patch

r/sr=bienvenu
Attachment #109210 - Flags: superreview+
Comment on attachment 109236 [details] [diff] [review]
update patch, with bienvenu suggestions rolled in

carrying over sr=bienvenu from last patch.
Attachment #109236 - Flags: superreview+
fixed.

any comments about the stand alone msg window affecting the last selected message?

the current might be desired.

to see the affect, do this:

1) open 3 pane, select message x, and double click to get stand alone msg window.
2) switch folders in 3 pane
3) in stand alone msg window, navigate to another message, y.
4) switch back to original folder.  instead of x being selected, y is selected.

this is sort of an edge case.

if we think it's a bug, let's spin it off.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Awesome Seth!!! whooo hoooo!!!!
I think this bug has had some unintended side affects.

With 1213 build, win2k, in three pane view, with the focus in the message index,
using the up/down arrow keys does what you would generally expect, ie, moves up
and down through the index.

With 1214 build, the up down arrow key will move up/down by one message, but
focus will then immediately change to the message pane.

With 1217 build of course, this is gone again, but I can't work what fixed it
exactly. :(

Cheers,
Karl P
The problems described in comment 75 where propably caused by the fix to bug
110718 which was backed out.
Blocks: 186504
This check-in created a few very annoying side-effects:

- when a folder with new messages is selected (in folders pane), the headers
panes is scrolled to a position where no new headers are visible (so I have to
scroll before being able to read new messages).
- if I mark a selected message as unread before leaving a folder (something I
almost always do when briefly browsing new messages), then when I come back to
the folder, the message is selected again, automatically marked read again, and
I have to keep marking it unread *every time*.

Filed bug 186504 on these issues.
OK using 2003-01-13 commercial trunk: win98, linux rh8.0 mac OS 10.2
Basics working and tested in mail (general POP and IMAP) and news accounts, a
few different view settings, 3-pane and standalone navigation. Tested that pref
works to enable/disable this feature. Not all scenarios tested -- will log any
issues found as separate bug report(s).

Marking this verified.
Status: RESOLVED → VERIFIED
*** Bug 195319 has been marked as a duplicate of this bug. ***
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: