Using jan24 commercial trunk build Using Next Unread Thread will currently not engage navigation across folders/groups. 1. Within one mail account, go to a populated user created folder. Put into threaded mode. Mark several of the messages unread. 2. Go to INBOX for that same mail account. There are no unread messages in the inbox. 3. Put the inbox in threaded mode. Select a message in the inbox, then Go|Next|Unread Thread. Actual result: nothing happens Expected: Should display a dialog "Advance to next unread message in <folder name with unread>?" Note: gave threaded mode example, should also work with flat sort.

nominating for next release

marking nsbeta1+

marking nsbeta1- and moving to future milestone.

When in threaded mode, do you have non-zero unread counts for any threads? If so, this is one of the effects of bug 59681.

With ID 2001072403 I have the following mail layout: * mail account (japj@oce.nl) * mail account (...) * Local Folders * news account * news account I have a message filter that moves mail from the japj@oce.nl mailaccount to a subdirectory of the "Local Folders".I have unread mails in Local Folders and in several newsgroups. If I stand in the Inbox folder of my first email account and press 'n' (Next Unread Mail), I get a dialog box for unread mail in my *LAST* news account. This is a bug, since it should go to the unread mails in my Local Folder first.

Laurel, can you please check if this is still reproducible (despite Seth's message navigation fixes recently) ?

Using jan10 commercial build, win98: This still exists exactly as written. Just to keep this clear, this is using next unread THREAD, not next unread message. To Jeroen in comment #5: your comment is addressing a different issue; not for this bug report.

This patch fixes both this bug and bug 150970. I rearranged the case nsMsgNavigationType::nextUnreadThread so that it always falls through nsMsgNavigationType::nextUnreadMessage since it didn't make sense to me why we weren't going to the next unread message if a message wasn't currently selected. It also didn't make sense to me why we were bailing if MarkThreadOfMsgRead failed so I removed that error checking. If the thread isn't successfully marked as read than the user will probably be taken to another message in the same thread and will eventually get to a new thread by repeating using next unread thread whereas if we bail than the user will stay at the same place and since the error is likely to repeat will never get anywhere. Regardless, both behaviors are equally broken so it makes sense to go with the simpler code. It may be possible(and better) to intelligently deal with an error, but the only thing I could think of was to rerun MarkThreadOfMsgRead, but if it had an error once it is likely to just fail again. Since with this change a message will be selected when we do cross folder navigation I added nextUnreadThread to the types of motion for which we do cross folder navigation thereby fixing this bug. I also discovered that we are marking the thread as read twice which is redundant so I removed the marking thread as read call from mail3PaneWindowCommands.js which should result in a negligible performance gain which is always good.

This is the most current IMAP log. Hope you find it helpful.

David, if TB is interested in this fix, I can do that, too...

Comment on attachment 245914 [details] [diff] [review] updated patch to trunk >+ if (type != nsMsgNavigationType.nextUnreadMessage && >+ type != nsMsgNavigationType.nextUnreadThread && >+ type != nsMsgNavigationType.forward && >+ type != nsMsgNavigationType.back) // currently, only do cross folder navigation for "next unread message" Comment is out of date ;-) > { >- nsMsgKeyArray idsMarkedRead; The { only exists here to scope the variable, so you could outdent the block. >+ nsMsgKeyArray idsMarkedRead; >+ rv = MarkThreadOfMsgRead(m_keys.GetAt(startIndex), startIndex, idsMarkedRead, PR_TRUE); > } Worth testing rv? >+ return NavigateFromPos(nsMsgNavigationType::nextUnreadMessage, startIndex, pResultKey, pResultIndex, pThreadIndex, PR_TRUE); > break; Useless break after return.

Comment on attachment 245914 [details] [diff] [review] updated patch to trunk >+ //First mark the current thread as read already handled. This comment makes no sense. r=me with this and my previous comments addressed.

Comment on attachment 245914 [details] [diff] [review] updated patch to trunk yes, I think this would be nice for TB as well, thx.

Updated patch to Neil's comments; see comment #8 why the rv value is ignored. Added TB changes, but in fact it's just one additional file, since msgViewNavigation.js and nsMsgDBView.cpp are shared anyway. Carrying over r+ and rerequesting sr.

Comment on attachment 246223 [details] [diff] [review] SM + TB patch thx, Karsten

Landed on trunk.

Comment on attachment 246223 [details] [diff] [review] SM + TB patch Asking for branch approval for SM1.1 and TB2 - low risk, fixes a long-standing UI inconsistency.

Comment on attachment 246223 [details] [diff] [review] SM + TB patch Sounds good to me for SeaMonkey, a=me, I think the additional TB approval counts enough for checkin

a=me for the thunderbird changes on the branch. 1.8.1.1 triage team, this has no impact on firefox. Karsten, I wonder if we should add thunderbird-2 approval flag to these suite bugs? that would allow me to approve mailnews only bugs on the branch for us. I don't have the right privileges for 1.8.1.1.

> Karsten, I wonder if we should add thunderbird-2 approval flag to these suite > bugs? Yeah, having TB flags for SM MailNews components sounds like a good idea.

Landed on MOZILLA_1_8_BRANCH.

Comment on attachment 246223 [details] [diff] [review] SM + TB patch Removing wrong branch flag, see comment #19 for details.

*** Bug 150970 has been marked as a duplicate of this bug. ***