Adding a separator to bookmarks toolbar makes it too tall

RESOLVED FIXED in Firefox 52

Status

()

Firefox
Theme
P3
normal
RESOLVED FIXED
7 months ago
3 months ago

People

(Reporter: jlongster, Assigned: stefanh)

Tracking

unspecified
Firefox 52
Unspecified
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 months ago
A coworker has reproduced this in Nightly. Here is my bookmarks toolbar in Nightly:

http://jlongster.com/s/upload/ddcb19a2-7594-43ce-af0f-c848fd909c12.png

This looks correct. If I add a separator, however, it becomes way too tall:

http://jlongster.com/s/upload/9bd2280a-85cc-46a3-823e-a3c2fd4ea90b.png

Updated

7 months ago
Component: Toolbars and Customization → Theme

Updated

7 months ago
OS: Unspecified → Mac OS X
Priority: -- → P3
(Assignee)

Comment 1

7 months ago
Gijs, is bug 726132, comment #10 still valid? Or was that just about the tabs toolbar?
Flags: needinfo?(gijskruitbosch+bugs)

Comment 2

7 months ago
(In reply to Stefan [:stefanh] from comment #1)
> Gijs, is bug 726132, comment #10 still valid? Or was that just about the
> tabs toolbar?

That's about customizable toolbar items. The bookmarks toolbar items is a bookmarks container, which is governed by other code, which apparently lets you insert separators (which are menuseparators in the menu, for instance).
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 3

7 months ago
Created attachment 8808744 [details] [diff] [review]
Remove hardcoded toolbarseparator min-height

So it appears that the min-height used to be needed when the toolbars had '-moz-box-align: center' (bug 583510). Despite an ambitious archeological investigation I haven't find out when this was removed, but toolbars are now back to stretching, so we can remove the min-height.
Assignee: nobody → stefanh
Status: NEW → ASSIGNED
Attachment #8808744 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Updated

7 months ago
Attachment #8808744 - Attachment description: Remove hardcoded toolbarseparator height → Remove hardcoded toolbarseparator min-height

Comment 4

7 months ago
Comment on attachment 8808744 [details] [diff] [review]
Remove hardcoded toolbarseparator min-height

Review of attachment 8808744 [details] [diff] [review]:
-----------------------------------------------------------------

wah. Huh, OK. Looks OK to me.
Attachment #8808744 - Flags: review?(gijskruitbosch+bugs) → review+

Comment 5

7 months ago
Pushed by stefanh@inbox.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/62b32efde227
Remove toolbarseparator min-height in browser.css since it affects the toolbar height. r=Gijs.

Comment 6

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/62b32efde227
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
I tested this on Fx 52.0b7 on Ubuntu. The problem does not reproduce anymore.
QA Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.