Last Comment Bug 545420 - E-mails found with advanced search cannot be opened by double click [RestoreSelectionWithoutContentLoad is not defined]
: E-mails found with advanced search cannot be opened by double click [RestoreS...
Status: RESOLVED FIXED
: fixed-seamonkey2.0.12
Product: SeaMonkey
Classification: Client Software
Component: MailNews: General (show other bugs)
: Trunk
: All All
: -- minor (vote)
: seamonkey2.1b2
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-02-10 09:04 PST by Andy Boze
Modified: 2010-12-25 10:39 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (1.04 KB, patch)
2010-12-09 15:23 PST, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review-
Details | Diff | Splinter Review
patch v2 (2.07 KB, patch)
2010-12-18 08:29 PST, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review-
Details | Diff | Splinter Review
patch v1a [Checkin: comments 14 and 16] (1.64 KB, patch)
2010-12-19 12:51 PST, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review+
mnyromyr: superreview+
mnyromyr: approval‑seamonkey2.0.12+
Details | Diff | Splinter Review

Description Andy Boze 2010-02-10 09:04:04 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9pre) Gecko/20100203 SeaMonkey/2.0.3pre compatible Firefox/3.5.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9pre) Gecko/20100203 SeaMonkey/2.0.3pre

When I search messages, I think I used to be able to double click on the messages in the match list to open them. Now I have to use the Open button.

Reproducible: Always

Steps to Reproduce:
1. Open the Advanced Message Search window.
2. Do a search that will return one or more matches.
3. Double click on one of the matching e-mails in the message list.
Actual Results:  
Nothing happens.

Expected Results:  
The double clicked message should open.
Comment 1 Jens Hatlak (:InvisibleSmiley) 2010-12-06 12:16:07 PST
Anything in the Error Console (Tools/Web Development)?
Comment 2 Andy Boze 2010-12-07 18:20:39 PST
Yes. When I double click on a message in the search results I get:

Error: RestoreSelectionWithoutContentLoad is not defined
Source File: chrome://messenger/content/threadPane.js
Line: 233
Comment 3 Philip Chee 2010-12-07 20:28:13 PST
Works for me:
Mozilla/5.0 (Windows; U; Windows NT 6.1; rv:1.9.1.17pre) Gecko/20101202 Lightning/1.0b2pre SeaMonkey/2.0.12pre

Works for me:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101207 SeaMonkey/2.1b2pre

You might have an incompatible extension. Can you reproduce this in -safe-mode?
Comment 4 Andy Boze 2010-12-09 08:58:27 PST
Tried in -safe-mode. Same error message.
Error: RestoreSelectionWithoutContentLoad is not defined
Source File: chrome://messenger/content/threadPane.js
Line: 233

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.17pre) Gecko/20101207 SeaMonkey/2.0.12pre

Works for me in SeaMonkey/2.1b2pre.
Comment 5 Philip Chee 2010-12-09 11:37:51 PST
> Works for me in SeaMonkey/2.1b2pre.

if you set |browser.tabs.opentabfor.doubleclick| to true then you'll see the problem.

Contrast: http://mxr.mozilla.org/comm-central/source/suite/mailnews/threadPane.js?mark=233#229
229   else if (AllowOpenTabOnDoubleClick())
230   {
231     // open the message in a new tab on double click
232     MsgOpenNewTabForMessage();
233     RestoreSelectionWithoutContentLoad(GetThreadTree());
234   }
235   else
236   {
237     MsgOpenSelectedMessages();
238   }
239 }

With: http://mxr.mozilla.org/comm-central/source/suite/mailnews/threadPane.js?mark=51,54#48
48     if (AllowOpenTabOnMiddleClick())
49     {
50       // we don't allow new tabs in the search dialog
51       if ("RestoreSelectionWithoutContentLoad" in window)
52       {
53         MsgOpenNewTabForMessage();
54         RestoreSelectionWithoutContentLoad(GetThreadTree());
55       }
Comment 6 Jens Hatlak (:InvisibleSmiley) 2010-12-09 15:23:14 PST
Created attachment 496647 [details] [diff] [review]
patch

Like this? (Kudos to Ratty, though I would probably have found that myself ;-))
Comment 7 Philip Chee 2010-12-09 20:34:29 PST
> +           "RestoreSelectionWithoutContentLoad" in window)
I think Neil said that if (window.foo) e.g. if (window.RestoreSelectionWithoutContentLoad) should also work but I could be wrong. Check on irc?

I don't know if it is worth checking for the window.opener and call the functions there?

var tabmail = window.opener;
if (tabmail && !tabmail.closed && tabmail.wintpe == "mail:3pane")
{
  tabmail.MsgOpenNewTabForMessage();
  tabmail.RestoreSelectionWithoutContentLoad(GetThreadTree());
}

Probably not. Mnyromyr?
Comment 8 Philip Chee 2010-12-09 20:41:34 PST
>   tabmail.MsgOpenNewTabForMessage();
>   tabmail.RestoreSelectionWithoutContentLoad(GetThreadTree());
Ugh I don't think this will work.
Comment 9 Karsten Düsterloh 2010-12-17 17:30:32 PST
Comment on attachment 496647 [details] [diff] [review]
patch

(In reply to comment #7)
> > +           "RestoreSelectionWithoutContentLoad" in window)
> I think Neil said that if (window.foo) e.g. if
> (window.RestoreSelectionWithoutContentLoad) should also work but I could be
> wrong. Check on irc?

It'll only kind of "work": if obj.elem does not exist, the expression value is undefined, which gets converted to false when used as an if condition. The problem is that such use is extremely impure and will result in an JavaScript strict warning "reference to undefined property obj.elem".
We do not want this.

That said, I actually think it'll much cleaner to actually check if we're (not)in the search dialog, i.e. 
  document.documentElement.id != "searchMailWindow"

And yes, I'm guilty of using Jens' solution myself - Jens, I'd be much obliged if you could correct my mistake as well. ;-)
Comment 10 Jens Hatlak (:InvisibleSmiley) 2010-12-18 08:29:55 PST
Created attachment 498519 [details] [diff] [review]
patch v2

While I was at it, I chose to make the two cases consistent, i.e. now middle clicking a search result works, too. Hope you like it. If you do, please decide whether this needs sr as well or moa is OK here.
Comment 11 Karsten Düsterloh 2010-12-19 09:24:03 PST
Comment on attachment 498519 [details] [diff] [review]
patch v2

>+    if (t.localName == "treechildren")
>     {
>+      if (AllowOpenTabOnMiddleClick() &&
>+          document.documentElement.id != "searchMailWindow")
>+      {   // we don't allow new tabs in the search dialog
>         MsgOpenNewTabForMessage();
>         RestoreSelectionWithoutContentLoad(GetThreadTree());
>       }
>+      else
>+      {
>+        MsgOpenSelectedMessages();
>+      }
>       return;

(In reply to comment #10)
> While I was at it, I chose to make the two cases consistent, i.e. now middle
> clicking a search result works, too.

While that is nice indeed, your code results in a collateral inconsistency when *not* in the search dialog: up to now, with opentabfor.middleclick=false, middleclicking a folder or thread pane item would both behave just as left click. With your change, (only) the thread pane middle click would be altered to open the message in a standalone window.

> please decide whether this needs sr as well or moa is OK here

moa will suffice.
Comment 12 Jens Hatlak (:InvisibleSmiley) 2010-12-19 12:51:00 PST
Created attachment 498635 [details] [diff] [review]
patch v1a [Checkin: comments 14 and 16]
Comment 13 Karsten Düsterloh 2010-12-19 14:48:06 PST
Comment on attachment 498635 [details] [diff] [review]
patch v1a [Checkin: comments 14 and 16]

r/moa=me
Comment 14 Jens Hatlak (:InvisibleSmiley) 2010-12-20 09:31:22 PST
Comment on attachment 498635 [details] [diff] [review]
patch v1a [Checkin: comments 14 and 16]

http://hg.mozilla.org/comm-central/rev/beb0459250df
Comment 15 Jens Hatlak (:InvisibleSmiley) 2010-12-25 02:19:08 PST
Comment on attachment 498635 [details] [diff] [review]
patch v1a [Checkin: comments 14 and 16]

This bug was reported against SM 2.0, and the patch is very safe AFAICT.
Comment 16 Jens Hatlak (:InvisibleSmiley) 2010-12-25 10:39:19 PST
Comment on attachment 498635 [details] [diff] [review]
patch v1a [Checkin: comments 14 and 16]

http://hg.mozilla.org/releases/comm-1.9.1/rev/3c9e330a5165
(trunk patch didn't apply 100% cleanly, fixed upon check-in)

Note You need to log in before you can comment on or make changes to this bug.