449359 - Middle-click on empty folder in Bookmarks Toolbar or Library creates empty new tab

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Rick]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1

This is very minor, but for thoroughness, I'll file it.
I also provided the (short) code to fix it below.

Points:
1. For the toolbar, this also occurs for any other kind of click that should produce a new tab, depending on how one's prefs are set up: Ctrl-left-click, Shift-left-click, Ctrl-shift-left-click.
2. For the library, only middle-clicks apply since Ctrl/Shift left-clicks are used for selection.
3. This includes folders that contain only other folders.
4. There is already code in the sidebar to prevent this from happening, so the toolbar and library should be consistent.

In fact, here is the self-tested code (in extension form).  I just did what the bookmarks sidebar does and put in a "hasChildURIs()" check before "openContainerNodeInTabs()" is called for each case.  Alternatively, you could put the check in "openContainerNodeInTabs()" itself (utils.js) which would take care of all the cases at once, though I haven't tried this.

1. For the Bookmarks Toolbar, change browser.js:

eval("BookmarksEventHandler.onClick ="+BookmarksEventHandler.onClick.toString().replace(
   /var target = aEvent.originalTarget;/,
   <><![CDATA[
    var target = aEvent.originalTarget;

    //RC Do not open blank new tab if container has nothing to open
    if (PlacesUtils.nodeIsContainer(target.node) && !PlacesUtils.hasChildURIs(target.node))
      return;
   ]]></>
));


2. For the Library, change places.js:

eval("PlacesOrganizer.onTreeClick ="+PlacesOrganizer.onTreeClick.toString().replace(
   /else if \(PlacesUtils.nodeIsContainer\(selectedNode\)\) {/,
   <><![CDATA[
      else if (PlacesUtils.nodeIsContainer(selectedNode)) {

        //RC Do not open blank new tab if container has nothing to open
        if (!PlacesUtils.hasChildURIs(selectedNode))
          return;
   ]]></>
));


Reproducible: Always

Steps to Reproduce:
1. Create empty folder (or contains only other folders), in BM toolbar or Library
2. Middle-click on it

Actual Results:  
Blank tab created.

Expected Results:  
Nothing should happen.

Comment 1

[Steve]

I can confirm this bug in last night's build (20080805). Confirm it affects the Bookmarks Menu and also in the Organize Bookmarks.
I am using Windows XP SP2
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/2008080504 Minefield/3.1a2pre

Bug 445301 and bug 260611 could be related (opening tabs in background)

Comment 2

[Steve]

In addition I noticed a small inconsistency.

When middle clicking to open tabs they all open in new tabs (leaving your current tab undisturbed) - but if you right click and select 'Open all in tabs' then your current tab is re-used.

Comment 3

[Dão Gottwald [:dao]]

Attachment: patch

Note that the fix for _openTabset would be sufficient to fix this particular bug, but I figure it's better to make tabbrowser's loadTabs less dumb as well.

Comment 4

[Marco Bonardo [:mak]]

Comment on attachment 357475 [details] [diff] [review]
patch

you need also a review from a browser peer, i'm forwarding to gavin

Comment 5

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/2da6386c98c0

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 357475 [details] [diff] [review]
patch

a191=beltzner

Comment 7

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d7e5f87ba8e7

Comment 8

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug's priority relative to the set of other polish bugs is:
P4 - Polish issue that is rarely encountered, and is easily identifiable.

Comment 9

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090714 Shiretoko/3.5.1pre (.NET CLR 3.5.30729) ID:20090714052038

and

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090714 Minefield/3.6a1pre (.NET CLR 3.5.30729) ID:20090714054201