1468080 - Missing padding between bookmark icon and folder text, in Add/Edit Bookmark popup

Status: VERIFIED FIXED

(Toolkit :: Themes, defect, P3)

Description

[Itiel]

Attachment: Screenshot

STR:
1. Open any webpage
2. Click the bookmark icon in the URL bar

AR:
Under "Folder", there's no padding between the bookmark icon and folder text.
This looks okay in the dropdown items though.

ER:
Padding between the text and icon, just like in the dropdown items.

See attached.

Comment 1

[Itiel]

This dates way back to even before Nightly 4.0b2pre on (and then I stopped bisecting it and looking for a regression window [if such even exists]).
As I see this popup is getting an upgrade, I thought it'd be nice to fix this while we're at it.

Comment 2

[Mark Banner (:standard8)]

Do you have a local userContent.css or userChrome.css file or something similar?

I'd be surprised if this was an issue that far back, as we've not had any other reports of it.

Comment 3

[Itiel]

Not that I'm aware of.
And besides, this is consistently occuring on clean installs.

Comment 4

[Mark Banner (:standard8)]

Which platform is this on? Please can you post the output of about:support and any other possible useful information like screen scaling sizes.

Moving to Themes, as I think that's more likely the issue area here.

Comment 5

[Itiel]

This happens on Windows 10 x86 RS4, Windows 8.1 x64.
The first is with 150% DPI, the latter is with 100% DPI.

Output of Nightly from the Windows 10 machine is attached.

Comment 6

[Itiel]

Attachment: about:support output

Comment 7

[Timea Cernea [:tbabos][inactive]]

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

I can reproduce the issue on latest Nightly 62.0a1 (2018-06-14), Firefox Beta 61.0b13 and Firefox Release 60.0.2. on Windows 10 x64.
On Mac OS and Ubuntu it looks fine, there is padding between the text and the icon. I also tried to run mozzregression on Nightly versions, however I couldn't get a regression range since the issue occurs even on Nightly 43.0a1 - 2015-09-12.

Comment 8

[Dão Gottwald [::dao]]

The problem is that we're setting the icon with list-style-image instead of the image attribute, and then this rule doesn't apply:

https://searchfox.org/mozilla-central/rev/88199de427d3c5762b7f3c2a4860c10734abd867/toolkit/themes/windows/global/menulist.css#30

This is a Windows-specific quirk, menulist.css handles this differently on Mac and Linux.

Comment 9

[Dão Gottwald [::dao]]

Attachment: Bug 1468080 - Clean up menulist icon and label spacing on Windows.

Review commit: https://reviewboard.mozilla.org/r/258094/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/258094/

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8993314 [details]
Bug 1468080 - Clean up menulist icon and label spacing on Windows.

https://reviewboard.mozilla.org/r/258094/#review265464

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/694fd83d0e63
Clean up menulist icon and label spacing on Windows. r=jaws

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/694fd83d0e63

Comment 13

[Dão Gottwald [::dao]]

Comment on attachment 8993314 [details]
Bug 1468080 - Clean up menulist icon and label spacing on Windows.

Approval Request Comment
[Feature/Bug causing the regression]: I'd like to fix this as part of the bookmark doorhanger refresh (bug 1459877), most of which ships in 62
[User impact if declined]: polish issue
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: not formally yet but I've checked that the fix works
[Needs manual test from QE? If yes, steps to reproduce]: see comment 0
[List of other uplifts needed for the feature/fix]: /
[Is the change risky?]: no
[Why is the change risky/not risky?]: this just makes the Windows menulist styling more consistent with that on Mac and Linux
[String changes made/needed]: /

Comment 14

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8993314 [details]
Bug 1468080 - Clean up menulist icon and label spacing on Windows.

CSS tweaks for bookmark tour, let's uplift to beta. Should end up in beta 12.

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/4af69471cde4

Comment 16

[Cristian Baica [:cbaica], Release Desktop QA]

I have managed to reproduce this issue on Windows 10 x64 using Firefox 62.0a1 buildID: 20180610100220.

The issue was verified using Firefox 63.0a1 buildID: 20180724223402 and a beta build (Fx 62.0b11 ID:20180724124430) available on treeherder, on Windows 10 x64, macOS 10.14 and Ubuntu 16.04 x64 LTS. The spacing was correctly added between the 'Other Bookmarks' text and the icon.