Bug 967168 - DevTools Addon icons should not be inverted on light theme;r=jwalker
authorBrian Grinstead <bgrinstead@mozilla.com>
Mon, 10 Feb 2014 07:50:13 -0600
changeset 167835 dc07278bb6f84dad9d4d458d4d16cbe4c6f04c0b
parent 167834 62dd35d5057894bfc98a7db8dbb018ebe6d49f08
child 167836 a1b47aeca0cee75fcddeb19553a45ca79bb11b48
push id26189
push userryanvm@gmail.com
push dateMon, 10 Feb 2014 20:32:33 +0000
treeherdermozilla-central@f5726434eedc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalker
bugs967168
milestone30.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 967168 - DevTools Addon icons should not be inverted on light theme;r=jwalker
browser/devtools/commandline/BuiltinCommands.jsm
browser/devtools/framework/gDevTools.jsm
browser/devtools/framework/test/browser_devtools_api.js
browser/devtools/framework/toolbox.js
browser/devtools/main.js
browser/devtools/responsivedesign/CmdResize.jsm
browser/devtools/scratchpad/CmdScratchpad.jsm
browser/devtools/tilt/CmdTilt.jsm
browser/themes/shared/devtools/toolbars.inc.css
--- a/browser/devtools/commandline/BuiltinCommands.jsm
+++ b/browser/devtools/commandline/BuiltinCommands.jsm
@@ -1986,17 +1986,17 @@ gcli.addCommand({
       onPaintFlashingChanged(context);
     }
   });
 
   gcli.addCommand({
     name: 'paintflashing toggle',
     hidden: true,
     buttonId: "command-button-paintflashing",
-    buttonClass: "command-button",
+    buttonClass: "command-button command-button-invertable",
     state: {
       isChecked: function(aTarget) {
         if (aTarget.isLocalTab) {
           let window = aTarget.tab.linkedBrowser.contentWindow;
           let wUtils = window.QueryInterface(Ci.nsIInterfaceRequestor).
                               getInterface(Ci.nsIDOMWindowUtils);
           return wUtils.paintFlashing;
         } else {
@@ -2279,17 +2279,17 @@ gcli.addCommand({
   /**
    * 'splitconsole' command (hidden)
    */
 
   gcli.addCommand({
     name: 'splitconsole',
     hidden: true,
     buttonId: "command-button-splitconsole",
-    buttonClass: "command-button",
+    buttonClass: "command-button command-button-invertable",
     tooltipText: gcli.lookup("splitconsoleTooltip"),
     state: {
       isChecked: function(aTarget) {
         let toolbox = gDevTools.getToolbox(aTarget);
         return toolbox &&
           toolbox.splitConsole;
       },
       onChange: function(aTarget, aChangeHandler) {
--- a/browser/devtools/framework/gDevTools.jsm
+++ b/browser/devtools/framework/gDevTools.jsm
@@ -50,16 +50,21 @@ 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. See browser/themes/shared/devtools/filters.svg#invert for
+   *         the filter being applied to the images (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)
    * - build: Function that takes an iframe, which has been populated with the
    *          markup from |url|, and also the toolbox containing the panel.
    *          And returns an instance of ToolPanel (function|required)
    */
--- a/browser/devtools/framework/test/browser_devtools_api.js
+++ b/browser/devtools/framework/test/browser_devtools_api.js
@@ -44,16 +44,22 @@ function runTests(aTab) {
     continueTests(toolbox);
   }).then(null, console.error);
 }
 
 function continueTests(toolbox, panel) {
   ok(toolbox.getCurrentPanel(), "panel value is correct");
   is(toolbox.currentToolId, toolId, "toolbox _currentToolId is correct");
 
+  ok(!toolbox.doc.getElementById("toolbox-tab-" + toolId).hasAttribute("icon-invertable"),
+    "The tool tab does not have the invertable attribute");
+
+  ok(toolbox.doc.getElementById("toolbox-tab-inspector").hasAttribute("icon-invertable"),
+    "The builtin tool tabs do have the invertable attribute");
+
   let toolDefinitions = gDevTools.getToolDefinitionMap();
   is(toolDefinitions.has(toolId), true, "The tool is in gDevTools");
 
   let toolDefinition = toolDefinitions.get(toolId);
   is(toolDefinition.id, toolId, "toolDefinition id is correct");
 
   gDevTools.unregisterTool(toolId);
   is(gDevTools.getToolDefinitionMap().has(toolId), false,
--- a/browser/devtools/framework/toolbox.js
+++ b/browser/devtools/framework/toolbox.js
@@ -549,17 +549,17 @@ Toolbox.prototype = {
 
   /**
    * Adding the element picker button is done here unlike the other buttons
    * since we want it to work for remote targets too
    */
   _buildPickerButton: function() {
     this._pickerButton = this.doc.createElement("toolbarbutton");
     this._pickerButton.id = "command-button-pick";
-    this._pickerButton.className = "command-button";
+    this._pickerButton.className = "command-button command-button-invertable";
     this._pickerButton.setAttribute("tooltiptext", toolboxStrings("pickButton.tooltip"));
 
     let container = this.doc.querySelector("#toolbox-buttons");
     container.appendChild(this._pickerButton);
 
     this._togglePicker = this.highlighterUtils.togglePicker.bind(this.highlighterUtils);
     this._pickerButton.addEventListener("command", this._togglePicker, false);
   },
@@ -583,21 +583,24 @@ Toolbox.prototype = {
     if (toolDefinition.ordinal == undefined || toolDefinition.ordinal < 0) {
       toolDefinition.ordinal = MAX_ORDINAL;
     }
 
     let radio = this.doc.createElement("radio");
     // The radio element is not being used in the conventional way, thus
     // the devtools-tab class replaces the radio XBL binding with its base
     // binding (the control-item binding).
-    radio.className = "toolbox-tab devtools-tab";
+    radio.className = "devtools-tab";
     radio.id = "toolbox-tab-" + id;
     radio.setAttribute("toolid", id);
     radio.setAttribute("ordinal", toolDefinition.ordinal);
     radio.setAttribute("tooltiptext", toolDefinition.tooltip);
+    if (toolDefinition.invertIconForLightTheme) {
+      radio.setAttribute("icon-invertable", "true");
+    }
 
     radio.addEventListener("command", () => {
       this.selectTool(id);
     });
 
     // spacer lets us center the image and label, while allowing cropping
     let spacer = this.doc.createElement("spacer");
     spacer.setAttribute("flex", "1");
--- a/browser/devtools/main.js
+++ b/browser/devtools/main.js
@@ -56,16 +56,17 @@ let Tools = {};
 exports.Tools = Tools;
 
 // Definitions
 Tools.options = {
   id: "options",
   ordinal: 0,
   url: "chrome://browser/content/devtools/framework/toolbox-options.xul",
   icon: "chrome://browser/skin/devtools/tool-options.svg",
+  invertIconForLightTheme: true,
   bgTheme: "theme-body",
   tooltip: l10n("optionsButton.tooltip", toolboxStrings),
   inMenu: false,
   isTargetSupported: function(target) {
     return true;
   },
   build: function(iframeWindow, toolbox) {
     let panel = new OptionsPanel(iframeWindow, toolbox);
@@ -75,16 +76,17 @@ Tools.options = {
 
 Tools.webConsole = {
   id: "webconsole",
   key: l10n("cmd.commandkey", webConsoleStrings),
   accesskey: l10n("webConsoleCmd.accesskey", webConsoleStrings),
   modifiers: Services.appinfo.OS == "Darwin" ? "accel,alt" : "accel,shift",
   ordinal: 1,
   icon: "chrome://browser/skin/devtools/tool-webconsole.svg",
+  invertIconForLightTheme: true,
   url: "chrome://browser/content/devtools/webconsole.xul",
   label: l10n("ToolboxTabWebconsole.label", webConsoleStrings),
   menuLabel: l10n("MenuWebconsole.label", webConsoleStrings),
   tooltip: l10n("ToolboxWebconsole.tooltip", webConsoleStrings),
   inMenu: true,
 
   preventClosingOnKey: true,
   onkey: function(panel, toolbox) {
@@ -105,16 +107,17 @@ Tools.webConsole = {
 
 Tools.inspector = {
   id: "inspector",
   accesskey: l10n("inspector.accesskey", inspectorStrings),
   key: l10n("inspector.commandkey", inspectorStrings),
   ordinal: 2,
   modifiers: osString == "Darwin" ? "accel,alt" : "accel,shift",
   icon: "chrome://browser/skin/devtools/tool-inspector.svg",
+  invertIconForLightTheme: true,
   url: "chrome://browser/content/devtools/inspector/inspector.xul",
   label: l10n("inspector.label", inspectorStrings),
   tooltip: l10n("inspector.tooltip", inspectorStrings),
   inMenu: true,
 
   preventClosingOnKey: true,
   onkey: function(panel) {
     panel.toolbox.highlighterUtils.togglePicker();
@@ -132,16 +135,17 @@ Tools.inspector = {
 
 Tools.jsdebugger = {
   id: "jsdebugger",
   key: l10n("debuggerMenu.commandkey", debuggerStrings),
   accesskey: l10n("debuggerMenu.accesskey", debuggerStrings),
   modifiers: osString == "Darwin" ? "accel,alt" : "accel,shift",
   ordinal: 3,
   icon: "chrome://browser/skin/devtools/tool-debugger.svg",
+  invertIconForLightTheme: true,
   highlightedicon: "chrome://browser/skin/devtools/tool-debugger-paused.svg",
   url: "chrome://browser/content/devtools/debugger.xul",
   label: l10n("ToolboxDebugger.label", debuggerStrings),
   tooltip: l10n("ToolboxDebugger.tooltip", debuggerStrings),
   inMenu: true,
 
   isTargetSupported: function(target) {
     return true;
@@ -155,16 +159,17 @@ Tools.jsdebugger = {
 
 Tools.styleEditor = {
   id: "styleeditor",
   key: l10n("open.commandkey", styleEditorStrings),
   ordinal: 4,
   accesskey: l10n("open.accesskey", styleEditorStrings),
   modifiers: "shift",
   icon: "chrome://browser/skin/devtools/tool-styleeditor.svg",
+  invertIconForLightTheme: true,
   url: "chrome://browser/content/devtools/styleeditor.xul",
   label: l10n("ToolboxStyleEditor.label", styleEditorStrings),
   tooltip: l10n("ToolboxStyleEditor.tooltip2", styleEditorStrings),
   inMenu: true,
 
   isTargetSupported: function(target) {
     return true;
   },
@@ -175,16 +180,17 @@ Tools.styleEditor = {
   }
 };
 
 Tools.shaderEditor = {
   id: "shadereditor",
   ordinal: 5,
   visibilityswitch: "devtools.shadereditor.enabled",
   icon: "chrome://browser/skin/devtools/tool-styleeditor.svg",
+  invertIconForLightTheme: true,
   url: "chrome://browser/content/devtools/shadereditor.xul",
   label: l10n("ToolboxShaderEditor.label", shaderEditorStrings),
   tooltip: l10n("ToolboxShaderEditor.tooltip", shaderEditorStrings),
 
   isTargetSupported: function(target) {
     return true;
   },
 
@@ -197,16 +203,17 @@ Tools.shaderEditor = {
 Tools.jsprofiler = {
   id: "jsprofiler",
   accesskey: l10n("profiler.accesskey", profilerStrings),
   key: l10n("profiler2.commandkey", profilerStrings),
   ordinal: 6,
   modifiers: "shift",
   visibilityswitch: "devtools.profiler.enabled",
   icon: "chrome://browser/skin/devtools/tool-profiler.svg",
+  invertIconForLightTheme: true,
   url: "chrome://browser/content/devtools/profiler.xul",
   label: l10n("profiler.label", profilerStrings),
   tooltip: l10n("profiler.tooltip2", profilerStrings),
   inMenu: true,
 
   isTargetSupported: function (target) {
     return true;
   },
@@ -220,16 +227,17 @@ Tools.jsprofiler = {
 Tools.netMonitor = {
   id: "netmonitor",
   accesskey: l10n("netmonitor.accesskey", netMonitorStrings),
   key: l10n("netmonitor.commandkey", netMonitorStrings),
   ordinal: 7,
   modifiers: osString == "Darwin" ? "accel,alt" : "accel,shift",
   visibilityswitch: "devtools.netmonitor.enabled",
   icon: "chrome://browser/skin/devtools/tool-network.svg",
+  invertIconForLightTheme: true,
   url: "chrome://browser/content/devtools/netmonitor.xul",
   label: l10n("netmonitor.label", netMonitorStrings),
   tooltip: l10n("netmonitor.tooltip", netMonitorStrings),
   inMenu: true,
 
   isTargetSupported: function(target) {
     return !target.isApp;
   },
@@ -240,16 +248,17 @@ Tools.netMonitor = {
   }
 };
 
 Tools.scratchpad = {
   id: "scratchpad",
   ordinal: 8,
   visibilityswitch: "devtools.scratchpad.enabled",
   icon: "chrome://browser/skin/devtools/tool-scratchpad.svg",
+  invertIconForLightTheme: true,
   url: "chrome://browser/content/devtools/scratchpad.xul",
   label: l10n("scratchpad.label", scratchpadStrings),
   tooltip: l10n("scratchpad.tooltip", scratchpadStrings),
   inMenu: false,
 
   isTargetSupported: function(target) {
     return target.isRemote;
   },
--- a/browser/devtools/responsivedesign/CmdResize.jsm
+++ b/browser/devtools/responsivedesign/CmdResize.jsm
@@ -31,17 +31,17 @@ gcli.addCommand({
   description: gcli.lookup('resizeModeOffDesc'),
   manual: gcli.lookupFormat('resizeModeManual2', [BRAND_SHORT_NAME]),
   exec: gcli_cmd_resize
 });
 
 gcli.addCommand({
   name: 'resize toggle',
   buttonId: "command-button-responsive",
-  buttonClass: "command-button",
+  buttonClass: "command-button command-button-invertable",
   tooltipText: gcli.lookup("resizeModeToggleTooltip"),
   description: gcli.lookup('resizeModeToggleDesc'),
   manual: gcli.lookupFormat('resizeModeManual2', [BRAND_SHORT_NAME]),
   state: {
     isChecked: function(aTarget) {
       let browserWindow = aTarget.tab.ownerDocument.defaultView;
       let mgr = browserWindow.ResponsiveUI.ResponsiveUIManager;
       return mgr.isActiveForTab(aTarget.tab);
--- a/browser/devtools/scratchpad/CmdScratchpad.jsm
+++ b/browser/devtools/scratchpad/CmdScratchpad.jsm
@@ -7,16 +7,16 @@ this.EXPORTED_SYMBOLS = [ ];
 Components.utils.import("resource://gre/modules/devtools/gcli.jsm");
 
 /**
  * 'scratchpad' command
  */
 gcli.addCommand({
   name: "scratchpad",
   buttonId: "command-button-scratchpad",
-  buttonClass: "command-button",
+  buttonClass: "command-button command-button-invertable",
   tooltipText: gcli.lookup("scratchpadOpenTooltip"),
   hidden: true,
   exec: function(args, context) {
     let chromeWindow = context.environment.chromeDocument.defaultView;
     chromeWindow.Scratchpad.ScratchpadManager.openScratchpad();
   }
 });
--- a/browser/devtools/tilt/CmdTilt.jsm
+++ b/browser/devtools/tilt/CmdTilt.jsm
@@ -49,17 +49,17 @@ gcli.addCommand({
 
 
 /**
  * 'tilt toggle' command
  */
 gcli.addCommand({
   name: "tilt toggle",
   buttonId: "command-button-tilt",
-  buttonClass: "command-button",
+  buttonClass: "command-button command-button-invertable",
   tooltipText: gcli.lookup("tiltToggleTooltip"),
   hidden: true,
   state: {
     isChecked: function(aTarget) {
       let browserWindow = aTarget.tab.ownerDocument.defaultView;
       return !!TiltManager.getTiltForBrowser(browserWindow).currentInstance;
     },
     onChange: function(aTarget, aChangeHandler) {
--- a/browser/themes/shared/devtools/toolbars.inc.css
+++ b/browser/themes/shared/devtools/toolbars.inc.css
@@ -748,37 +748,37 @@
 .devtools-tab[selected] > .highlighted-icon,
 .devtools-tab:not([selected])[highlighted] > .default-icon {
   visibility: collapse;
 }
 
 /* Invert the colors of certain dark theme images for displaying
  * inside of the light theme.
  */
-.theme-light .devtools-tab > image,
+.theme-light .devtools-tab[icon-invertable] > image,
 .theme-light #toolbox-dock-buttons > toolbarbutton > image,
-.theme-light .command-button > image,
-.theme-light .command-button:active > image,
+.theme-light .command-button-invertable > image,
+.theme-light .command-button-invertable:active > image,
 .theme-light .devtools-closebutton > image,
 .theme-light .devtools-toolbarbutton > image,
 .theme-light .devtools-option-toolbarbutton > image,
 .theme-light #breadcrumb-separator-normal,
 .theme-light .scrollbutton-up > .toolbarbutton-icon,
 .theme-light .scrollbutton-down > .toolbarbutton-icon,
 .theme-light #black-boxed-message-button .button-icon,
 .theme-light #requests-menu-perf-notice-button .button-icon,
 .theme-light #requests-menu-network-summary-button .button-icon {
   filter: url(filters.svg#invert);
 }
 
 /* Since selected backgrounds are blue, we want to use the normal
  * (light) icons. */
-.theme-light .command-button[checked=true]:not(:active) > image,
-.theme-light .devtools-tab[selected] > image,
-.theme-light .devtools-tab[highlighted] > image,
+.theme-light .command-button-invertable[checked=true]:not(:active) > image,
+.theme-light .devtools-tab[icon-invertable][selected] > image,
+.theme-light .devtools-tab[icon-invertable][highlighted] > image,
 .theme-light .devtools-option-toolbarbutton[open] > image,
 .theme-light #resume[checked] > image {
   filter: none !important;
 }
 
 .theme-light .command-button:hover {
   background-color: inherit;
 }