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)
Firefox
Toolbars and Customization
Tracking
()
RESOLVED
FIXED
Firefox 29
People
(Reporter: Felipe, Assigned: enndeakin)
References
(Blocks 1 open bug, )
Details
(Whiteboard: [Australis:P2])
Attachments
(2 files)
(deleted),
video/ogg
|
Details | |
(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
Comment 3•11 years ago
|
||
(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.
Comment 4•11 years ago
|
||
P2 on figuring this out because this is weird and nondeterminism here makes no sense at all.
Whiteboard: [Australis:P2]
Reporter | ||
Comment 5•11 years ago
|
||
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.
Comment 6•11 years ago
|
||
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?
Comment 7•11 years ago
|
||
(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.
Updated•11 years ago
|
OS: Mac OS X → All
Comment 8•11 years ago
|
||
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).
Updated•11 years ago
|
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
Reporter | ||
Comment 10•11 years ago
|
||
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.
Comment 11•11 years ago
|
||
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.
Comment 12•11 years ago
|
||
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.
Comment 13•11 years ago
|
||
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?
Updated•11 years ago
|
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Hardware: x86 → All
Comment 14•11 years ago
|
||
Now that bug 941409 has landed, are you able to reproduce this any more Felipe?
Flags: needinfo?(felipc)
Comment 15•11 years ago
|
||
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
Reporter | ||
Comment 16•11 years ago
|
||
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)
Comment 17•11 years ago
|
||
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.
Comment 18•11 years ago
|
||
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)
Assignee | ||
Comment 19•11 years ago
|
||
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.
Comment 20•11 years ago
|
||
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+
Comment 21•11 years ago
|
||
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.)
Reporter | ||
Comment 23•11 years ago
|
||
FYI, the patch fixes the problem for me!
Assignee | ||
Comment 24•11 years ago
|
||
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
Comment 25•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Comment 26•11 years ago
|
||
(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)
Assignee | ||
Comment 27•11 years ago
|
||
Bah, no wonder it worked perfectly. I'll change and retest.
Flags: needinfo?(enndeakin)
Comment 29•11 years ago
|
||
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 ?
Comment 30•11 years ago
|
||
(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.
Comment 31•11 years ago
|
||
(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.
Comment 32•11 years ago
|
||
(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.
Comment 33•11 years ago
|
||
(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.
Comment 34•11 years ago
|
||
(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.
Assignee | ||
Comment 35•11 years ago
|
||
Checked in the fix for comment 26:
https://hg.mozilla.org/integration/fx-team/rev/7238893e4faa
Comment 36•11 years ago
|
||
Comment 37•11 years ago
|
||
Neil, can you attach the (folded) patch to this bug here and request approval for m-a?
Thanks!
Flags: needinfo?(enndeakin)
Assignee | ||
Comment 38•11 years ago
|
||
(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)
Comment 39•11 years ago
|
||
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…
Comment 40•11 years ago
|
||
(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.
Description
•