Closed
Bug 1429857
Opened 7 years ago
Closed 7 years ago
Bookmarks list appears double-spaced
Categories
(Firefox :: Theme, defect)
Tracking
()
RESOLVED
FIXED
Firefox 59
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox57 | --- | unaffected |
firefox58 | --- | unaffected |
firefox59 | --- | fixed |
People
(Reporter: bgrins, Assigned: bgrins)
References
Details
(Keywords: regression)
Attachments
(1 file)
+++ This bug was initially created as a clone of Bug #1420229 +++
STR:
Open Customize and drag the Bookmark Icon to the NavBar
Open the bookmarks list and notice that its double-spaced and stretched
20171122220056 960f50c2e0a991ab2ab313132e69fb2c96cb7866 Good
20171123100420 b6bed1b710c3e22cab49f22f1b5f44d80286bcb9 Bad
Mozregress points to:
Bug 1416493
Assignee | ||
Comment 1•7 years ago
|
||
This is a clone as per https://bugzilla.mozilla.org/show_bug.cgi?id=1420229#c52, to handle the visual regression for Firefox 59. Screenshots of the issue can be seen in Bug 1420229.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•7 years ago
|
||
Dão, this fixes the UI regression. I don't love the patch, but I notice there are other places already in this file where we are specifically overriding menu.css so it doesn't feel too out of place. In those cases we appear to be using !important so I'd be happy to switch to that if you'd prefer (instead of adding .subviewbutton onto each selector).
After debugging this I feel more confident about the plan to deprioritize these XBL sheets as UA sheets instead of loading them as doc styles and reworking specificity during the process of replacing bindings - it would be a ton of work for each one to do this properly.
Updated•7 years ago
|
Component: Bookmarks & History → Theme
Assignee | ||
Comment 4•7 years ago
|
||
We could also prefix these with `#BMB_bookmarksPopup` if we want to target the fix to that single piece of UI
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8942321 [details]
Bug 1429857 - Override -moz-appearance on bookmarks popup subviewbuttons, and styles on menuitem-iconic-left;
https://reviewboard.mozilla.org/r/212588/#review218998
I haven't tested this, but rs+ since this is supposed to be a temporary hack.
::: browser/themes/shared/customizableui/panelUI.inc.css:1267
(Diff revision 1)
> + padding-top: 0;
> + -moz-appearance: none;
> +}
> +
> +menuitem.subviewbutton,
> +menuitem[type="checkbox"].subviewbutton,
This is probably already covered by menuitem.subviewbutton...
::: browser/themes/shared/customizableui/panelUI.inc.css:1269
(Diff revision 1)
> +}
> +
> +menuitem.subviewbutton,
> +menuitem[type="checkbox"].subviewbutton,
> +menuitem[type="checkbox"].subviewbutton > .menu-iconic-left,
> +menuitem.subviewbutton > .menu-iconic-left > .menu-iconic-icon {
.menu-iconic-icon shouldn't have -moz-appearance in the first place
Attachment #8942321 -
Flags: review?(dao+bmo) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #5)
> Comment on attachment 8942321 [details]
> Bug 1429857 - Override -moz-appearance on menuitem and descendants, and
> padding on menuitem-iconic-left;
>
> https://reviewboard.mozilla.org/r/212588/#review218998
>
> I haven't tested this, but rs+ since this is supposed to be a temporary hack.
>
> ::: browser/themes/shared/customizableui/panelUI.inc.css:1267
> (Diff revision 1)
> > + padding-top: 0;
> > + -moz-appearance: none;
> > +}
> > +
> > +menuitem.subviewbutton,
> > +menuitem[type="checkbox"].subviewbutton,
>
> This is probably already covered by menuitem.subviewbutton...
I believe it was needed because some rule in menu.css for type=checkbox had more specificity than `menuitem.subviewbutton`. I went back to revisit this and pushed up a change that is simpler and targets the bookmarks popup specifically. The only selector needed in this rule is now `#BMB_bookmarksPopup .subviewbutton`, which covers all cases.
> ::: browser/themes/shared/customizableui/panelUI.inc.css:1269
> (Diff revision 1)
> > +}
> > +
> > +menuitem.subviewbutton,
> > +menuitem[type="checkbox"].subviewbutton,
> > +menuitem[type="checkbox"].subviewbutton > .menu-iconic-left,
> > +menuitem.subviewbutton > .menu-iconic-left > .menu-iconic-icon {
>
> .menu-iconic-icon shouldn't have -moz-appearance in the first place
You are right - removed this from the list.
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/05bab8e59cd1
Override -moz-appearance on bookmarks popup subviewbuttons, and styles on menuitem-iconic-left;r=dao
Comment 9•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Updated•7 years ago
|
status-firefox57:
--- → unaffected
status-firefox58:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Updated•7 years ago
|
QA Whiteboard: [good first verify]
Comment 10•7 years ago
|
||
I have reproduced this bug on nightly according to (2018-01-11)
Fixing bug is verified on Latest Beta--
Build ID :20180208193705
User Agent :Mozilla/5.0 (Windows NT 6.1; rv:59.0) Gecko/20100101 Firefox/59.0
Tested OS-- Windows7 32bit
QA Whiteboard: [good first verify] → [bugday-20180214]
Comment 11•7 years ago
|
||
I Have tried to Reproduce the Same Bug in Firefox Nightly 60.0a1 (2018-02-13)
Tested OS: Windows 10 (64bit) & Windows 7 (32bit)
------------------------------------------------------------------------------------------
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20180212100154
Firefox Version: 60.0a1
------------------------------------------------------------------------------------------
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20180211220056
Firefox Version: 60.0a1
------------------------------------------------------------------------------------------
The bug is verified on Latest Beta too
[Steps to Reproduce]
1) Open the Firefox with Cleaned Profile
2) Click on View -> Toolbars -> Customize
3) Drag the Bookmark Icon to the NavBar
4) Open the Bookmarks list
Actual Results:
The Bookmarks list is displaying fine. there is no sort of stretching or Double-spaced
Status: Fixed & Verified
[bugday-20180214]
You need to log in
before you can comment on or make changes to this bug.
Description
•