641090 - context/right-click menu "Search Google for" broken in popup windows, displays assertion failed dialog, And Firefox should open the new tab in a full browser window

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

[Steffen Wilberg]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110311 Firefox/4.0b13pre

The context menu "Search Google for" is broken in popup windows.

STR:
1. Open a popup without navigation toolbar and tab bar:
In the error console, evaluate this:
window.open("http://www.mozilla.org", "", "location=no")
2. In the popup window, select any text, right-click, and select "Search Google for" in the context menu.

Expected result: new tab opened either in the popup window (like in 3.6) or the main window.

Actual results:
1. Nothing.
2. When you close the supposedly single tab in the popup by pressing Ctrl+w, there's your search tab, hidden behind the one you just closed.
After a second or two, an alert dialog opens:
Assertion Failed
ASSERT: Giving up waiting for the tab closing animation to finish (bug 608589) Stack Trace: 0:([object XULElement],[object XULElement],0)
3. After closing that dialog, interacting with the search results page is broken in strange ways and throws several errors.

Comment 1

[Steffen Wilberg]

Attachment: testcase

Instead of step 1, open the testcase and click the link to open the popup.
Then proceed with step 2.

Comment 2

[Steffen Wilberg]

Being a release build, Firefox 4.0rc1 doesn't display the Assertion Failed dialog and reports to the error console instead.

Comment 3

[Alice0775 White]

And a hidden tab is created after doing search select.
Firefox should open the new tab in a full browser window.

I think this should block 2.0

Comment 4

[Dão Gottwald [:dao]]

Attachment: patch

I've also filed bug 641157

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Though this makes us obey the browser.tabs.loadBookmarksInBackground pref where we wouldn't before, which I think may be problematic. Maybe this should depend on the aInBackground changes from bug 610203's patch...

Comment 6

[Dão Gottwald [:dao]]

Nope, this uses browser.tabs.loadInBackground just like gBrowser.loadOneTab.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #6)
> Nope, this uses browser.tabs.loadInBackground just like gBrowser.loadOneTab.

Oh, I missed that loadOneTab checks the pref. It checks browser.tabs.loadInBackground, though, whereas with that patch won't we end up using browser.tabs.loadBookmarksInBackground (aFromChrome is true via openLinkIn)...

Comment 9

[Dão Gottwald [:dao]]

(In reply to comment #8)
> Oh, I missed that loadOneTab checks the pref. It checks
> browser.tabs.loadInBackground, though, whereas with that patch won't we end up
> using browser.tabs.loadBookmarksInBackground (aFromChrome is true via
> openLinkIn)...

openUILinkIn sets fromChrome, openLinkIn doesn't.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Quite true. I misread the patch. I hate it when I do that.

Comment 11

[Dão Gottwald [:dao]]

http://hg.mozilla.org/projects/cedar/rev/dced24264c95

Comment 12

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/dced24264c95

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.2a1pre) Gecko/20110326 Firefox/4.2a1pre

Comment 14

[Vlad [QA]]

Verified fixed on Mozilla/5.0 (Windows NT 6.1; rv:2.2a1pre) Gecko/20110404 Firefox/4.2a1pre