Closed Bug 616919 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox :: Bookmarks & History, defect)

x86
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: zandr, Assigned: heycam)

References

(Depends on 1 open bug)

Details

(Whiteboard: [softblocker][fx4-fixed-bugday])

Attachments

(1 file, 1 obsolete file)

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.
Version: unspecified → Trunk
when you say close all windows, which windows are we talking about?
(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.
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.
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.
blocking2.0: --- → ?
How does this behave in 3.6?
(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.
blocking2.0: ? → betaN+
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
(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"
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.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #500656 - Flags: review?(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.
Attachment #500656 - Flags: review?(sdwilsh) → review?(mak77)
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...
Attachment #500656 - Flags: review?(mak77)
yes probably should be better handled by bug 562998
Depends on: 562998
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.
Whiteboard: [softblocker]
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?
Mano has other blockers, if you wish to make a patch he can review it though.
loadOneOrMoreURIs now checks for the case where there is no open browser
window, so this patch has become quite simple.
Attachment #508982 - Flags: review?(mano)
Attachment #500656 - Attachment is obsolete: true
Depends on: 630255
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.
Attachment #508982 - Flags: review?(mano) → review+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/133443c09564
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Blocks: 603646
Works now.  Verified.
Status: RESOLVED → VERIFIED
Whiteboard: [softblocker] → [softblocker][fx4-fixed-bugday]
You need to log in before you can comment on or make changes to this bug.