Bug 1399886 - remove unnecessary invertable CSS classes on devtools icons;r=gl draft
authorJulian Descottes <jdescottes@mozilla.com>
Wed, 04 Oct 2017 21:13:48 +0200
changeset 675063 391f3567c2bdf319dcfd0a3b0c87f0479f85eabd
parent 675062 c613fc38ce7fedcfe111bed74216c1c38b511b9d
child 675557 3c79e0c7f49e7f7239300ad833e15f1847dab39a
push id83028
push userjdescottes@mozilla.com
push dateWed, 04 Oct 2017 19:44:50 +0000
reviewersgl
bugs1399886
milestone58.0a1
Bug 1399886 - remove unnecessary invertable CSS classes on devtools icons;r=gl Using fill instead of filter we don't need to define each icon as invertable or not. If the icon is a SVG and supports fill="context-fill" then it will be inverted/highlighted etc... as expected. If not then it won't be impacted by DevTools themes. MozReview-Commit-ID: CLFprKMuCt9
devtools/client/definitions.js
devtools/client/framework/components/toolbox-tab.js
devtools/client/framework/components/toolbox-toolbar.js
devtools/client/framework/devtools.js
devtools/client/framework/test/browser_devtools_api.js
devtools/client/inspector/inspector.xhtml
devtools/client/responsive.html/commands.js
devtools/client/scratchpad/scratchpad-commands.js
devtools/client/webconsole/console-commands.js
devtools/shared/gcli/commands/measure.js
devtools/shared/gcli/commands/paintflashing.js
devtools/shared/gcli/commands/rulers.js
devtools/shared/gcli/commands/screenshot.js
--- a/devtools/client/definitions.js
+++ b/devtools/client/definitions.js
@@ -40,17 +40,16 @@ var Tools = {};
 exports.Tools = Tools;
 
 // Definitions
 Tools.options = {
   id: "options",
   ordinal: 0,
   url: "chrome://devtools/content/framework/toolbox-options.xhtml",
   icon: "chrome://devtools/skin/images/tool-options.svg",
-  invertIconForDarkTheme: true,
   bgTheme: "theme-body",
   label: l10n("options.label"),
   iconOnly: true,
   panelLabel: l10n("options.panelLabel"),
   tooltip: l10n("optionsButton.tooltip"),
   inMenu: false,
 
   isTargetSupported: function () {
@@ -62,17 +61,16 @@ Tools.options = {
   }
 };
 
 Tools.inspector = {
   id: "inspector",
   accesskey: l10n("inspector.accesskey"),
   ordinal: 1,
   icon: "chrome://devtools/skin/images/tool-inspector.svg",
-  invertIconForDarkTheme: true,
   url: "chrome://devtools/content/inspector/inspector.xhtml",
   label: l10n("inspector.label"),
   panelLabel: l10n("inspector.panelLabel"),
   get tooltip() {
     return l10n("inspector.tooltip2",
     (osString == "Darwin" ? "Cmd+Opt+" : "Ctrl+Shift+") +
     l10n("inspector.commandkey"));
   },
@@ -97,17 +95,16 @@ Tools.inspector = {
 };
 Tools.webConsole = {
   id: "webconsole",
   accesskey: l10n("webConsoleCmd.accesskey"),
   ordinal: 2,
   oldWebConsoleURL: "chrome://devtools/content/webconsole/webconsole.xul",
   newWebConsoleURL: "chrome://devtools/content/webconsole/webconsole.html",
   icon: "chrome://devtools/skin/images/tool-webconsole.svg",
-  invertIconForDarkTheme: true,
   label: l10n("ToolboxTabWebconsole.label"),
   menuLabel: l10n("MenuWebconsole.label"),
   panelLabel: l10n("ToolboxWebConsole.panelLabel"),
   get tooltip() {
     return l10n("ToolboxWebconsole.tooltip2",
     (osString == "Darwin" ? "Cmd+Opt+" : "Ctrl+Shift+") +
     l10n("webconsole.commandkey"));
   },
@@ -145,17 +142,16 @@ Services.prefs.addObserver(
   { observe: switchWebconsole }
 );
 
 Tools.jsdebugger = {
   id: "jsdebugger",
   accesskey: l10n("debuggerMenu.accesskey"),
   ordinal: 3,
   icon: "chrome://devtools/skin/images/tool-debugger.svg",
-  invertIconForDarkTheme: true,
   url: "chrome://devtools/content/debugger/debugger.xul",
   label: l10n("ToolboxDebugger.label"),
   panelLabel: l10n("ToolboxDebugger.panelLabel"),
   get tooltip() {
     return l10n("ToolboxDebugger.tooltip2",
     (osString == "Darwin" ? "Cmd+Opt+" : "Ctrl+Shift+") +
     l10n("debugger.commandkey"));
   },
@@ -192,17 +188,16 @@ Services.prefs.addObserver(
 );
 
 Tools.styleEditor = {
   id: "styleeditor",
   ordinal: 4,
   visibilityswitch: "devtools.styleeditor.enabled",
   accesskey: l10n("open.accesskey"),
   icon: "chrome://devtools/skin/images/tool-styleeditor.svg",
-  invertIconForDarkTheme: true,
   url: "chrome://devtools/content/styleeditor/styleeditor.xul",
   label: l10n("ToolboxStyleEditor.label"),
   panelLabel: l10n("ToolboxStyleEditor.panelLabel"),
   get tooltip() {
     return l10n("ToolboxStyleEditor.tooltip3",
     "Shift+" + functionkey(l10n("styleeditor.commandkey")));
   },
   inMenu: true,
@@ -217,17 +212,16 @@ Tools.styleEditor = {
   }
 };
 
 Tools.shaderEditor = {
   id: "shadereditor",
   ordinal: 5,
   visibilityswitch: "devtools.shadereditor.enabled",
   icon: "chrome://devtools/skin/images/tool-shadereditor.svg",
-  invertIconForDarkTheme: true,
   url: "chrome://devtools/content/shadereditor/shadereditor.xul",
   label: l10n("ToolboxShaderEditor.label"),
   panelLabel: l10n("ToolboxShaderEditor.panelLabel"),
   tooltip: l10n("ToolboxShaderEditor.tooltip"),
 
   isTargetSupported: function (target) {
     return target.hasActor("webgl") && !target.chrome;
   },
@@ -237,17 +231,16 @@ Tools.shaderEditor = {
   }
 };
 
 Tools.canvasDebugger = {
   id: "canvasdebugger",
   ordinal: 6,
   visibilityswitch: "devtools.canvasdebugger.enabled",
   icon: "chrome://devtools/skin/images/tool-canvas.svg",
-  invertIconForDarkTheme: true,
   url: "chrome://devtools/content/canvasdebugger/canvasdebugger.xul",
   label: l10n("ToolboxCanvasDebugger.label"),
   panelLabel: l10n("ToolboxCanvasDebugger.panelLabel"),
   tooltip: l10n("ToolboxCanvasDebugger.tooltip"),
 
   // Hide the Canvas Debugger in the Add-on Debugger and Browser Toolbox
   // (bug 1047520).
   isTargetSupported: function (target) {
@@ -258,17 +251,16 @@ Tools.canvasDebugger = {
     return new CanvasDebuggerPanel(iframeWindow, toolbox);
   }
 };
 
 Tools.performance = {
   id: "performance",
   ordinal: 7,
   icon: "chrome://devtools/skin/images/tool-profiler.svg",
-  invertIconForDarkTheme: true,
   url: "chrome://devtools/content/performance/performance.xul",
   visibilityswitch: "devtools.performance.enabled",
   label: l10n("performance.label"),
   panelLabel: l10n("performance.panelLabel"),
   get tooltip() {
     return l10n("performance.tooltip", "Shift+" +
     functionkey(l10n("performance.commandkey")));
   },
@@ -283,17 +275,16 @@ Tools.performance = {
     return new PerformancePanel(frame, target);
   }
 };
 
 Tools.memory = {
   id: "memory",
   ordinal: 8,
   icon: "chrome://devtools/skin/images/tool-memory.svg",
-  invertIconForDarkTheme: true,
   url: "chrome://devtools/content/memory/memory.xhtml",
   visibilityswitch: "devtools.memory.enabled",
   label: l10n("memory.label"),
   panelLabel: l10n("memory.panelLabel"),
   tooltip: l10n("memory.tooltip"),
 
   isTargetSupported: function (target) {
     return target.getTrait("heapSnapshots") && !target.isAddon;
@@ -305,17 +296,16 @@ Tools.memory = {
 };
 
 Tools.netMonitor = {
   id: "netmonitor",
   accesskey: l10n("netmonitor.accesskey"),
   ordinal: 9,
   visibilityswitch: "devtools.netmonitor.enabled",
   icon: "chrome://devtools/skin/images/tool-network.svg",
-  invertIconForDarkTheme: true,
   url: "chrome://devtools/content/netmonitor/index.html",
   label: l10n("netmonitor.label"),
   panelLabel: l10n("netmonitor.panelLabel"),
   get tooltip() {
     return l10n("netmonitor.tooltip2",
     (osString == "Darwin" ? "Cmd+Opt+" : "Ctrl+Shift+") +
     l10n("netmonitor.commandkey"));
   },
@@ -331,17 +321,16 @@ Tools.netMonitor = {
 };
 
 Tools.storage = {
   id: "storage",
   ordinal: 10,
   accesskey: l10n("storage.accesskey"),
   visibilityswitch: "devtools.storage.enabled",
   icon: "chrome://devtools/skin/images/tool-storage.svg",
-  invertIconForDarkTheme: true,
   url: "chrome://devtools/content/storage/storage.xul",
   label: l10n("storage.label"),
   menuLabel: l10n("storage.menuLabel"),
   panelLabel: l10n("storage.panelLabel"),
   get tooltip() {
     return l10n("storage.tooltip3", "Shift+" +
     functionkey(l10n("storage.commandkey")));
   },
@@ -357,17 +346,16 @@ Tools.storage = {
   }
 };
 
 Tools.webAudioEditor = {
   id: "webaudioeditor",
   ordinal: 11,
   visibilityswitch: "devtools.webaudioeditor.enabled",
   icon: "chrome://devtools/skin/images/tool-webaudio.svg",
-  invertIconForDarkTheme: true,
   url: "chrome://devtools/content/webaudioeditor/webaudioeditor.xul",
   label: l10n("ToolboxWebAudioEditor1.label"),
   panelLabel: l10n("ToolboxWebAudioEditor1.panelLabel"),
   tooltip: l10n("ToolboxWebAudioEditor1.tooltip"),
 
   isTargetSupported: function (target) {
     return !target.chrome && target.hasActor("webaudio");
   },
@@ -377,17 +365,16 @@ Tools.webAudioEditor = {
   }
 };
 
 Tools.scratchpad = {
   id: "scratchpad",
   ordinal: 12,
   visibilityswitch: "devtools.scratchpad.enabled",
   icon: "chrome://devtools/skin/images/tool-scratchpad.svg",
-  invertIconForDarkTheme: true,
   url: "chrome://devtools/content/scratchpad/scratchpad.xul",
   label: l10n("scratchpad.label"),
   panelLabel: l10n("scratchpad.panelLabel"),
   tooltip: l10n("scratchpad.tooltip"),
   inMenu: false,
   commands: "devtools/client/scratchpad/scratchpad-commands",
 
   isTargetSupported: function (target) {
@@ -400,17 +387,16 @@ Tools.scratchpad = {
 };
 
 Tools.dom = {
   id: "dom",
   accesskey: l10n("dom.accesskey"),
   ordinal: 13,
   visibilityswitch: "devtools.dom.enabled",
   icon: "chrome://devtools/skin/images/tool-dom.svg",
-  invertIconForDarkTheme: true,
   url: "chrome://devtools/content/dom/dom.html",
   label: l10n("dom.label"),
   panelLabel: l10n("dom.panelLabel"),
   get tooltip() {
     return l10n("dom.tooltip",
       (osString == "Darwin" ? "Cmd+Opt+" : "Ctrl+Shift+") +
       l10n("dom.commandkey"));
   },
--- a/devtools/client/framework/components/toolbox-tab.js
+++ b/devtools/client/framework/components/toolbox-tab.js
@@ -24,21 +24,16 @@ module.exports = createClass({
   render() {
     const {panelDefinition, currentToolId, highlightedTool, selectTool,
            focusedButton, focusButton} = this.props;
     const {id, tooltip, label, iconOnly} = panelDefinition;
     const isHighlighted = id === currentToolId;
 
     const className = [
       "devtools-tab",
-      panelDefinition.invertIconForLightTheme || panelDefinition.invertIconForDarkTheme
-        ? "icon-invertable"
-        : "",
-      panelDefinition.invertIconForLightTheme ? "icon-invertable-light-theme" : "",
-      panelDefinition.invertIconForDarkTheme ? "icon-invertable-dark-theme" : "",
       currentToolId === id ? "selected" : "",
       highlightedTool === id ? "highlighted" : "",
       iconOnly ? "devtools-tab-icon-only" : ""
     ].join(" ");
 
     return button(
       {
         className,
--- a/devtools/client/framework/components/toolbox-toolbar.js
+++ b/devtools/client/framework/components/toolbox-toolbar.js
@@ -113,17 +113,17 @@ function renderToolboxButtons({toolboxBu
         isChecked,
         className: buttonClass,
         onKeyDown
       } = command;
       return button({
         id,
         title: description,
         className: (
-          "command-button command-button-invertable devtools-button "
+          "command-button devtools-button "
           + buttonClass + (isChecked ? " checked" : "")
         ),
         onClick: (event) => {
           onClick(event);
           focusButton(id);
         },
         onFocus: () => focusButton(id),
         tabIndex: id === focusedButton ? "0" : "-1",
--- a/devtools/client/framework/devtools.js
+++ b/devtools/client/framework/devtools.js
@@ -94,20 +94,16 @@ DevTools.prototype = {
    *
    * Each toolDefinition has the following properties:
    * - id: Unique identifier for this tool (string|required)
    * - visibilityswitch: Property name to allow us to hide this tool from the
    *                     DevTools Toolbox.
    *                     A falsy value indicates that it cannot be hidden.
    * - icon: URL pointing to a graphic which will be used as the src for an
    *         16x16 img tag (string|required)
-   * - invertIconForLightTheme: The icon can automatically have an inversion
-   *         filter applied (default is false).  All builtin tools are true, but
-   *         addons may omit this to prevent unwanted changes to the `icon`
-   *         image. filter: invert(1) is applied to the image (boolean|optional)
    * - url: URL pointing to a XUL/XHTML document containing the user interface
    *        (string|required)
    * - label: Localized name for the tool to be displayed to the user
    *          (string|required)
    * - hideInOptions: Boolean indicating whether or not this tool should be
                       shown in toolbox options or not. Defaults to false.
    *                  (boolean)
    * - build: Function that takes an iframe, which has been populated with the
--- a/devtools/client/framework/test/browser_devtools_api.js
+++ b/devtools/client/framework/test/browser_devtools_api.js
@@ -138,24 +138,16 @@ function runTests2() {
     continueTests(toolbox);
   });
 }
 
 var continueTests = Task.async(function* (toolbox, panel) {
   ok(toolbox.getCurrentPanel(), "panel value is correct");
   is(toolbox.currentToolId, toolId2, "toolbox _currentToolId is correct");
 
-  ok(!toolbox.doc.getElementById("toolbox-tab-" + toolId2)
-     .classList.contains("icon-invertable"),
-     "The tool tab does not have the invertable class");
-
-  ok(toolbox.doc.getElementById("toolbox-tab-inspector")
-     .classList.contains("icon-invertable"),
-     "The builtin tool tabs do have the invertable class");
-
   let toolDefinitions = gDevTools.getToolDefinitionMap();
   ok(toolDefinitions.has(toolId2), "The tool is in gDevTools");
 
   let toolDefinition = toolDefinitions.get(toolId2);
   is(toolDefinition.id, toolId2, "toolDefinition id is correct");
 
   info("Testing toolbox tool-unregistered event");
   let toolSelected = toolbox.once("select");
--- a/devtools/client/inspector/inspector.xhtml
+++ b/devtools/client/inspector/inspector.xhtml
@@ -52,17 +52,17 @@
         <div class="devtools-toolbar-spacer"></div>
         <span id="inspector-searchlabel"></span>
         <div id="inspector-search" class="devtools-searchbox has-clear-btn">
           <input id="inspector-searchbox" class="devtools-searchinput"
                  type="search"
                  data-localization="placeholder=inspectorSearchHTML.label3"/>
           <button id="inspector-searchinput-clear" class="devtools-searchinput-clear" tabindex="-1"></button>
         </div>
-        <button id="inspector-eyedropper-toggle" class="devtools-button command-button-invertable"></button>
+        <button id="inspector-eyedropper-toggle" class="devtools-button"></button>
       </div>
       <div id="markup-box"></div>
       <div id="inspector-breadcrumbs-toolbar" class="devtools-toolbar">
         <div id="inspector-breadcrumbs" class="breadcrumbs-widget-container"
              role="group" data-localization="aria-label=inspector.breadcrumbs.label" tabindex="0"></div>
       </div>
     </div>
 
--- a/devtools/client/responsive.html/commands.js
+++ b/devtools/client/responsive.html/commands.js
@@ -38,17 +38,17 @@ exports.items = [
     manual: l10n.lookupFormat("resizeModeManual2", [BRAND_SHORT_NAME]),
     exec: resize
   },
   {
     item: "command",
     runAt: "client",
     name: "resize toggle",
     buttonId: "command-button-responsive",
-    buttonClass: "command-button command-button-invertable",
+    buttonClass: "command-button",
     tooltipText: l10n.lookupFormat(
       "resizeModeToggleTooltip2",
       [(osString == "Darwin" ? "Cmd+Opt+M" : "Ctrl+Shift+M")]
     ),
     description: l10n.lookup("resizeModeToggleDesc"),
     manual: l10n.lookupFormat("resizeModeManual2", [BRAND_SHORT_NAME]),
     state: {
       isChecked: function (target) {
--- a/devtools/client/scratchpad/scratchpad-commands.js
+++ b/devtools/client/scratchpad/scratchpad-commands.js
@@ -7,16 +7,16 @@
 const l10n = require("gcli/l10n");
 const {Cu} = require("chrome");
 
 exports.items = [{
   item: "command",
   runAt: "client",
   name: "scratchpad",
   buttonId: "command-button-scratchpad",
-  buttonClass: "command-button command-button-invertable",
+  buttonClass: "command-button",
   tooltipText: l10n.lookup("scratchpadOpenTooltip"),
   hidden: true,
   exec: function (args, context) {
     const {ScratchpadManager} = Cu.import("resource://devtools/client/scratchpad/scratchpad-manager.jsm", {});
     ScratchpadManager.openScratchpad();
   }
 }];
--- a/devtools/client/webconsole/console-commands.js
+++ b/devtools/client/webconsole/console-commands.js
@@ -12,17 +12,17 @@ loader.lazyRequireGetter(this, "gDevTool
 
 exports.items = [
   {
     item: "command",
     runAt: "client",
     name: "splitconsole",
     hidden: true,
     buttonId: "command-button-splitconsole",
-    buttonClass: "command-button command-button-invertable",
+    buttonClass: "command-button",
     tooltipText: l10n.lookupFormat("splitconsoleTooltip2", ["Esc"]),
     isRemoteSafe: true,
     state: {
       isChecked: function (target) {
         let toolbox = gDevTools.getToolbox(target);
         return !!(toolbox && toolbox.splitConsole);
       },
       onChange: function (target, changeHandler) {
--- a/devtools/shared/gcli/commands/measure.js
+++ b/devtools/shared/gcli/commands/measure.js
@@ -23,17 +23,17 @@ exports.items = [
   // only and redirects to the server command to actually toggle the measuring
   // tool (see `measure_server` below).
   {
     name: "measure",
     runAt: "client",
     description: l10n.lookup("measureDesc"),
     manual: l10n.lookup("measureManual"),
     buttonId: "command-button-measure",
-    buttonClass: "command-button command-button-invertable",
+    buttonClass: "command-button",
     tooltipText: l10n.lookup("measureTooltip"),
     state: {
       isChecked: (target) => CommandState.isEnabledForTarget(target, "measure"),
       onChange: (target, handler) => CommandState.on("changed", handler),
       offChange: (target, handler) => CommandState.off("changed", handler)
     },
     exec: function* (args, context) {
       let { target } = context.environment;
--- a/devtools/shared/gcli/commands/paintflashing.js
+++ b/devtools/shared/gcli/commands/paintflashing.js
@@ -141,17 +141,17 @@ exports.items = [
     }
   },
   {
     item: "command",
     runAt: "client",
     name: "paintflashing toggle",
     hidden: true,
     buttonId: "command-button-paintflashing",
-    buttonClass: "command-button command-button-invertable",
+    buttonClass: "command-button",
     state: {
       isChecked: (target) => CommandState.isEnabledForTarget(target, "paintflashing"),
       onChange: (_, handler) => CommandState.on("changed", handler),
       offChange: (_, handler) => CommandState.off("changed", handler),
     },
     tooltipText: l10n.lookup("paintflashingTooltip"),
     description: l10n.lookup("paintflashingToggleDesc"),
     manual: l10n.lookup("paintflashingManual"),
--- a/devtools/shared/gcli/commands/rulers.js
+++ b/devtools/shared/gcli/commands/rulers.js
@@ -23,17 +23,17 @@ exports.items = [
   // and redirects to the server command to actually toggle the rulers (see
   // rulers_server below).
   {
     name: "rulers",
     runAt: "client",
     description: l10n.lookup("rulersDesc"),
     manual: l10n.lookup("rulersManual"),
     buttonId: "command-button-rulers",
-    buttonClass: "command-button command-button-invertable",
+    buttonClass: "command-button",
     tooltipText: l10n.lookup("rulersTooltip"),
     state: {
       isChecked: (target) => CommandState.isEnabledForTarget(target, "rulers"),
       onChange: (target, handler) => CommandState.on("changed", handler),
       offChange: (target, handler) => CommandState.off("changed", handler)
     },
     exec: function* (args, context) {
       let { target } = context.environment;
--- a/devtools/shared/gcli/commands/screenshot.js
+++ b/devtools/shared/gcli/commands/screenshot.js
@@ -179,17 +179,17 @@ exports.items = [
   {
     item: "command",
     runAt: "client",
     name: "screenshot",
     description: l10n.lookup("screenshotDesc"),
     manual: l10n.lookup("screenshotManual"),
     returnType: "imageSummary",
     buttonId: "command-button-screenshot",
-    buttonClass: "command-button command-button-invertable",
+    buttonClass: "command-button",
     tooltipText: l10n.lookup("screenshotTooltipPage"),
     params: [
       filenameParam,
       clientScreenshotParams,
     ],
     exec: function (args, context) {
       // Re-execute the command on the server
       const command = context.typed.replace(/^screenshot/, "screenshot_server");