When bookmarks button is on an autohide menubar it should stay on navbar

RESOLVED DUPLICATE of bug 581238

Status

()

Firefox
Bookmarks & History
RESOLVED DUPLICATE of bug 581238
8 years ago
5 years ago

People

(Reporter: mak, Unassigned)

Tracking

({regression})

Trunk
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [places-next-wanted][needs evaluation])

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
This was implemented through isElementVisible but that caused 2 weird issues:
- it was not consistent when opening new windows on Win
- it was causing button handling in pinstripe to not work at all

We need to find a better way to handle this special case without regressing pinstripe and being more consistent on win.
(Reporter)

Comment 1

8 years ago
remind this should be tested to be working correctly also with an empty bookmarks toolbar, that means bo.width will most likely be 0

Updated

8 years ago
Blocks: 575218
No longer depends on: 575218
Keywords: regression
http://hg.mozilla.org/mozilla-central/rev/fa7b6f39faf4 should be reverted when this gets fixed.
(Reporter)

Comment 3

8 years ago
that rule hides the button if the menubar is visible, that is something we always want, no?
(Reporter)

Comment 4

8 years ago
I think we should just check if bookmarks items element is on a bar with autohide=true, and if so don't move it to it, checking the attribute should be more reliable than using isElementVisible and won't cause reflows.
This means just adding an additional condition on the first if in updatePosition
(In reply to comment #3)
> that rule hides the button if the menubar is visible, that is something we
> always want, no?

Ah, yes, we probably still want that. I misread that code as #toolbar-menubar[autohide="true"] > ...
(Reporter)

Comment 6

8 years ago
Created attachment 454855 [details] [diff] [review]
patch v1.0

something like this, btw if we find something even faster, it will be fine.
(Reporter)

Comment 7

8 years ago
since menubar is the only toolbar with autohide we could even just check parentNode.id != "toolbar-menubar"
(Reporter)

Updated

7 years ago
Whiteboard: [places-next-wanted][needs evaluation]

Comment 8

5 years ago
Related to bug 581238 / bug 631330.
(Reporter)

Comment 9

5 years ago
just duping, the plan is bug 748894 btw.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 581238
You need to log in before you can comment on or make changes to this bug.