Increase the bookmarks toolbar's horizontal padding

VERIFIED FIXED in Firefox 57

Status

()

P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: dao, Assigned: johannh)

Tracking

Trunk
Firefox 57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 verified, firefox58 verified)

Details

(Whiteboard: [reserve-photon-visual][p4])

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
The left and right padding are currently 4px each (after bug 1369415 made this consistent across platforms), the spec says they should be 6px. This will however make bug 1352042 slightly more annoying in that you need to move the mouse a bit more from the screen edge to reach the outermost button. Seems like a step in the wrong direction. Stephen, should we really make this change?

Putting this straight into the reserve backlog since this doesn't feel very important.
Flags: needinfo?(shorlander)
Flags: qe-verify?
Priority: -- → P3
(Reporter)

Updated

2 years ago
Flags: qe-verify? → qe-verify+
QA Contact: brindusa.tot
(Assignee)

Updated

2 years ago
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Priority: P3 → P1
Iteration: --- → 57.2 - Aug 29
Comment hidden (mozreview-request)
(Reporter)

Comment 3

2 years ago
mozreview-review
Comment on attachment 8897811 [details]
Bug 1388699 - Increase the bookmark toolbar padding per Photon spec.

https://reviewboard.mozilla.org/r/169106/#review174438

::: browser/themes/shared/browser.inc.css:25
(Diff revision 2)
>  
>  #navigator-toolbox > toolbar:not(#toolbar-menubar):not(#TabsToolbar):not(#nav-bar) {
>    overflow: -moz-hidden-unscrollable;
>    max-height: 4em;
>    transition: min-height 170ms ease-out, max-height 170ms ease-out;
> -  padding: 0 4px 1px;
> +  padding: 0 6px 2px;

This adds extra bottom padding on Linux and Windows.
Attachment #8897811 - Flags: review-
(In reply to Dão Gottwald [::dao] from comment #0)
> The left and right padding are currently 4px each (after bug 1369415 made
> this consistent across platforms), the spec says they should be 6px. This
> will however make bug 1352042 slightly more annoying in that you need to
> move the mouse a bit more from the screen edge to reach the outermost
> button. Seems like a step in the wrong direction. Stephen, should we really
> make this change?
> 
> Putting this straight into the reserve backlog since this doesn't feel very
> important.

Are we just talking about the left and right padding on the toolbar itself? Or on the bookmark items?

I don't think 8px is going to make be a significant difference from 4px WRT to the infinite hit area on the left or right. Any non-interactive start / end spacing will break that.
Flags: needinfo?(shorlander)
(Reporter)

Comment 5

2 years ago
(In reply to Stephen Horlander [:shorlander] from comment #4)
> (In reply to Dão Gottwald [::dao] from comment #0)
> > The left and right padding are currently 4px each (after bug 1369415 made
> > this consistent across platforms), the spec says they should be 6px. This
> > will however make bug 1352042 slightly more annoying in that you need to
> > move the mouse a bit more from the screen edge to reach the outermost
> > button. Seems like a step in the wrong direction. Stephen, should we really
> > make this change?
> > 
> > Putting this straight into the reserve backlog since this doesn't feel very
> > important.
> 
> Are we just talking about the left and right padding on the toolbar itself?
> Or on the bookmark items?

Both contribute to the problem.

> I don't think 8px is going to make be a significant difference from 4px WRT
> to the infinite hit area on the left or right. Any non-interactive start /
> end spacing will break that.

Sure, it already is broken. Still, the more space there is, the more annoying it is to move the cursor over to the element you intended to click.

The more fundamental question would be whether we shouldn't rather unbreak what's already broken by reverting bug 1294885. Nobody seems to have complained about the missing padding over many years, so it's not really clear to me why we should prioritize this over Fitts' Law.
(Reporter)

Updated

2 years ago
Blocks: 1391625
No longer depends on: 1391625
Iteration: 57.2 - Aug 29 → 57.3 - Sep 19
(Reporter)

Updated

2 years ago
Duplicate of this bug: 1399047
Are we going to get this into 57?
Flags: needinfo?(jhofmann)
(Assignee)

Comment 8

2 years ago
(In reply to Jim Mathies [:jimm] from comment #7)
> Are we going to get this into 57?

Yeah, sorry for the delay, I'll move the horizontal padding to bug 1391593 to avoid the controversy.
Flags: needinfo?(jhofmann)
Comment hidden (mozreview-request)

Comment 10

2 years ago
mozreview-review
Comment on attachment 8897811 [details]
Bug 1388699 - Increase the bookmark toolbar padding per Photon spec.

https://reviewboard.mozilla.org/r/169106/#review184980
Attachment #8897811 - Flags: review?(nhnt11) → review+

Comment 11

2 years ago
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7819af073f1c
Increase the bookmark toolbar padding per Photon spec. r=nhnt11
https://hg.mozilla.org/mozilla-central/rev/7819af073f1c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Posted image a.bmp
I verify this issue on Windows 10 x64 and in my case i have 4px to the left and 4px to the right padding. 
Please tell me, which are according to specifications?
Flags: needinfo?(shorlander)
(In reply to Valentina Claudia Ona from comment #13)
> Created attachment 8909655 [details]
> a.bmp
> 
> I verify this issue on Windows 10 x64 and in my case i have 4px to the left
> and 4px to the right padding. 
> Please tell me, which are according to specifications?

Yes, 4px is correct.
Flags: needinfo?(shorlander)
Verified as fixed on Nightly 58.0a1  on Windows 10 x64.

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0

Build ID 20170925220207

Based on comment #14, I will mark this as verified fixed.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
Flags: qe-verify+
(Reporter)

Updated

2 years ago
status-firefox57: verified → fixed
status-firefox58: --- → verified
Flags: qe-verify+
I verified this issue on Ubuntu 16.04, Windows 7 x64, Mac OS X 10.12, using Latest Nightly 58.0a1 and Latest Firefox Beta 57.0b4, with Build ID 20170928180207.
I will mark this as verified fixed.
status-firefox57: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.