FYI, this will have to wait until 56 is off mozilla-central (tomorrow I think). Not sure whether we should land this before or after bug 1349555; updating either patch after the other is probably no big deal.

ntim, could you please rebase your patch? There are only some minor conflicts in browser/themes/windows/browser.css and browser/themes/shared/sidebar.inc.css.

Comment on attachment 8891741 [details] [mq]: foo https://reviewboard.mozilla.org/r/162808/#review169710

Comment on attachment 8891741 [details] [mq]: foo https://reviewboard.mozilla.org/r/162808/#review169762 ::: toolkit/themes/shared/close-icon.inc.css:19 (Diff revision 7) > +.close-icon:hover { > + --close-icon-background-color: rgba(12,12,13,0.1); > +} > + > +.close-icon:hover:active { > + --close-icon-background-color: rgba(12,12,13,0.2); I think this should probably part of the SVG, using context-fill and context-fill-opacity. As written your background color isn't visible on dark backgrounds. ::: toolkit/themes/shared/icons/close.svg:5 (Diff revision 7) > +<!-- This Source Code Form is subject to the terms of the Mozilla Public > + - License, v. 2.0. If a copy of the MPL was not distributed with this > + - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> > +<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16"> > + <path fill="context-fill" d="M9.061 8l3.47-3.47a.75.75 0 0 0-1.061-1.06L8 6.939 4.53 3.47a.75.75 0 1 0-1.06 1.06L6.939 8 3.47 11.47a.75.75 0 1 0 1.06 1.06L8 9.061l3.47 3.47a.75.75 0 0 0 1.06-1.061z"></path> nit: <path/> rather than <path></path>

Comment on attachment 8891741 [details] [mq]: foo https://reviewboard.mozilla.org/r/162808/#review169774 ::: browser/extensions/onboarding/content/onboarding.css:76 (Diff revision 8) > position: absolute; > top: 15px; > offset-inline-end: 15px; > cursor: pointer; > width: 16px; > height: 16px; These numbers need tweaking ::: browser/extensions/onboarding/content/onboarding.css:79 (Diff revision 8) > cursor: pointer; > width: 16px; > height: 16px; > padding: 12px; > border: none; > background: var(--onboarding-overlay-dialog-background-color); I think this can go away? ::: browser/extensions/onboarding/content/onboarding.css:83 (Diff revision 8) > border: none; > background: var(--onboarding-overlay-dialog-background-color); > } > > .onboarding-close-btn::before { > - content: url(chrome://browser/skin/sidebar/close.svg); > + content: url(chrome://global/skin/icons/close.svg); Need to set -moz-context-properties, fill, and fill-opacity. ::: toolkit/themes/shared/close-icon.inc.css:6 (Diff revision 8) > +/* This Source Code Form is subject to the terms of the Mozilla Public > + * License, v. 2.0. If a copy of the MPL was not distributed with this > + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > + > +.close-icon { > + --close-icon-background-color: transparent; nit: remove this :)

Comment on attachment 8891741 [details] [mq]: foo https://reviewboard.mozilla.org/r/162808/#review169850 ::: browser/extensions/onboarding/content/onboarding.css:71 (Diff revision 9) > #onboarding-tour-sync-page[data-login-state=logged-out] .show-on-logged-in { > display: none; > } > > .onboarding-close-btn { > + background: url(chrome://global/skin/icons/close.svg); Pretty sure we need to keep using ::before for high-contrast themes.

Comment on attachment 8891741 [details] [mq]: foo https://reviewboard.mozilla.org/r/162808/#review169854 Thanks a lot!

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 3e01e40718dd -d fa94c0594016: rebasing 411392:3e01e40718dd "Bug 1385702 - Update and clean up close icon styling for photon. r=dao" (tip) merging browser/themes/osx/compacttheme.css warning: conflicts while merging browser/themes/osx/compacttheme.css! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment on attachment 8893469 [details] Bug 1385702 - Update and clean up close icon styling for photon. https://reviewboard.mozilla.org/r/164548/#review169864

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 8496455dbefa -d fa94c0594016: rebasing 411392:8496455dbefa "Bug 1385702 - Update and clean up close icon styling for photon. r=dao" (tip) merging browser/themes/osx/compacttheme.css warning: conflicts while merging browser/themes/osx/compacttheme.css! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/autoland/rev/04cf5e9b8f72 Update and clean up close icon styling for photon. r=dao

I see some AWSY improvements from this landing: == Change summary for alert #8532 (as of August 03 2017 17:41 UTC) == Improvements: 3% Images summary linux64-stylo opt stylo 7,652,586.31 -> 7,442,150.60 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8532

Sorry, this took a bit while we were fixing Windows, but here are the screenshots: https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=36ad88e6b7b248c2f2ae59b80477e5474dd653dc&newProject=mozilla-central&newRev=5742919ec43f834cc061a96d87c767af1a1f7f75

I verified this issue on Mac OS X 10.12, Mac OS X 10.10, Windows 10, Windows 7 and Ubuntu 16.04 with FF Nightly 57.0a1(2017-08-16) and I can confirm that the close button respects the spec from comment 28. I will mark this as verified fixed.