Closed Bug 1446800 Opened 2 years ago Closed 2 years ago

Bookmarks menu items have small padding between icon and text

Categories

(Firefox :: Theme, defect)

59 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: joshas, Assigned: Paolo)

References

Details

(Keywords: regression)

Attachments

(4 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20180315233128

Steps to reproduce:

Open bookmarks menu by clicking "Show your bookmarks" icon.


Actual results:

Bookmark items and folders icons have smaller padding than in sidebar.


Expected results:

Bookmark icons in bookmark menu should have bigger padding, same as in sidebar.
Summary: Icons in bookmarks menu have smaller padding than in sidebar → Bookmarks menu items have small padding between icon and text
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0

I have tested this issue on Windows 10 x64 with the latest Firefox release (59.0.1) and the latest Nightly (61.0a1-20180321040527) and managed to reproduce it.

When checking the bookmark items and folders icons in sidebar and the ones from the bookmarks menu it seems that ones from the sidebar are smaller.

This seems to be more of an enhancement than an issue in my opinion. 
However, I am assigning a component to this issue in order to involve the development team and get an opinion on this.
Status: UNCONFIRMED → NEW
Component: Untriaged → Bookmarks & History
Ever confirmed: true
they are different kind of views, they don't necessarily need to appear identical or with the same padding.

(In reply to Emil Pasca [:emilpasca], Desktop Engineering QA from comment #1)
> When checking the bookmark items and folders icons in sidebar and the ones
> from the bookmarks menu it seems that ones from the sidebar are smaller.

I put them side by side in a screenshot, and they are identical for me.
If you have further evidence please post it.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → INVALID
I've attached overlayed Bookmark Menu versions from Firefox 58 and Firefox 59.0.2 . In Firefox 59 text moved towards icons by 3px. I've also found commit, which broke this layout: https://hg.mozilla.org/mozilla-central/diff/5c9a0be10b65/browser/themes/shared/customizableui/panelUI.inc.css#l1.295
.menu-iconic-left margin was changed from 3px to 0.

Probably the best way to fix this is to update Browser Menu component to new one (like one used in Library) to keep consistency.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Thank you for finding the regression point. Seems this is more theme related so moving it there.

Also needinfo to Paolo as he made that change.
Blocks: 1414244
Status: REOPENED → NEW
Component: Bookmarks & History → Theme
Flags: needinfo?(paolo.mozmail)
Keywords: regression
Thanks for reporting and finding the regression point! It makes sense to add a custom style for this menu, which as you noted is a special case for the moment, and at some point we hope to replace with something consistent with the other menus.
Flags: needinfo?(paolo.mozmail)
Blocks: 1457907
Note that this makes the footer slightly misaligned on Windows, and this is difficult to fix due to bug 1457907.

We may want to wait to land this bug until we can fix that issue, or we can land the new margin anyways considering that this is not part of the default user interface and the issue is minor.
Comment on attachment 8971974 [details]
Bug 1446800 - Bookmarks menu items have small padding between icon and text.

https://reviewboard.mozilla.org/r/240724/#review246736

Unfortunately I don't have a Windows build to hand to see what the footer looks like, and Windows builds are busted on treeherder. Mac looks fine though. I'd have thought the footer would look too bad though, so yes, we could perhaps land this and leave that until later. Maybe pass it by a UX person if you're not fully sure?
Attachment #8971974 - Flags: review?(standard8) → review+
I may just land this next cycle. There's progress being made in bug 1457907, so we'll probably figure out the best way to override the required rules then.
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/344ff948dc28
Bookmarks menu items have small padding between icon and text. r=Standard8
https://hg.mozilla.org/mozilla-central/rev/344ff948dc28
Status: NEW → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Target Milestone: Firefox 61 → Firefox 62
(In reply to :Paolo Amadini from comment #11)
> I may just land this next cycle. There's progress being made in bug 1457907,
> so we'll probably figure out the best way to override the required rules
> then.

Do you want to consider this wontfix for 61 then?
Flags: needinfo?(paolo.mozmail)
Yep. The padding has already been smaller for a few releases now, and it would be nice to align the footer at the same time once bug 1457907 is fixed.
Flags: needinfo?(paolo.mozmail)
You need to log in before you can comment on or make changes to this bug.