616919 - With no windows open, choosing a bookmark from the menu does nothing.

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Zandr Milewski [:zandr]]

User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101205 Firefox/4.0b8pre
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101205 Firefox/4.0b8pre

If I don't have any windows open, choosing a bookmark from the Bookmarks menu doesn't do anything.

Reproducible: Always

Steps to Reproduce:
1.Launch FF
2.Close All Windows
3.Choose a bookmark from the Bookmarks menu
Actual Results:  
Nothing

Expected Results:  
A window should open and load the selected bookmark.

Comment 1

[Marco Bonardo [:mak]]

when you say close all windows, which windows are we talking about?

Comment 2

[Zandr Milewski [:zandr]]

(In reply to comment #1)
> when you say close all windows, which windows are we talking about?

All of them. Every window. Nothing but dimmed 'Minimize' and 'Zoom' in the Window menu. Bounce on cmd-w until they're all gone.

I thought this was a bit of an odd question, so I tried a few things just now. You really have to have all windows closed. Having any window, even non-browser windows like Downloads, Preferences, or Error Console open will cause a new browser window to open when you select a bookmark. But if there are no windows of any kind open, choosing a bookmark does nothing.

Comment 3

[Zandr Milewski [:zandr]]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9pre) Gecko/20101221 Firefox/4.0b9pre ID:20101221030401

This is worse in the latest nightly. Now, choosing a bookmark when no windows are open will break the browser. Every option on every menu except the Minefield menu will be dimmed. Keyboard shortcuts beep. This requires restarting the browser to fix.

Comment 4

[Zandr Milewski [:zandr]]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9pre) Gecko/20101222 Firefox/4.0b9pre ID:20101222030351

No longer able to reproduce the behavior in #c3.

The main bug still exists, but it no longer breaks the entire browser.

Nominating, though, this is very annoying.

Comment 5

[Shawn Wilsher :sdwilsh]

How does this behave in 3.6?

Comment 6

[Zandr Milewski [:zandr]]

(In reply to comment #5)
> How does this behave in 3.6?

As described in "Expected results:" above.

Close all windows, choose a bookmark, new window opens.

Comment 7

[Stefan [:stefanh]]

It looks like this is because code in PlacesUIUtils.jsm relies on that we have a browser window (which we might not have on mac):

Error: this._getCurrentActiveWin() is null
Source File: resource:///modules/PlacesUIUtils.jsm
Line: 890

Comment 8

[Stefan [:stefanh]]

(In reply to comment #7)
> It looks like this is because code in PlacesUIUtils.jsm relies on that we have
> a browser window (which we might not have on mac):

Should probably read "a window open"

Comment 9

[Cameron McCormack (:heycam)]

Attachment: Make bookmark, "Home" and "Add-ons" menu items work when there are no other windows open

This makes bookmark menu items (including the "Open All in Tabs" ones), "History > Home" and "Tools > Add-ons" work when there are no windows open.

Mind you I don't really know what I'm doing in chrome JS, so let me know if the patch should be doing things another way.  (I mostly copied things from other menu item handlers that did work, and opened new windows.)  I don't actually understand what happens when there are no windows open, except that gBrowser is null.  I suspect that has something to do with the fact that the makeURI function wasn't available, either.

Comment 10

[Shawn Wilsher :sdwilsh]

Comment on attachment 500656 [details] [diff] [review]
Make bookmark, "Home" and "Add-ons" menu items work when there are no other windows open

Marco is a better candidate to review this code than I.

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 500656 [details] [diff] [review]
Make bookmark, "Home" and "Add-ons" menu items work when there are no other windows open

>--- a/browser/base/content/browser.js
>+++ b/browser/base/content/browser.js
>@@ -1936,9 +1936,12 @@
> 
>   // Home page should open in a new tab when current tab is an app tab
>   if (where == "current" &&
>-      gBrowser.selectedTab.pinned)
>+      gBrowser && gBrowser.selectedTab.pinned)
>     where = "tab";
> 
>+  if (!gBrowser)
>+    where = "window";

openUILinkIn should be doing this automatically.

> function switchToTabHavingURI(aURI, aOpenNew, aCallback) {
>   function switchIfURIInWindow(aWindow) {
>-    if (!("gBrowser" in aWindow))
>+    if (!aWindow.gBrowser)
>       return false;
>     let browsers = aWindow.gBrowser.browsers;
>     for (let i = 0; i < browsers.length; i++) {
>@@ -7996,8 +7999,9 @@
>   }
> 
>   // This can be passed either nsIURI or a string.
>-  if (!(aURI instanceof Ci.nsIURI))
>-    aURI = makeURI(aURI);
>+  if (!(aURI instanceof Ci.nsIURI)) {
>+    aURI = Services.io.newURI(aURI, null, null);
>+  }
> 
>   // Prioritise this window.
>   if (switchIfURIInWindow(window))
>@@ -8016,12 +8020,7 @@
> 
>   // No opened tab has that url.
>   if (aOpenNew) {
>-    if (isTabEmpty(gBrowser.selectedTab))
>-      gBrowser.selectedBrowser.loadURI(aURI.spec);
>-    else
>-      gBrowser.selectedTab = gBrowser.addTab(aURI.spec);
>-    if (aCallback) {
>-      let browser = gBrowser.selectedBrowser;
>+    function registerCallback(browser) {
>       browser.addEventListener("pageshow", function(event) {
>         if (event.target.location.href != aURI.spec)
>           return;
>@@ -8029,6 +8028,29 @@
>         aCallback(browser);
>       }, true);
>     }
>+
>+    if (!gBrowser) {
>+      // No open browser window, so create one.
>+      let sa = Cc["@mozilla.org/supports-array;1"].
>+               createInstance(Ci.nsISupportsArray);
>+      let wuri = Cc["@mozilla.org/supports-string;1"].
>+                 createInstance(Ci.nsISupportsString);
>+      wuri.data = aURI.spec;
>+      sa.AppendElement(wuri);
>+      let w = Services.ww.openWindow(null,
>+                                     "chrome://browser/content/browser.xul",
>+                                     null, "chrome,dialog=no,all", sa);
>+      w.addEventListener("load", function(event) {
>+        registerCallback(w.document.getElementById("content").selectedBrowser);
>+      }, true);
>+    } else {
>+      if (isTabEmpty(gBrowser.selectedTab))
>+        gBrowser.selectedBrowser.loadURI(aURI.spec);
>+      else
>+        gBrowser.selectedTab = gBrowser.addTab(aURI.spec);
>+      if (aCallback)
>+        registerCallback(gBrowser.selectedBrowser);
>+    }
>     return true;
>   }

This doesn't belong in this bug.

This bug appears to overlap with bug 562998 too...

Comment 12

[Marco Bonardo [:mak]]

yes probably should be better handled by bug 562998

Comment 13

[Cameron McCormack (:heycam)]

The "Tools > Add-ons" part is being handled in bug 565667.

Will the patch in bug 562998 also handle the "History > Home" part?  If so, then I think we can just forget this patch RESO DUPL this bug.

Comment 14

[Cameron McCormack (:heycam)]

It looks like bug 562998 didn't do anything about History > Home.  Mano, did you want to take it, or should I pick this patch back up?

Comment 15

[Marco Bonardo [:mak]]

Mano has other blockers, if you wish to make a patch he can review it though.

Comment 16

[Cameron McCormack (:heycam)]

Attachment: Make "History > Home" work on Mac if no browser windows are open.

loadOneOrMoreURIs now checks for the case where there is no open browser
window, so this patch has become quite simple.

Comment 17

[Asaf Romano (gone)]

Comment on attachment 508982 [details] [diff] [review]
Make "History > Home" work on Mac if no browser windows are open.

This will do for now. r=mano.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/133443c09564

Comment 19

[Cameron Dawson [:camd]]

Works now.  Verified.