545420 - E-mails found with advanced search cannot be opened by double click [RestoreSelectionWithoutContentLoad is not defined]

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: General, defect)

Description

[Andy Boze]

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)]

Anything in the Error Console (Tools/Web Development)?

Comment 2

[Andy Boze]

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]

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]

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]

> 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)]

Attachment: patch

Like this? (Kudos to Ratty, though I would probably have found that myself ;-))

Comment 7

[Philip Chee]

> +           "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]

>   tabmail.MsgOpenNewTabForMessage();
>   tabmail.RestoreSelectionWithoutContentLoad(GetThreadTree());
Ugh I don't think this will work.

Comment 9

[Karsten Düsterloh]

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)]

Attachment: 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]

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)]

Attachment: patch v1a [Checkin: comments 14 and 16]

Comment 13

[Karsten Düsterloh]

Comment on attachment 498635 [details] [diff] [review]
patch v1a [Checkin: comments 14 and 16]

r/moa=me

Comment 14

[Jens Hatlak (:InvisibleSmiley)]

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)]

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)]

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)