Closed Bug 1468080 Opened 2 years ago Closed 2 years ago

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

Categories

(Toolkit :: Themes, defect, P3)

Unspecified
Windows
defect

Tracking

()

VERIFIED FIXED
mozilla63
Tracking Status
firefox61 --- wontfix
firefox62 --- verified
firefox63 --- verified

People

(Reporter: itiel_yn8, Assigned: dao)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Attached image 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.
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.
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.
Not that I'm aware of.
And besides, this is consistently occuring on clean installs.
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.
Component: Bookmarks & History → Theme
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.
Attached file about:support output (obsolete) —
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.
Priority: -- → P3
Blocks: 1459877
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.
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Component: Theme → Themes
OS: Unspecified → Windows
Product: Firefox → Toolkit
Attachment #8985134 - Attachment is obsolete: true
Comment on attachment 8993314 [details]
Bug 1468080 - Clean up menulist icon and label spacing on Windows.

https://reviewboard.mozilla.org/r/258094/#review265464
Attachment #8993314 - Flags: review?(jaws) → review+
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/694fd83d0e63
Clean up menulist icon and label spacing on Windows. r=jaws
https://hg.mozilla.org/mozilla-central/rev/694fd83d0e63
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Flags: qe-verify+
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]: /
Attachment #8993314 - Flags: approval-mozilla-beta?
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.
Attachment #8993314 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.