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

Status: RESOLVED DUPLICATE of bug 581238

(Firefox :: Bookmarks & History, defect)

Description

[Marco Bonardo [:mak]]

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.

Comment 1

[Marco Bonardo [:mak]]

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

Comment 2

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/fa7b6f39faf4 should be reverted when this gets fixed.

Comment 3

[Marco Bonardo [:mak]]

that rule hides the button if the menubar is visible, that is something we always want, no?

Comment 4

[Marco Bonardo [:mak]]

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

Comment 5

[Dão Gottwald [:dao]]

(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"] > ...

Comment 6

[Marco Bonardo [:mak]]

Attachment: patch v1.0

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

Comment 7

[Marco Bonardo [:mak]]

since menubar is the only toolbar with autohide we could even just check parentNode.id != "toolbar-menubar"

Comment 8

[[:Aleksej]]

Related to bug 581238 / bug 631330.

Comment 9

[Marco Bonardo [:mak]]

just duping, the plan is bug 748894 btw.