This is related to bug 238964. The suggestion in bug 238964 comment #3 did not work (it caused other side effects), so I'm not going to incorporate this into a patch for that bug.

This works, though I'm not entirely sure it's ideal. The overflow popup stays open, so you could middle-click multiple links conveniently, but if you only want to open one, you'd have to click away to get rid of the overflow popup. I don't think changing this would be ideal in other situations.

Do we really want this? When I fixed bug 72361 I intentionally left out the menus (see bug 72361 comment 45 and bug 72361 comment 60). Menus aren't supposed to react on anything but normal clicks (the command event). Firefox has an implementation for middle- and right-clicks on menus but this caused some problems and is IMHO an usability issue in itself. There is still Ctrl-Click to open a bookmark in a new tab from a menu...

I changed my mind about the scope of this bug :). New patch coming shortly.

Make main bookmarks menu, bookmarks on PTF, and bookmarks in PTF overflow all middle-clickable. Middle-clicking the top items ("Bookmark this page", "File bookmark", etc) does nothing, and doesn't give any JS errors, so this doesn't seem to break anything.

(In reply to comment #2) >When I fixed bug 72361 I intentionally left out the menus But (un?)fortunately the menus on the personal toolbar do react...

(In reply to comment #5) > (In reply to comment #2) > >When I fixed bug 72361 I intentionally left out the menus > But (un?)fortunately the menus on the personal toolbar do react... Either we should remove that (please don't!), or we should make the overflow area behave the same way. I don't know whether or not the bookmarks menus should respond to middle clicks, but I don't see any downside. The one bookmarks menu that lives in the PTF probably should be consistent with the rest of the PTF, but in my opinion the main bookmarks menu doesn't *have* to behave exactly the same way (though as I said, I see no downside).

Jag? Timeless? Anyone else disagree with comment #2? I cc'ed you to hopefully get opinions from other xpfe people ;)

i expect to be able to right click on bookmarks menus, this isn't unusual behavior. but it will almost never work on macosx.

Sorry for the late response, I had lots to do. Ok, looking at the other responses, seems that I am outnumbered. I noticed three issues with this patch so far: 1. "Error: uncaught exception: 2147500035" when middle-clicking a bookmark folder (for some reason Ctrl-Click on the folder doesn't throw an exception). 2. The bookmarks menu doesn't close after a middle-click. Patch to bug 72361 backed out a function called loadBookmarkMiddleClick() that used BookmarksMenuDNDObserver.onDragCloseTarget() to close the menu. I think this is a very hacky way to do it so we shouldn't revive this function. Instead we might insert following code into BookmarksUtils.loadBookmarkBrowser(): // Close any open popups if (aEvent.type == "click" && aEvent.button == 1) { var node = aEvent.target; while (node) { if (node.nodeType == node.ELEMENT_NODE && node.tagName == "menupopup") node.hidePopup(); node = (node == node.parentNode ? null : node.parentNode); } } 3. The overflow menu for the personal toolbar hasn't been fixed. Isn't that what this bug was originally about? FYI, the overflow menu appears on the right side if the browser window isn't wide enough for all personal toolbar items to fit into it.

(In reply to comment #9) > 1. "Error: uncaught exception: 2147500035" when middle-clicking a bookmark > folder (for some reason Ctrl-Click on the folder doesn't throw an exception). WFM - do you get a line number? > 2. The bookmarks menu doesn't close after a middle-click. I thought about it, and did it by design. This way, I can open the menu and open garfield, dilbert, and foxtrot with 4 clicks instead of 6. Maybe that should get a pref (since I can imagine many people wouldn't like this). > 3. The overflow menu for the personal toolbar hasn't been fixed. Isn't that what > this bug was originally about? WFM - as you noted, that was the main point of this bug. I just diffed my tree, and I still match the patch as far as I can tell, so I'm not sure why it doesn't work for you. What OS are you using?

(In reply to comment #10) > 1. WFM - do you get a line number? No, that's the whole error message. Maybe I should try a more recent nightly... > 2. I thought about it, and did it by design. This way, I can open the menu and > open garfield, dilbert, and foxtrot with 4 clicks instead of 6. Maybe that > should get a pref (since I can imagine many people wouldn't like this). A menu definitely needs to close when you click something. Here, the annoyance of closing the menu manually (which is a non-standard behaviour, thus even more annoying) by far outweights the advantages in a rather uncommon case where you want to open more than one bookmark. If you want to open a dozen of bookmarks - that's what the Bookmarks dialog is good for. > 3. WFM - as you noted, that was the main point of this bug. Yes, it works, my fault...

Make menus close once you middle-click a bookmark.

Comment on attachment 169314 [details] [diff] [review] patch I don't think menuitem has a property open. This doesn't work.

Hopefully I got it right this time. Neil wanted me to close the menus before opening the bookmarks, so this code does that. For the main bookmarks menu, I used the same method as Firefox (drag'n'drop to close menus).

Comment on attachment 169608 [details] [diff] [review] Fixed patch >+ onclick="if (event.button == 1) {this.open=false; BookmarksUtils.loadBookmarkBrowser(event, this.database);}"> I doubt this works for nested folders. Mind you I feel sure that there exist cases where onDragCloseTarget doesn't look as if it does what at first glance it should.

(In reply to comment #15) > (From update of attachment 169608 [details] [diff] [review] [edit]) > >+ onclick="if (event.button == 1) {this.open=false; BookmarksUtils.loadBookmarkBrowser(event, this.database);}"> > I doubt this works for nested folders. Has anyone had the opportunity to look into this? As far as I can tell, it does in fact work.

(In reply to comment #16) > Has anyone had the opportunity to look into this? As far as I can tell, it does > in fact work. As I already wrote - I tried it and |this.open = false| doesn't work at all, why should it? Look for leftovers of previous experiments in your tree... And again, it is IMHO a very bad idea to revive loadBookmarkMiddleClick() in this form - using onDragCloseTarget() here really is bad style. The code from comment 9 should be more obvious and reliable.

(In reply to comment #17) > (In reply to comment #16) > > Has anyone had the opportunity to look into this? As far as I can tell, it does > > in fact work. > > As I already wrote - I tried it and |this.open = false| doesn't work at all, why > should it? Look for leftovers of previous experiments in your tree... I pulled a clean tree and applied attachment 169608 [details] [diff] [review], and everything seems to work. If it's not working for you, I need access to whatever platform you're testing on. I will consider eliminating the use of onDragCloseTarget, though that should be fixed for Firefox at the same time (I used it as a reference).

Comment on attachment 169608 [details] [diff] [review] Fixed patch As discussed on IRC, call loadBookmarkMiddleClick on all clicks and in there call hidePopup in a loop.

Closes popups as Wladimir suggested, and uses loadBookmarMiddleClick for everything.

timeless: it seems, you really were in a hurry to give r+ here... cst: + onclick="if (event.button == 1) BookmarksMenu.loadBookmarkMiddleClick(event, this.database);"> Please be consistent. Either you always check event.button or never. I would prefer you relying on the check in loadBookmarkMiddleClick() and removing the event.button check in all the other places. Other than that the last patch looks good.

Comment on attachment 172766 [details] [diff] [review] patch You'll need to patch the existing middle-click handler so that middle-clicks on ptf folder menuitems work. >+ // unlike for command events, we have to close the menus manually >+ var node = aEvent.target; >+ while (node) { >+ if (node.nodeType == node.ELEMENT_NODE && node.tagName == "menupopup") >+ node.hidePopup(); >+ >+ node = (node == node.parentNode ? null : node.parentNode); >+ } node is never going to equal its own parent. Also, you can stop the loop at aEvent.currentTarget (which you know never has any menupopup ancestors). And it would be better if you used a for loop instead of a while loop.

(In reply to comment #23) > node is never going to equal its own parent. Yes, I mixed it up with the parent propery. The correct loop will be then: for (var node = aEvent.target; node && node != aEvent.currentTarget; node = node.parentNode) { if (node.nodeType == node.ELEMENT_NODE && node.tagName == "menupopup") node.hidePopup(); }

Let's see how many more tries it takes me to get this right.

Checking in mozilla/xpfe/browser/resources/content/navigator.xul; /cvsroot/mozilla/xpfe/browser/resources/content/navigator.xul,v <-- navigator. xul new revision: 1.427; previous revision: 1.426 done Checking in mozilla/xpfe/browser/resources/content/navigatorOverlay.xul; /cvsroot/mozilla/xpfe/browser/resources/content/navigatorOverlay.xul,v <-- nav igatorOverlay.xul new revision: 1.307; previous revision: 1.306 done Checking in mozilla/xpfe/components/bookmarks/resources/bookmarksMenu.js; /cvsroot/mozilla/xpfe/components/bookmarks/resources/bookmarksMenu.js,v <-- bo okmarksMenu.js new revision: 1.21; previous revision: 1.20 done

So did this finally fix bug 33761?

*** Bug 33761 has been marked as a duplicate of this bug. ***

Verified FIXED with build 2005-03-03-05 on Windows XP using Seamonkey. A top-level click in the Bookmarks menu or in the Manage Bookmarks window will open the website URLs in the subfolder (the exception being that clicking a top-level folder which contains _multiple_ sub-levels will just open the default homepage). Looks good.

Hmm ... with Mozilla 1.8b2 2005031304 on WinNT4 it works for me only in the bookmark manager. Middle-Click (for a new tab) does not work with the bookmarks menu or the bookmark pulldown menu from the personal toolbar. Ctrl+Click does work for all. Can anyone confirm this?

Can anyone confirm comment #30?

middle-click on the bookmarks menu (and via personal toolbar) worksforme (opens in a new tab) with linux trunk 20050403 and 20050313

(In reply to comment #30) With current SeaMonkey Branch and Trunk builds on WinXP it works now for me. Thanks.