Closed
Bug 1377655
Opened 7 years ago
Closed 7 years ago
Remove controlcenter-specific subview arrows
Categories
(Firefox :: Theme, enhancement)
Firefox
Theme
Tracking
()
VERIFIED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: dao, Assigned: dao)
Details
Attachments
(1 file)
No description provided.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8882785 [details] Bug 1377655 - Remove controlcenter-specific subview arrows. https://reviewboard.mozilla.org/r/153892/#review159046 I'm generally in favor of this kind of optimization, but we have to be careful here. I'm seeing a noticeable lag/stutter on my 2014 Windows laptop when switching between the main view and the subview on the identity popup after this change. It's buttery smooth without the patch. Can you confirm the stutter on your computer? ::: browser/themes/shared/controlcenter/panel.inc.css:132 (Diff revision 1) > -moz-appearance: none; > - background: url("chrome://browser/skin/controlcenter/arrow-subview.svg") center no-repeat; > + background: url("chrome://browser/skin/arrow-left.svg") center no-repeat; > background-size: 16px, auto; > + -moz-context-properties: fill; > + fill: currentColor; > + color: inherit; This doesn't work for me (on Windows). The icon is black in both the expanded and non-expanded state. I'm having a hard time validating that this pattern works at all, since all usages I've found so far are simply black icons. Explicitly setting fill: GrayText and fill: HighlightText works, and I don't really see how it's worse than this :)
Attachment #8882785 -
Flags: review?(jhofmann) → review-
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Johann Hofmann [:johannh] from comment #2) > I'm generally in favor of this kind of optimization, but we have to be > careful here. I'm seeing a noticeable lag/stutter on my 2014 Windows laptop > when switching between the main view and the subview on the identity popup > after this change. It's buttery smooth without the patch. > > Can you confirm the stutter on your computer? No, I cannot. Seems to work fine on Ubuntu and Windows 10. > ::: browser/themes/shared/controlcenter/panel.inc.css:132 > (Diff revision 1) > > -moz-appearance: none; > > - background: url("chrome://browser/skin/controlcenter/arrow-subview.svg") center no-repeat; > > + background: url("chrome://browser/skin/arrow-left.svg") center no-repeat; > > background-size: 16px, auto; > > + -moz-context-properties: fill; > > + fill: currentColor; > > + color: inherit; > > This doesn't work for me (on Windows). The icon is black in both the > expanded and non-expanded state. This worked on Ubuntu, just checked Windows 10 too. > I'm having a hard time validating that this pattern works at all, since all > usages I've found so far are simply black icons. We do use fill: currentColor for non-black icons, for instance http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/browser/themes/shared/contextmenu.inc.css#16 on Ubuntu.
Comment hidden (mozreview-request) |
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8882785 [details] Bug 1377655 - Remove controlcenter-specific subview arrows. https://reviewboard.mozilla.org/r/153892/#review166710 Sorry for the delay. This works fine for me now, in all regards. I'm really not sure what was breaking it before, but I can't reproduce it.
Attachment #8882785 -
Flags: review?(jhofmann) → review+
Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/efd5c608d474 Remove controlcenter-specific subview arrows. r=johannh
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/efd5c608d474
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Comment 8•7 years ago
|
||
Screenshots: https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=388d81ed93fa640f91d155f36254667c734157cf&newProject=mozilla-central&newRev=f1693d664f8e8ee4c79801630c181c28095cad56&filter=controlCenter Not sure what happened to Windows, I hope it's just a hickup due to the Taskcluster transition.
Status: RESOLVED → VERIFIED
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•