Closed Bug 941051 Opened 11 years ago Closed 11 years ago

Clicking the bookmarks dropdown sometimes opens an item in the dropdown (a bookmark, the bookmarks sidebar, or the library window), rather than just showing the dropdown

Categories

(Firefox :: Toolbars and Customization, defect)

Product:
Firefox
Component:
Toolbars and Customization
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 29

People

(Reporter: Felipe, Assigned: enndeakin)

References

(Blocks 1 open bug,
URL
)

Details

(Whiteboard: [Australis:P2])

Attachments

(2 files)

bookmarks-menu.ogg
(deleted), video/ogg
Details
popupcenteronepixel
(deleted), patch
neil
: review+
Details | Diff | Splinter Review 
When I click on the Bookmarks dropdown icon, sometimes the Library window is invoked. It doesn't happen everytime. See http://screencast.com/t/SQeaA5jxzsTB

This is my currentset:
"urlbar-container,bookmarks-menu-button,search-container,tbtnJSShell,downloads-button,home-button,webrtc-status-button,social-toolbar-item,social-share-button"

And the DOM children of nav-bar-customization-target reflects the currentset correctly (i.e. there are no other unknown elements in between)

P.S. The bugzilla "Possible duplicates" brought up bug 406646, which seems very similar but is very old.
Moving the Bookmarks button seems to cause additional problems.

Placed in the top left corner before the tabs the right side of the bookmarks button is restoring the window up and down (same on a fresh profile with just the button moved to the same place). Replacing the the button to its default position (or using restore defaults) seems to fix the issue.

http://forums.mozillazine.org/viewtopic.php?p=13197389#p13197389
(In reply to anleck from comment #1)
> Moving the Bookmarks button seems to cause additional problems.
> 
> Placed in the top left corner before the tabs the right side of the
> bookmarks button is restoring the window up and down (same on a fresh
> profile with just the button moved to the same place). Replacing the the
> button to its default position (or using restore defaults) seems to fix the
> issue.
> 
> http://forums.mozillazine.org/viewtopic.php?p=13197389#p13197389

Please file a new bug for this issue.
P2 on figuring this out because this is weird and nondeterminism here makes no sense at all.
Whiteboard: [Australis:P2]
My theory is that this happens because the panel is shown on mousedown, and while the panel animation is being opened the mouseup ends up targeting the first entry in the menu and clicking it.

You can see in the screencast that on the times where the bug happened, the "Show All Bookmarks" is briefly highlighted on the menu, and that doesn't happen on the times when the bug didn't happen.


FWIW for some reason, i'm having a much harder time reproducing the bug today.
also consider we have this special code
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-places.js#1002
Could be overflowed property is bogus?
(In reply to :Felipe Gomes from comment #5)
> My theory is that this happens because the panel is shown on mousedown, and
> while the panel animation is being opened the mouseup ends up targeting the
> first entry in the menu and clicking it.
> 
> You can see in the screencast that on the times where the bug happened, the
> "Show All Bookmarks" is briefly highlighted on the menu, and that doesn't
> happen on the times when the bug didn't happen.
> 
> 
> FWIW for some reason, i'm having a much harder time reproducing the bug
> today.

Besides the Library window, clicking on the bookmarks button can also open up the bookmarks sidebar. This information was present in my original bug report on this issue (falsely marked as a duplicate IMO). That it is not just the library window that can get opened but also the bookmarks sidebar is very relevant information IMO.

Here's my complete bug report:

Steps to reproduce:

I upgraded to the latest Nightly version (28.0a1) (2013-11-18). Then I clicked on the button which is supposed to open my bookmarks in a drop down list.


Actual results:

The button to open my Bookmarks did not work reliably in the current Nightly (the first version with Australis enabled), the first ten times I used it. The first time I clicked it it opened my bookmarks library in a new window. Most of the times when I clicked this button it opened up my bookmarks library. Other times it opened up my bookmarks sidebar. And only 2-3 times out of the 10 times I clicked the button it indeed opened up the bookmarks drop down list that it is supposed to open.

Now that I have clicked the button ten times it does work reliably. It opens up the bookmarks drop down list everytime. But this is not good enough for me. The button should work reliably the first ten times it is pressed as well.


Expected results:

After upgrading to the latest Nightly, the bookmarks button should immediately work reliably and open the bookmarks in a drop down list. It should never open the bookmarks in a new library window and also never in a sidebar.
OS: Mac OS X → All
Attached video bookmarks-menu.ogg (deleted) — 
So the weird thing is I can't reproduce no matter what I try. I *can* reproduce the first item in the menu being hover-styled, but not getting it to be activated. Here's a screencast - you can see me holding the mouse by looking at the star button which has a pressed state (which shouldn't be activated by clicking this menu, but that's another bug).
Summary: Clicking the bookmarks dropdown sometimes open the Library window → Clicking the bookmarks dropdown sometimes opens an item in the dropdown (a bookmark, the bookmarks sidebar, or the library window), rather than just showing the dropdown
Gijs, another tidbit: instead of doing a long mouse press, try to do a mouse click as quick as possible. This way the mouseup event can happen while the pop-up is still opening, before it completes to slide down.

I think it's also directly related to how slow the menu open. So having many tabs open, and/or an old profile with hundreds of bookmarks like mine, helps.   Or if you can adjust the animation params to make the menu slide down more slowly that may help trigger it.
Two clues were given in this MZ thread:
http://forums.mozillazine.org/viewtopic.php?f=23&t=2777213&sid=f97ef225eb1e77c5cfc36de14f1628aa

1. deleted localstore.rdf - fixed the problem
2. One had moved the bookmarks button to left end of the address bar, when moved back to where it was the problem resolved itself.
We animate the panel's position when opening, right? (Ie, so it appears to slide down.)

Is is possible it's initially under the pointer, triggering the hover?

I can't seem to reproduce this, though.
I wonder if there's a clue in https://bugzilla.mozilla.org/show_bug.cgi?id=941409#c5 :

(In reply to Mark Hammond [:markh] from comment #5)
> TBH, I don't think this is a panel bug per-se, but related to the fact that
> http://mxr.mozilla.org/mozilla-central/source/browser/components/places/
> content/menu.xml#593 attempts to change the anchor in the popupshown event. 
> I could be wrong though (and it obviously wouldn't be the first time ;)

OTOH, that would be an adjustment of just a few pixels, so I don't see how that could be causing this to happen all over the place, and to various items... maybe dolske's guess in comment #12 is more likely?
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Hardware: x86 → All
Depends on: 899530
Now that bug 941409 has landed, are you able to reproduce this any more Felipe?
Flags: needinfo?(felipc)
I can still reproduce the problem.
http://hg.mozilla.org/mozilla-central/rev/7e79536aca0a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140129030602
Yeah, I can still reproduce it, intermittently. Not sure if related or not, but both the bookmarks and the menu panel always feel like they're opening twice with each click: they briefly blink full-size, then disappear, then reappear w/ the animation.
Flags: needinfo?(felipc)
Ok, I'm able to reproduce this reliably.

STR:

1) Have a bunch of bookmarks - enough to cause the bookmarks menu popup to have the scroll arrow at the bottom
2) Click and hold for about 1 second on the _lower half_ of the bookmarks menu button drop marker
3) Release the click

ER:  Menu should just open with no other action occurring

AR:  Top menu item (in my case, that's Show all Bookmarks) is selected immediately and the popup closes.
Hey Neil,

I think this is happening because the menupopup expands to the top boundary of the anchor once the menupopup overflows. After the stretch, the mouse is unluckily positioned near the top element in the menupopup, which receives the click event.

Is there a way we can prevent the menupopup from attempting to anchor at the top boundary of the anchor?
Flags: needinfo?(enndeakin)
Attached patch popupcenteronepixel (deleted) — — Splinter Review 
This patch changes the anchor width/height to be one pixel wide. This way the later call to IntersectRect doesn't think the anchor rectangle is empty. This should fix positioning issues with large arrow panels.

Also includes some fun rounding changes to fix the arrow panel test.
Assignee: mdeboer → enndeakin
Attachment #8367498 - Flags: review?(neil)
Flags: needinfo?(enndeakin)
Comment on attachment 8367498 [details] [diff] [review]
popupcenteronepixel

Seems reasonable if it works.

>-      is(Math.round(panelRect.top), Math.round(anchorRect.top * zoomFactor +  + adj), "anchored on top");
>+      is(Math.round(panelRect.top), Math.round(Math.round(anchorRect.top) * zoomFactor +  + adj), "anchored on top");
>-      is(Math.round(panelRect.bottom), Math.round(anchorRect.bottom * zoomFactor +  - adj), "anchored on bottom");
>+      is(Math.round(panelRect.bottom), Math.round(Math.round(anchorRect.bottom) * zoomFactor +  - adj), "anchored on bottom");
>-      is(Math.round(panelRect.right), Math.round(anchorRect.right * zoomFactor +  - adj), "anchored on right");
>+      is(Math.round(panelRect.right), Math.round(Math.round(anchorRect.right) * zoomFactor +  - adj), "anchored on right");
[What's with the extra " + "s in three of these cases?]
Attachment #8367498 - Flags: review?(neil) → review+
I'm able to reproduce this reliably in a clean profile by increasing the size of the bookmarks menu. With enough bookmarks the menu is incorrectly positioned (https://bugzilla.mozilla.org/attachment.cgi?id=8367755). With just the right number of menu items clicking on the bottom of the bookmark button brings up the library.

(While reliably reproducible, it is fragile. If I add a bookmark to the menu and then delete a bookmark I can't reproduce it --but deleting another menu item and adding one back restores the problem.)

(29 January Nightly on Ubuntu using XFCE, with my screen size and XFCE configuration.)
FYI, the patch fixes the problem for me!
I checked this in with some changes to the arrowpanel test to just use a within half-pixel check rather than precise check. This should reduce the likelihood of failure due to rounding.

https://hg.mozilla.org/integration/fx-team/rev/fe5ce0351d72
https://hg.mozilla.org/mozilla-central/rev/fe5ce0351d72
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
(In reply to Neil Deakin from comment #24)
> I checked this in with some changes to the arrowpanel test to just use a
> within half-pixel check rather than precise check. This should reduce the
> likelihood of failure due to rounding.
> 
> https://hg.mozilla.org/integration/fx-team/rev/fe5ce0351d72

Neil, you kinda forgot to re-add the actual assertion in `isWithinHalfPixel()`
You prolly wanted to do:
```js
ok(isWithinHalfPixel(a, b), "assertion description");
```
Flags: needinfo?(enndeakin)
Bah, no wonder it worked perfectly. I'll change and retest.
Flags: needinfo?(enndeakin)
Mozilla/5.0 (Windows NT 6.1; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140201074923 CSet: d09f9a9f81ae

I can still reproduce this.
really fixed ?
(In reply to pal-moz from comment #29)
> Mozilla/5.0 (Windows NT 6.1; rv:29.0) Gecko/20100101 Firefox/29.0
> ID:20140201074923 CSet: d09f9a9f81ae
> 
> I can still reproduce this.
> really fixed ?

There seems to be some inconsistency I think.

This bug only refers to re-opening tabs that are in 'recent closed tabs' list.
Left clicking on a recent-closed will open the link in a new tab and close the panel.
Mid-clicking will open the tab, but leave the panel visible.  (another bug I think, likely not filed)

Elsewhere in the History list
Left clicking a link will open tab in the 'current active', unlike left click from recent closed opening in a new tab, and the panel will close.
Mid-clicking a link from History will open in a new tab, and close the panel.

I think no matter what you open or how they open, left click, or mid-click should be consistent.

I hope I've stated these action correctly.. sort of confusing, really.
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #30)
> (In reply to pal-moz from comment #29)
> > Mozilla/5.0 (Windows NT 6.1; rv:29.0) Gecko/20100101 Firefox/29.0
> > ID:20140201074923 CSet: d09f9a9f81ae
> > 
> > I can still reproduce this.
> > really fixed ?
> 
> There seems to be some inconsistency I think.
> 
> This bug only refers to re-opening tabs that are in 'recent closed tabs'
> list.

Did you intend to comment in bug 948213? This bug is about the bookmarks panel...

> Left clicking on a recent-closed will open the link in a new tab and close
> the panel.
> Mid-clicking will open the tab, but leave the panel visible.  (another bug I
> think, likely not filed)

Please file this.

> Elsewhere in the History list
> Left clicking a link will open tab in the 'current active', unlike left
> click from recent closed opening in a new tab, and the panel will close.
> Mid-clicking a link from History will open in a new tab, and close the panel.
> 
> I think no matter what you open or how they open, left click, or mid-click
> should be consistent.
> 
> I hope I've stated these action correctly.. sort of confusing, really.

See also bug 966125.
(In reply to pal-moz from comment #29)
> Mozilla/5.0 (Windows NT 6.1; rv:29.0) Gecko/20100101 Firefox/29.0
> ID:20140201074923 CSet: d09f9a9f81ae
> 
> I can still reproduce this.
> really fixed ?

It should be. Please file a new bug with steps to reproduce if you still can.
(In reply to :Gijs Kruitbosch from comment #32)
> (In reply to pal-moz from comment #29)
> > Mozilla/5.0 (Windows NT 6.1; rv:29.0) Gecko/20100101 Firefox/29.0
> > ID:20140201074923 CSet: d09f9a9f81ae
> > 
> > I can still reproduce this.
> > really fixed ?
> 
> It should be. Please file a new bug with steps to reproduce if you still can.

just click bookmark menu button.
no other steps.
(In reply to :Gijs Kruitbosch from comment #31)
> 
> Did you intend to comment in bug 948213? This bug is about the bookmarks
> panel...
> 

Gay! yes.. sorry, old brain getting bugs mixed together.
Depends on: 966241
Neil, can you attach the (folded) patch to this bug here and request approval for m-a?

Thanks!
Flags: needinfo?(enndeakin)
Blocks: 961727
(In reply to Mike de Boer [:mikedeboer] from comment #37)
> Neil, can you attach the (folded) patch to this bug here and request
> approval for m-a?

The patches have already been uplifted there, including the one in bug 966241.
Flags: needinfo?(enndeakin)
I can still sometimes reproduce this.
When pressing the mouse over the menu icon, then dragging down while holding and releasing over an item in the list, that item gets opened. Sometimes that also happens accidentally when I just intended to open the menu.

The issue seems to be that:
a) There is an inconsistency in how panels are triggered. Bookmarks and the main menu use mousedown, all the others (history, devtools...) user mouseup/click.
b) Even between the main menu and the bookmarks, there is an inconsistency in that bookmarks panel items open on a simple mouseup, regardless of where the mousedown occurred.

I'm not sure if this is within the scope of this bug. If not I can open a new one…
(In reply to Philipp Sackl [:phlsa] from comment #39)
> I can still sometimes reproduce this.

Bug 973217

> When pressing the mouse over the menu icon, then dragging down while holding
> and releasing over an item in the list, that item gets opened. Sometimes
> that also happens accidentally when I just intended to open the menu.
> 
> The issue seems to be that:
> a) There is an inconsistency in how panels are triggered. Bookmarks and the
> main menu use mousedown, all the others (history, devtools...) user
> mouseup/click.
> b) Even between the main menu and the bookmarks, there is an inconsistency
> in that bookmarks panel items open on a simple mouseup, regardless of where
> the mousedown occurred.
> 
> I'm not sure if this is within the scope of this bug. If not I can open a
> new one…

bug 971260
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: