Bug 1451592 - Hide the iframe button when there are no child iframes; r=jryans
authorBrian Birtles <birtles@gmail.com>
Mon, 16 Apr 2018 19:49:38 +0200
changeset 467511 00c968ee4e0c02b8491dfb4c905c862a34ccb94e
parent 467510 4c40ad65f4724f1e647002dc0f84b6b71cae5f50
child 467512 2ccaad0350355586508ac2b4f8acfa80a00f39f7
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjryans
bugs1451592
milestone61.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 1451592 - Hide the iframe button when there are no child iframes; r=jryans MozReview-Commit-ID: 7Q3PojIzmy4
devtools/client/framework/components/toolbox-toolbar.js
devtools/client/framework/test/browser_toolbox_options_disable_buttons.js
devtools/client/framework/toolbox-options.js
devtools/client/framework/toolbox.js
--- a/devtools/client/framework/components/toolbox-toolbar.js
+++ b/devtools/client/framework/components/toolbox-toolbar.js
@@ -172,26 +172,26 @@ function renderToolboxButtons({focusedBu
           onKeyDown(event);
         }
       });
     });
 
   // Add the appropriate separator, if needed.
   let children = renderedButtons;
   if (renderedButtons.length) {
+    if (isStart) {
+      children.push(renderSeparator());
     // For the end group we add a separator *before* the RDM button if it
-    // exists.
-    if (rdmIndex !== -1) {
+    // exists, but only if it is not the only button.
+    } else if (rdmIndex !== -1 && visibleButtons.length > 1) {
       children.splice(
         children.length - 1,
         0,
         renderSeparator()
       );
-    } else {
-      children.push(renderSeparator());
     }
   }
 
   return div({id: `toolbox-buttons-${isStart ? "start" : "end"}`}, ...children);
 }
 
 /**
  * Render a separator.
--- a/devtools/client/framework/test/browser_toolbox_options_disable_buttons.js
+++ b/devtools/client/framework/test/browser_toolbox_options_disable_buttons.js
@@ -1,17 +1,22 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-const TEST_URL = "data:text/html;charset=utf8,test for dynamically " +
-                 "registering and unregistering tools";
+let TEST_URL = "data:text/html;charset=utf8,test for dynamically " +
+               "registering and unregistering tools";
+
+// The frames button is only shown if the page has at least one iframe so we
+// need to add one to the test page.
+TEST_URL += "<iframe src=\"data:text/plain,iframe\"></iframe>";
+
 var doc = null, toolbox = null, panelWin = null, modifiedPrefs = [];
 
 function test() {
   addTab(TEST_URL).then(tab => {
     let target = TargetFactory.forTab(tab);
     gDevTools.showToolbox(target)
       .then(testSelectTool)
       .then(testToggleToolboxButtons)
--- a/devtools/client/framework/toolbox-options.js
+++ b/devtools/client/framework/toolbox-options.js
@@ -179,17 +179,17 @@ OptionsPanel.prototype = {
 
     let createCommandCheckbox = button => {
       let checkboxLabel = this.panelDoc.createElement("label");
       let checkboxSpanLabel = this.panelDoc.createElement("span");
       checkboxSpanLabel.textContent = button.description;
       let checkboxInput = this.panelDoc.createElement("input");
       checkboxInput.setAttribute("type", "checkbox");
       checkboxInput.setAttribute("id", button.id);
-      if (button.isVisible) {
+      if (Services.prefs.getBoolPref(button.visibilityswitch, true)) {
         checkboxInput.setAttribute("checked", true);
       }
       checkboxInput.addEventListener("change",
         onCheckboxClick.bind(this, checkboxInput));
 
       checkboxLabel.appendChild(checkboxInput);
       checkboxLabel.appendChild(checkboxSpanLabel);
       return checkboxLabel;
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -761,32 +761,35 @@ Toolbox.prototype = {
    *                       argument, to be called whenever the checked state changes.
    * @property {Function} teardown - Function run on toolbox close to let a chance to
    *                      unregister listeners set when `setup` was called and avoid
    *                      memory leaks. The same arguments than `setup` function are
    *                      passed to `teardown`.
    * @property {Function} isTargetSupported - Function to automatically enable/disable
    *                      the button based on the target. If the target don't support
    *                      the button feature, this method should return false.
+   * @property {Function} isCurrentlyVisible - Function to automatically
+   *                      hide/show the button based on current state.
    * @property {Function} isChecked - Optional function called to known if the button
    *                      is toggled or not. The function should return true when
    *                      the button should be displayed as toggled on.
    */
   _createButtonState: function(options) {
     let isCheckedValue = false;
     const {
       id,
       className,
       description,
       disabled,
       onClick,
       isInStartContainer,
       setup,
       teardown,
       isTargetSupported,
+      isCurrentlyVisible,
       isChecked,
       onKeyDown
     } = options;
     const toolbox = this;
     const button = {
       id,
       className,
       description,
@@ -797,16 +800,17 @@ Toolbox.prototype = {
         }
       },
       onKeyDown(event) {
         if (typeof onKeyDown == "function") {
           onKeyDown(event, toolbox);
         }
       },
       isTargetSupported,
+      isCurrentlyVisible,
       get isChecked() {
         if (typeof isChecked == "function") {
           return isChecked(toolbox);
         }
         return isCheckedValue;
       },
       set isChecked(value) {
         // Note that if options.isChecked is given, this is ignored
@@ -1235,16 +1239,19 @@ Toolbox.prototype = {
   _buildFrameButton() {
     this.frameButton = this._createButtonState({
       id: "command-button-frames",
       description: L10N.getStr("toolbox.frames.tooltip"),
       onClick: this.showFramesMenu,
       isTargetSupported: target => {
         return target.activeTab && target.activeTab.traits.frames;
       },
+      isCurrentlyVisible: () => {
+        return this.frameMap.size > 1;
+      },
       onKeyDown: this.handleKeyDownOnFramesButton
     });
 
     return this.frameButton;
   },
 
   /**
    * Toggle the picker, but also decide whether or not the highlighter should
@@ -1376,26 +1383,33 @@ Toolbox.prototype = {
   },
 
   /**
    * Ensure the visibility of each toolbox button matches the preference value.
    */
   _commandIsVisible: function(button) {
     const {
       isTargetSupported,
+      isCurrentlyVisible,
       visibilityswitch
     } = button;
 
-    let visible = Services.prefs.getBoolPref(visibilityswitch, true);
-
-    if (isTargetSupported) {
-      return visible && isTargetSupported(this.target);
+    if (!Services.prefs.getBoolPref(visibilityswitch, true)) {
+      return false;
+    }
+
+    if (isTargetSupported && !isTargetSupported(this.target)) {
+      return false;
     }
 
-    return visible;
+    if (isCurrentlyVisible && !isCurrentlyVisible()) {
+      return false;
+    }
+
+    return true;
   },
 
   /**
    * Build a panel for a tool definition.
    *
    * @param {string} toolDefinition
    *        Tool definition of the tool to build a tab for.
    */
@@ -2247,20 +2261,16 @@ Toolbox.prototype = {
    *                 id {Number}: frame ID
    *                 url {String}: frame URL
    *                 title {String}: frame title
    *                 destroy {Boolean}: Set to true if destroyed
    *                 parentID {Number}: ID of the parent frame (not set
    *                                    for top level window)
    */
   _updateFrames: function(data) {
-    if (!Services.prefs.getBoolPref("devtools.command-button-frames.enabled")) {
-      return;
-    }
-
     // We may receive this event before the toolbox is ready.
     if (!this.isReady) {
       return;
     }
 
     // Store (synchronize) data about all existing frames on the backend
     if (data.destroyAll) {
       this.frameMap.clear();
@@ -2297,16 +2307,20 @@ Toolbox.prototype = {
     let topFrameSelected = frame ? !frame.parentID : false;
     this._framesButtonChecked = false;
 
     // If non-top level frame is selected the toolbar button is
     // marked as 'checked' indicating that a child frame is active.
     if (!topFrameSelected && this.selectedFrameId) {
       this._framesButtonChecked = false;
     }
+
+    // We may need to hide/show the frames button now.
+    this.frameButton.isVisible = this._commandIsVisible(this.frameButton);
+    this.component.setToolboxButtons(this.toolbarButtons);
   },
 
   /**
    * Returns a 0-based selected frame depth.
    *
    * For example, if the root frame is selected, the returned value is 0.  For a sub-frame
    * of the root document, the returned value is 1, and so on.
    */