Bookmarks toolbar is bigger when added a new icon on it

VERIFIED FIXED in Firefox 57

Status

()

P1
normal
VERIFIED FIXED
2 years ago
a year ago

People

(Reporter: Eddwardiq, Assigned: johannh)

Tracking

(Depends on: 1 bug, {regression, ux-consistency})

57 Branch
Firefox 57
regression, ux-consistency
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 disabled, firefox56 disabled, firefox57 verified)

Details

(Whiteboard: [reserve-photon-visual] )

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
Created attachment 8896791 [details]
Bookmarks toolbar size.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170813100233

Steps to reproduce:

1. Open the latest Nightly and enable Bookmarks toolbar
2. See height of the toolbar
3. Open Menu - Customization - add any Icon on the Bookmarks toolbar



Actual results:

See that toolbar is higher


Expected results:

Expected results are that Bookmarks toolbar height is not depending on the presence of any icon, manually added or not. 
Size should be consistent and depends only on the UI density option.
Regression observed after Bug 1377184 landed. 

For a comparison see attachment where I've checked all density options.

Behavior before Bug 1377184 was that all buttons on the Bookmarks toolbar have automatically shrinked to the size of the toolbar. 

I think this breaks UX consistency.
(Reporter)

Updated

2 years ago
Blocks: 1377184
Has STR: --- → yes
Component: Untriaged → Theme
Keywords: regression, ux-consistency
Whiteboard: [photon-visual][triage]

Updated

2 years ago
Status: UNCONFIRMED → NEW
status-firefox55: --- → disabled
status-firefox56: --- → disabled
status-firefox57: --- → affected
status-firefox-esr52: --- → unaffected
Ever confirmed: true
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1390851
(Assignee)

Updated

2 years ago
Flags: qe-verify+
Priority: -- → P4
Whiteboard: [photon-visual][triage] → [reserve-photon-visual]
QA Contact: brindusa.tot
Whiteboard: [reserve-photon-visual] → [reserve-photon-visual]

Updated

a year ago
Duplicate of this bug: 1386593

Updated

a year ago
Depends on: 1392931
See Also: bug 1396325
Duplicate of this bug: 1396325

Updated

a year ago
See Also: → bug 1395596

Updated

a year ago
Duplicate of this bug: 1396679

Updated

a year ago
Duplicate of this bug: 1397641
(Reporter)

Comment 7

a year ago
I know that you probably still have more urgent issues to solve, but is there any possibility to make this bug fixed for 57 ? Maybe it's not an issue visible on the first look, but still quite a big one. Duplicates are coming and I can imagine that with beta or even release audience will even more. 
My main concern here is, that with this bug the UI density options, especially the compact one, will be a bit of ruined since the compact UI will occupy the same screen space as the normal UI today.

Updated

a year ago
Duplicate of this bug: 1392931

Comment 9

a year ago
Given the frequency of reports, should we reconsider prioritizing this and/or bug 1388794 for 57? Maybe at least for the bookmarks overflow chevron case?
Flags: needinfo?(dao+bmo)
It's not a high priority for the time being because the bookmarks toolbar is disabled by default, and we usually don't switch between the two heights on the fly but only when the user has many bookmarks in the toolbar or adds another button.
Flags: needinfo?(dao+bmo)
(Reporter)

Comment 11

a year ago
Using pretty common Windows feature Snap causing higher Bookmarks toolbar as well, so I would say this will hit a lot if not all users pretty quickly. And since previous size will not come back until new session it will be definitely annoying.
(Assignee)

Updated

a year ago
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Priority: P4 → P1
Comment hidden (mozreview-request)
Iteration: --- → 57.3 - Sep 19

Comment 13

a year ago
mozreview-review
Comment on attachment 8909515 [details]
Bug 1389966 - Make toolbarbuttons in the bookmarks toolbar adjust to toolbar height.

https://reviewboard.mozilla.org/r/180988/#review186390

The !important with descendant selectors is a bit sad, but I agree this makes sense, probably moreso than the alternative.

::: browser/themes/shared/toolbarbuttons.inc.css:196
(Diff revision 1)
>  toolbarbutton.bookmark-item[open="true"],
>  toolbar .toolbarbutton-1:not([disabled=true]):-moz-any([open],[checked],:hover:active) > .toolbarbutton-icon,
>  toolbar .toolbarbutton-1:not([disabled=true]):-moz-any([open],[checked],:hover:active) > .toolbarbutton-text,
>  toolbar .toolbarbutton-1:not([disabled=true]):-moz-any([open],[checked],:hover:active) > .toolbarbutton-badge-stack {
>    background-color: var(--toolbarbutton-active-background);
>    transition-duration: var(--toolbarbutton-active-transition-duration);

It looks like I didn't notice we needed to remove this in bug 1393057. Can you remove it and the variable for it, please?

::: browser/themes/shared/toolbarbuttons.inc.css:273
(Diff revision 1)
> +/* To allow toolbarbuttons in the bookmarks toolbar to grow in
> + * height with the toolbar (like bookmark items), we apply background
> + * and padding to the buttons, not the button contents. This rule
> + * overrides attributes that would otherwise be duplicated. */

Nit: given that the rest of the styling is for 'normal' bookmark buttons, and that's what the header of this section says, can you move these 2 rules to the bottom?
Attachment #8909515 - Flags: review?(gijskruitbosch+bugs) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 15

a year ago
(In reply to :Gijs from comment #13)
> Comment on attachment 8909515 [details]
> Bug 1389966 - Make toolbarbuttons in the bookmarks toolbar adjust to toolbar
> height.
> 
> https://reviewboard.mozilla.org/r/180988/#review186390
> 
> The !important with descendant selectors is a bit sad, but I agree this
> makes sense, probably moreso than the alternative.

Yeah, I'm not terribly excited about this patch either, but it solves the problem fine, AFAICT.

Comment 16

a year ago
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fa4c0d777076
Make toolbarbuttons in the bookmarks toolbar adjust to toolbar height. r=Gijs

Comment 17

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fa4c0d777076
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57

Comment 18

a year ago
Tested on Mac OSX 10.12.5 Latest Nightly 57.0a1 Build ID 20170920100426
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:57.0) Gecko/20100101 Firefox/57.0 

The Bookmarks toolbar height is consistent and not depending of addition of new icons(24px). I will mark this as Verified Fixed. Thank you
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
Flags: qe-verify+

Updated

a year ago
Duplicate of this bug: 1401537

Updated

a year ago
Depends on: 1401823
You need to log in before you can comment on or make changes to this bug.