Closed
Bug 616919
Opened 14 years ago
Closed 14 years ago
With no windows open, choosing a bookmark from the menu does nothing.
Categories
(Firefox :: Bookmarks & History, defect)
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)
(deleted),
patch
|
asaf
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•14 years ago
|
Version: unspecified → Trunk
Comment 1•14 years ago
|
||
when you say close all windows, which windows are we talking about?
Reporter | ||
Comment 2•14 years ago
|
||
(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.
Reporter | ||
Comment 3•14 years ago
|
||
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.
Reporter | ||
Comment 4•14 years ago
|
||
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: --- → ?
Comment 5•14 years ago
|
||
How does this behave in 3.6?
Reporter | ||
Comment 6•14 years ago
|
||
(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.
Updated•14 years ago
|
blocking2.0: ? → betaN+
Comment 7•14 years ago
|
||
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•14 years ago
|
||
(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"
Assignee | ||
Comment 9•14 years ago
|
||
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•14 years ago
|
||
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 11•14 years ago
|
||
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)
Comment 12•14 years ago
|
||
yes probably should be better handled by bug 562998
Assignee | ||
Comment 13•14 years ago
|
||
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.
Updated•14 years ago
|
Whiteboard: [softblocker]
Assignee | ||
Comment 14•14 years ago
|
||
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•14 years ago
|
||
Mano has other blockers, if you wish to make a patch he can review it though.
Assignee | ||
Comment 16•14 years ago
|
||
loadOneOrMoreURIs now checks for the case where there is no open browser
window, so this patch has become quite simple.
Assignee | ||
Updated•14 years ago
|
Attachment #508982 -
Flags: review?(mano)
Assignee | ||
Updated•14 years ago
|
Attachment #500656 -
Attachment is obsolete: true
Comment 17•14 years ago
|
||
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+
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Keywords: checkin-needed
Updated•14 years ago
|
Whiteboard: [softblocker] → [softblocker][fx4-fixed-bugday]
You need to log in
before you can comment on or make changes to this bug.
Description
•