1122697 - [Linux Only] When opening a bookmark folder, no tabs are opened when the opening multiple tabs dialog warning is dismissed

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[u347682]

User Agent: Mozilla/5.0 (X11; Linux i686; rv:35.0) Gecko/20100101 Firefox/35.0 SeaMonkey/2.32
Build ID: 20150112202541

Steps to reproduce:

User agent: Mozilla/5.0 (X11; Linux i686; rv:35.0) Gecko/20100101 Firefox/35.0 SeaMonkey/2.32
Build identifier: 20150112202541

Create a bookmarks folder with more than browser.tabs.maxOpenBeforeWarn bookmarks in it.

Right-click on folder --> "Open All in Tabs" or middle-click on folder.


Actual results:

"Confirm open" dialog appears ("You are about to open ... tabs. ...").

Press "Open tabs" button. --> Nothing happens.

Repeat and uncheck "Warn me..." checkbox. Click "Open tabs" button. --> Nothing happens.

Repeat. --> Tabs will be opened because the dialog has been disabled.

Tried a fresh profile, but it's still the same.


Expected results:

When clicking the "Open tabs" button in the warning dialog, all bookmarks in the folder should be opened in tabs.

Comment 1

[Philip Chee]

Try restarting SeaMonkey in safe mode. Go: Help -> Restart with Add-ons Disabled.

Comment 2

[therube]

WFM (Windows XP).

> fresh profile

No extensions installed?

If that's the case, disable all your Plugins & test again.

Comment 3

[u347682]

Looks like a Linux only problem. It is OK on Windows (tested with a virtual Windows XP).

But it does neither work with SeaMonkey 32-bit on Debian Wheezy, nor with the "unofficial" SeaMonkey 64-bit on Jessie.

When you hit "Open tabs" the dialog just closes, but no tabs of the bookmarks folder are opened.

I deleted the complete ".mozilla" directory (profile and all add-ons). Additionally I tried "Help -> Restart with Add-ons Disabled", but this does not change anything.

Comment 4

[u347682]

OK, some more info, although I really do not understand what is going on here:

On Linux, when I press "Open tabs" the error console will show

>> TypeError: focusManager.activeWindow is null
>> resource://gre/modules/PlacesUIUtils.jsm in line 610 .

So I put breakpoints in PlacesUIUtils.jsm on line 610 and 772 (to check the reallyOpen and buttonPressed vars).

But when I hit the breakpoints, reallyOpen *is* true, buttonPressed *is* 0, and focusManager.activeWindow *is not* null. So when I continue, all the tabs in the folder *do* open.

After removing the breakpoints, it stops working again.

Is this some sort of race condition?

Comment 5

[Philip Chee]

>   _getTopBrowserWin: function PUIU__getTopBrowserWin() {
>     return focusManager.activeWindow.gPrivate ||
>            Services.wm.getMostRecentWindow("navigator:browser");
>   },
Possible wallpaper fix:
-     return focusManager.activeWindow.gPrivate ||
+     return focusManager.activeWindow && focusManager.activeWindow.gPrivate ||

NEEDINFO->Neil

Comment 6

[[e-mail]]

(In reply to Philip Chee from comment #5)
> >   _getTopBrowserWin: function PUIU__getTopBrowserWin() {
> >     return focusManager.activeWindow.gPrivate ||
> >            Services.wm.getMostRecentWindow("navigator:browser");
> >   },
> Possible wallpaper fix:
> -     return focusManager.activeWindow.gPrivate ||
> +     return focusManager.activeWindow && focusManager.activeWindow.gPrivate ||

If focusManager.activeWindow is "broken" we should fix _getCurrentActiveWin and make _getTopBrowserWin use that.

Comment 7

[Philip Chee]

(In reply to [e-mail] from comment #6)
> If focusManager.activeWindow is "broken" we should fix _getCurrentActiveWin
> and make _getTopBrowserWin use that.
I'm drawing a blank on your "fix getCurrentActiveWin() and make _getTopBrowserWin use that". How about a suggestion?

Comment 8

[[e-mail]]

(In reply to Philip Chee from comment #7)
> (In reply to comment #6)
> > If focusManager.activeWindow is "broken" we should fix _getCurrentActiveWin
> > and make _getTopBrowserWin use that.
> I'm drawing a blank on your "fix getCurrentActiveWin() and make
> _getTopBrowserWin use that". How about a suggestion?
Sorry, I meant to say we should try getMostRecentWindow(null) either instead of or as well as focusManager.activeWindow.

Comment 9

[Philip Chee]

Attachment: Patch v1.0 Possible fix.

(In reply to [e-mail] from comment #8)
> Sorry, I meant to say we should try getMostRecentWindow(null) either instead
> of or as well as focusManager.activeWindow.
Since this bug manifests itself only on Linux I'm going for "as well as"

While investigating _getCurrentActiveWin() I noticed a bug in openNodeIn() which has been there since we converted history to places.
> -        this.openContainerNodeInTabs(aNode, aWhere);
> +      this.openContainerNodeInTabs(aNode, null, aWhere);

Comment 10

[[e-mail]]

(In reply to Philip Chee from comment #9)
> While investigating _getCurrentActiveWin() I noticed a bug in openNodeIn()
> which has been there since we converted history to places.
> > -        this.openContainerNodeInTabs(aNode, aWhere);
> > +      this.openContainerNodeInTabs(aNode, null, aWhere);
OK but that's not on beta right?

Comment 11

[[e-mail]]

Comment on attachment 8554737 [details] [diff] [review]
Patch v1.0 Possible fix.

Looks like we never actually call openNodeIn for a container node, so the problem never arises...

Comment 12

[Philip Chee]

Comment on attachment 8554737 [details] [diff] [review]
Patch v1.0 Possible fix.

[Approval Request Comment]
Regression caused by (bug #): N/A
User impact if declined: On Linux unable to open a bookmark folder into tabs after dismissing the multiple tabs dialog warning.
Testing completed (on m-c, etc.): Tested on comm-central.
Risk to taking this patch (and alternatives if risky): low to none. If this doesn't work users aren't any worse off.
String changes made by this patch: None.

Comment 13

[Philip Chee]

http://hg.mozilla.org/comm-central/rev/658d9430c6c1

Comment 14

[Philip Chee]

http://hg.mozilla.org/releases/comm-release/rev/3c4eaadf0fdd

Comment 15

[Philip Chee]

http://hg.mozilla.org/releases/comm-aurora/rev/e8b1aa433622
http://hg.mozilla.org/releases/comm-beta/rev/a1d89be16919