commit 7d205a638012 Author: Ian Moody Date: Sun Oct 8 13:23:33 2017 +0100 Bug 1404568 - Improve webext browser_action icon fallbacks. r=mixedpuppy, a=ritu Currently if there is no default icon at the specified size, the default icon falls back to the light text icon at that size. This is wrong in two ways: First, the default theme uses dark text, so it should fallback to the dark icon Secondly, authors expect the unsized default_icon to be used if specified This patch fixes both of these issues, so that the default icon first falls back to the unsized default_icon, and then only if that is not specified falls back to the dark icon MozReview-Commit-ID: C3RRTKhYq6r --HG-- extra : source : ca81275884eb6147bd1022779fcac81fa0930128 --- .../extensions/test/browser/browser_ext_browserAction_theme_icons.js | 4 ++-- toolkit/components/extensions/ExtensionParent.jsm | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git browser/components/extensions/test/browser/browser_ext_browserAction_theme_icons.js browser/components/extensions/test/browser/browser_ext_browserAction_theme_icons.js index 05d8474ab2b6..b910568e0f78 100644 --- browser/components/extensions/test/browser/browser_ext_browserAction_theme_icons.js +++ browser/components/extensions/test/browser/browser_ext_browserAction_theme_icons.js @@ -46,7 +46,7 @@ async function testStaticTheme(options) { await extension.startup(); // Confirm that the browser action has the correct default icon before a theme is loaded. - let expectedDefaultIcon = withDefaultIcon ? "default.png" : "light.png"; + let expectedDefaultIcon = withDefaultIcon ? "default.png" : "dark.png"; await testBrowserAction(extension, expectedDefaultIcon); let theme = ExtensionTestUtils.loadExtension({ @@ -164,7 +164,7 @@ add_task(async function browseraction_theme_icons_dynamic_theme() { "theme_icons": [{ "light": "light.png", "dark": "dark.png", - "size": 19, + "size": 16, }], }, }, diff --git toolkit/components/extensions/ExtensionParent.jsm toolkit/components/extensions/ExtensionParent.jsm index e608ff2d19c6..75df6a285195 100644 --- toolkit/components/extensions/ExtensionParent.jsm +++ toolkit/components/extensions/ExtensionParent.jsm @@ -1306,9 +1306,9 @@ let IconDetails = { this._checkURL(lightURL, extension); this._checkURL(darkURL, extension); - let defaultURL = result[size]; + let defaultURL = result[size] || result[19]; // always fallback to default first result[size] = { - "default": defaultURL || lightURL, // Fallback to the light url if no default is specified. + "default": defaultURL || darkURL, // Fallback to the dark url if no default is specified. "light": lightURL, "dark": darkURL, }; commit 1cceb370d13c Author: Ian Moody Date: Wed Oct 4 01:40:45 2017 +0100 Bug 1404568 - Use the correct browser_action theme icons when the action is in a menu-panel. r=mixedpuppy, a=ritu The patch adding support for specifying theme icons had a bug in the CSS: it added styles for the action in a menu-panel depending on theme, but missed out the theme pseudo-class selectors. Therefore the dark text icon was always used since it was last in the CSS. Additionally, the menu panels can't be styled, so still have light backgrounds and dark text even in light text themes. If a light icon is used in the menu panel in a light text theme it will be hard to see. Thus, this patch adds the pseudo-class for dark text themes, but removes the selector entirely for light text themes. MozReview-Commit-ID: AmKVDYwGGKj --HG-- extra : source : 38347dff77997359905b6efc1f6061e4103431d6 --- browser/base/content/browser.css | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git browser/base/content/browser.css browser/base/content/browser.css index bbd7310f55d0..fe940cf58408 100644 --- browser/base/content/browser.css +++ browser/base/content/browser.css @@ -377,12 +377,11 @@ toolbarpaletteitem > toolbaritem[sdkstylewidget="true"][cui-areatype="toolbar"] list-style-image: var(--webextension-menupanel-image, inherit); } - .webextension-browser-action[cui-areatype="menu-panel"], toolbarpaletteitem[place="palette"] > .webextension-browser-action:-moz-lwtheme-brighttext { list-style-image: var(--webextension-menupanel-image-light, inherit); } - .webextension-browser-action[cui-areatype="menu-panel"], + .webextension-browser-action[cui-areatype="menu-panel"]:-moz-lwtheme-darktext, toolbarpaletteitem[place="palette"] > .webextension-browser-action:-moz-lwtheme-darktext { list-style-image: var(--webextension-menupanel-image-dark, inherit); } @@ -414,12 +413,11 @@ toolbarpaletteitem > toolbaritem[sdkstylewidget="true"][cui-areatype="toolbar"] list-style-image: var(--webextension-menupanel-image-2x, inherit); } - .webextension-browser-action[cui-areatype="menu-panel"], toolbarpaletteitem[place="palette"] > .webextension-browser-action:-moz-lwtheme-brighttext { list-style-image: var(--webextension-menupanel-image-2x-light, inherit); } - .webextension-browser-action[cui-areatype="menu-panel"], + .webextension-browser-action[cui-areatype="menu-panel"]:-moz-lwtheme-darktext, toolbarpaletteitem[place="palette"] > .webextension-browser-action:-moz-lwtheme-darktext { list-style-image: var(--webextension-menupanel-image-2x-dark, inherit); }