+++ This bug was initially created as a clone of Bug #660882 +++ When performing a search via the search window (Tools -> Search Messages) it's impossible to open a second message by double-clicking or selecting it and hitting the open button, unless mailnews.reuse_message_window is set to false. This is a regression from SeaMonkey 2.0. This should make SM 2.3 (currently in Beta) if possible. Here's what happens: 1. MsgOpenSelectedMessages (mailWindowOverlay.js) is called 2. MsgOpenSelectedMessageInExistingWindow is called from there The difference between 2.0 and later is that with 2.0, gDBView.URIForFirstSelectedMessage in MsgOpenSelectedMessageInExistingWindow was not defined so the catch block was called and the function returned false, which made MsgOpenSelectedMessages not return early. With 2.1 and 2.2, gDBView.URIForFirstSelectedMessage is now defined so the try block is executed until the end. Further down it calls windowID.LoadMessageByMsgKey which seems to have a different gDBView; in any case it doesn't do the right thing. Instead of trying to investigate what LoadMessageByMsgKey is supposed to do I took the short track: Don't go there in the first place! We really want to open new windows for each message opened from Advanced Search, no matter what the pref says. While I was at it I took the liberty to kill the useless gWindowReuse variable. Contrary to its name, it's a local variable that is not used anywhere else. I hope the checking for the dbView search type is correct. ;-)

Comment on attachment 545952 [details] [diff] [review] patch >+ var reuse_win = gPrefBranch.getBoolPref("mailnews.reuse_message_window"); >+ if (gDBView.viewType == nsMsgViewType.eShowSearch) >+ reuse_win = false; >+ >+ if (reuse_win && numMessages == 1 && MsgOpenSelectedMessageInExistingWindow()) > return; I would be tempted to write this in one statement i.e. if (numMessages == 1 && gDBView.viewType != nsMsgViewType.eShowSearch && Services.prefs.getBoolPref("mailnews.reuse_message_window") && MsgOpenSelectedMessageInExistingWindow()) return;

Sure, why not. The extended comment still explains the logic.

Comment on attachment 546415 [details] [diff] [review] patch v1a (In reply to comment #0) > We really want to open new windows for each message opened from Advanced > Search, no matter what the pref says. Actually, no — why would we want to do that?! That's exactly what the pref is for.

(In reply to comment #3) > Comment on attachment 546415 [details] [diff] [review] [review] > patch v1a > > (In reply to comment #0) > > We really want to open new windows for each message opened from Advanced > > Search, no matter what the pref says. > > Actually, no — why would we want to do that?! Please go back to a previous version and check. That's what we have (always?) been doing before it regressed. > That's exactly what the pref is for. Debatable. I'd say opening a message from the main window or from search results is not the same. Anyway, if you're serious about respecting the pref in any case (which would actually be a change rather than a fix IMO) then I'd appreciate some help on how to get this right. As I explained in previous comments, the code in question wasn't reached in former times, and additionally we're dealing with different gDBViews now (maybe we'll have to change some functions to accept a dbView parameter; what a mess).

(In reply to comment #3) > > We really want to open new windows for each message opened from Advanced > > Search, no matter what the pref says. > > Actually, no — why would we want to do that?! If I open search results, I might want to compare different results side by side or leave some open longer than others. For that I need multiple standalone windows. This is in contrast to the normal usage of the standalone window (opened from the main MailNews window) which you open just once and then use navigation keys like N or Space to proceed (by default; IMO the point of having the pref is to allow changing this behavior of a message flow reading window).

(In reply to comment #5) > (In reply to comment #3) > > > We really want to open new windows for each message opened from Advanced > > > Search, no matter what the pref says. > > I want to open several search result for different reason, as used to do it in former SM versions

(In reply to comment #4) > > > We really want to open new windows for each message opened from Advanced > > > Search, no matter what the pref says. > > > > Actually, no — why would we want to do that?! > > Please go back to a previous version and check. That's what we have > (always?) been doing before it regressed. It may have been broken for quite some time now, but that was surely not intended, see <http://mxr.mozilla.org/comm-central/source/suite/mailnews/mailWindowOverlay.js?mark=1678-1679#1675>. > As I explained in previous comments, the code in > question wasn't reached in former times, and additionally we're dealing with > different gDBViews now (maybe we'll have to change some functions to accept > a dbView parameter; what a mess). Actually, the problem is a bit less odd: - we clone the gDBView from the window which requests the opening - nsMsgDBView::LoadMessageByViewIndex checks mSuppressMsgDisplay, which is true for the search window gDBView (we never have a message pane there) and false for e.g. the main message window We just need to make sure that <search>.gDBView.suppressMsgDisplay is overridden before requesting a message load. This patch is doing that. (In reply to comment #5) > If I open search results, I might want to compare different results side by > side or leave some open longer than others. For that I need multiple > standalone windows. This is in contrast to the normal usage of the > standalone window Well, that may be an interesting enhancement, but it's not the actual bug here. The standard thread pane context menu (which includes eg. "Open Message in New Window") is missing in the search window anyway …

(In reply to comment #7) > We just need to make sure that <search>.gDBView.suppressMsgDisplay is > overridden before requesting a message load. > > This patch is doing that. Hmm, I'd still like to get rid of gWindowReuse... Same file. > (In reply to comment #5) > > If I open search results, I might want to compare different results side by > > side or leave some open longer than others. For that I need multiple > > standalone windows. This is in contrast to the normal usage of the > > standalone window > > Well, that may be an interesting enhancement, but it's not the actual bug > here. I think I'm at a point where I'll just leave that discussion and set the pref to false for me. I don't use the standalone message window for browsing anyway, and this whole discussion seems to be more on personal preferences (and maybe consistency with internal prefs).

(In reply to comment #8) > Hmm, I'd still like to get rid of gWindowReuse... Same file. Done; carrying over r+. > I think I'm at a point where I'll just leave that discussion and set the > pref to false for me. I don't use the standalone message window for browsing > anyway, and this whole discussion seems to be more on personal preferences > (and maybe consistency with internal prefs). JFTR: If you select multiple search results and hit the Open button, multiple message windows will get spawned. The pref only handles the "open one (additional) message" case, and behaviour of that should be consistent.

Pushed to trunk as <http://hg.mozilla.org/comm-central/rev/ea935b82337f>.

Comment on attachment 548003 [details] [diff] [review] fix reusing message window from search, remove gWindowReuse No L10n changes so should be okay to go into both comm-beta and comm-aurora

Pushed to aurora as <http://hg.mozilla.org/releases/comm-aurora/rev/23c92504abe8>. Pushed to beta as <http://hg.mozilla.org/releases/comm-beta/rev/45a0b6334810>.

Thanks for the trial to change it ... but to open several messages at once is not that, what we had in former times. I think, that all who have notified for this bug want to open each message after the other till the "one" will be found. Another great (additional) solution would be, to add a message (preview) area in the Advanced Search Window likewise it exists already for the Inbox/Sent/Trash folder.

(In reply to Glutton.Vidal from comment #13) > but to open several messages at once > is not that, what we had in former times. I think, that all who have > notified for this bug want to open each message after the other till the > "one" will be found. Well, the module owner made a decision so there is little point to continue this road. However, you can can set mailnews.reuse_message_window to false in about:config and get the old behavior back (though at the cost of also changing the behavior of opening individual messages from the thread pane). > Another great (additional) solution would be, to add a message (preview) > area in the Advanced Search Window likewise it exists already for the > Inbox/Sent/Trash folder. Huh? I understand the first part but not the second one. :-/

... but not the best decision maybe this following Screenshot helps to understand

Frame for message preview

It would be even a compromise to open several messages from Advanced Search Window with right mouse click likewise it exists already for the Inbox/Sent/Trash folder. But so far it isn't possible.