Bug 1456069 - Always show frame button if user is on options panel;r=jryans,birtles draft
authorJulian Descottes <jdescottes@mozilla.com>
Mon, 23 Apr 2018 11:20:12 +0200
changeset 792377 bfabb0095ef3f1ebb4db69955853d2ee1510cfa3
parent 792152 2af6164b5677dc5807a7d3a9cf9c04080aa545e3
push id109098
push userjdescottes@mozilla.com
push dateTue, 08 May 2018 09:55:22 +0000
reviewersjryans, birtles
bugs1456069
milestone61.0a1
Bug 1456069 - Always show frame button if user is on options panel;r=jryans,birtles MozReview-Commit-ID: CqwcVlxrQMd
devtools/client/framework/test/browser.ini
devtools/client/framework/test/browser_toolbox_options_frames_button.js
devtools/client/framework/toolbox.js
devtools/client/locales/en-US/toolbox.properties
--- a/devtools/client/framework/test/browser.ini
+++ b/devtools/client/framework/test/browser.ini
@@ -83,16 +83,17 @@ skip-if = os == 'win' || debug # Bug 128
 skip-if = os == "mac" # Full keyboard navigation on OSX only works if Full Keyboard Access setting is set to All Control in System Keyboard Preferences
 [browser_toolbox_options.js]
 [browser_toolbox_options_multiple_tabs.js]
 [browser_toolbox_options_disable_buttons.js]
 [browser_toolbox_options_disable_cache-01.js]
 [browser_toolbox_options_disable_cache-02.js]
 [browser_toolbox_options_disable_js.js]
 [browser_toolbox_options_enable_serviceworkers_testing.js]
+[browser_toolbox_options_frames_button.js]
 [browser_toolbox_raise.js]
 disabled=Bug 962258
 [browser_toolbox_races.js]
 [browser_toolbox_ready.js]
 [browser_toolbox_remoteness_change.js]
 run-if = e10s
 [browser_toolbox_select_event.js]
 skip-if = e10s # Bug 1069044 - destroyInspector may hang during shutdown
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/test/browser_toolbox_options_frames_button.js
@@ -0,0 +1,58 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that the frames button is always visible when the user is on the options panel.
+// Test that the button is disabled if the current target has no frames.
+// Test that the button is enabled otherwise.
+
+const TEST_URL = "data:text/html;charset=utf8,test frames button visibility";
+const TEST_URL_FRAMES = TEST_URL + "<iframe src=\"data:text/plain,iframe\"></iframe>";
+const FRAME_BUTTON_PREF = "devtools.command-button-frames.enabled";
+
+add_task(async function() {
+  // Hide the button by default.
+  await pushPref(FRAME_BUTTON_PREF, false);
+
+  let tab = await addTab(TEST_URL);
+  let target = TargetFactory.forTab(tab);
+
+  info("Open the toolbox on the Options panel");
+  let toolbox = await gDevTools.showToolbox(target, "options");
+  let doc = toolbox.doc;
+
+  let optionsPanel = toolbox.getCurrentPanel();
+
+  let framesButton = doc.getElementById("command-button-frames");
+  ok(!framesButton, "Frames button is not rendered.");
+
+  let optionsDoc = optionsPanel.panelWin.document;
+  let framesButtonCheckbox = optionsDoc.getElementById("command-button-frames");
+  framesButtonCheckbox.click();
+
+  framesButton = doc.getElementById("command-button-frames");
+  ok(framesButton, "Frames button is rendered.");
+  ok(framesButton.disabled, "Frames button is disabled.");
+
+  info("Leave the options panel, the frames button should not be rendered.");
+  await toolbox.selectTool("webconsole");
+  framesButton = doc.getElementById("command-button-frames");
+  ok(!framesButton, "Frames button is no longer rendered.");
+
+  info("Go back to the options panel, the frames button should rendered.");
+  await toolbox.selectTool("options");
+  framesButton = doc.getElementById("command-button-frames");
+  ok(framesButton, "Frames button is rendered again.");
+
+  info("Navigate to a page with frames, the frames button should be enabled.");
+  await BrowserTestUtils.loadURI(tab.linkedBrowser, TEST_URL_FRAMES);
+
+  framesButton = doc.getElementById("command-button-frames");
+  ok(framesButton, "Frames button is still rendered.");
+
+  await waitUntil(() => !framesButton.disabled);
+  ok(!framesButton.disabled, "Frames button is not disabled.");
+
+  Services.prefs.clearUserPref(FRAME_BUTTON_PREF);
+});
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -154,17 +154,18 @@ function Toolbox(target, selectedTool, h
   this._onToolbarFocus = this._onToolbarFocus.bind(this);
   this._onToolbarArrowKeypress = this._onToolbarArrowKeypress.bind(this);
   this._onPickerClick = this._onPickerClick.bind(this);
   this._onPickerKeypress = this._onPickerKeypress.bind(this);
   this._onPickerStarted = this._onPickerStarted.bind(this);
   this._onPickerStopped = this._onPickerStopped.bind(this);
   this._onInspectObject = this._onInspectObject.bind(this);
   this._onNewSelectedNodeFront = this._onNewSelectedNodeFront.bind(this);
-  this.updatePickerButton = this.updatePickerButton.bind(this);
+  this._onToolSelected = this._onToolSelected.bind(this);
+  this.updateToolboxButtonsVisibility = this.updateToolboxButtonsVisibility.bind(this);
   this.selectTool = this.selectTool.bind(this);
   this._pingTelemetrySelectTool = this._pingTelemetrySelectTool.bind(this);
   this.toggleSplitConsole = this.toggleSplitConsole.bind(this);
 
   this._target.on("close", this.destroy);
 
   if (!selectedTool) {
     selectedTool = Services.prefs.getCharPref(this._prefs.LAST_TOOL);
@@ -179,19 +180,17 @@ function Toolbox(target, selectedTool, h
   EventEmitter.decorate(this);
 
   this._target.on("will-navigate", this._onWillNavigate);
   this._target.on("navigate", this._refreshHostTitle);
   this._target.on("frame-update", this._updateFrames);
   this._target.on("inspect-object", this._onInspectObject);
 
   this.on("host-changed", this._refreshHostTitle);
-  this.on("select", this._refreshHostTitle);
-  this.on("select", this.updatePickerButton);
-
+  this.on("select", this._onToolSelected);
   this.on("ready", this._showDevEditionPromo);
 
   gDevTools.on("tool-registered", this._toolRegistered);
   gDevTools.on("tool-unregistered", this._toolUnregistered);
 
   this.on("picker-started", this._onPickerStarted);
   this.on("picker-stopped", this._onPickerStopped);
 
@@ -1242,17 +1241,19 @@ Toolbox.prototype = {
     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;
+        const hasFrames = this.frameMap.size > 1;
+        const isOnOptionsPanel = this.currentToolId === "options";
+        return hasFrames || isOnOptionsPanel;
       },
       onKeyDown: this.handleKeyDownOnFramesButton
     });
 
     return this.frameButton;
   },
 
   /**
@@ -1371,18 +1372,33 @@ Toolbox.prototype = {
       currentPanel.updatePickerButton();
     } else {
       // If the current panel doesn't define a custom updatePickerButton,
       // revert the button to its default state
       button.description = L10N.getStr("pickButton.tooltip");
       button.className = null;
       button.disabled = null;
     }
-
-    this.component.setToolboxButtons(this.toolbarButtons);
+  },
+
+  /**
+   * Update the visual state of the Frame picker button.
+   */
+  updateFrameButton() {
+    if (this.currentToolId === "options" && this.frameMap.size <= 1) {
+      // If the button is only visible because the user is on the Options panel, disable
+      // the button and set an appropriate description.
+      this.frameButton.disabled = true;
+      this.frameButton.description = L10N.getStr("toolbox.frames.disabled.tooltip");
+    } else {
+      // Otherwise, enable the button and update the description.
+      this.frameButton.disabled = false;
+      this.frameButton.description = L10N.getStr("toolbox.frames.tooltip");
+    }
+    this.frameButton.isVisible = this._commandIsVisible(this.frameButton);
   },
 
   /**
    * Ensure the visibility of each toolbox button matches the preference value.
    */
   _commandIsVisible: function(button) {
     const {
       isTargetSupported,
@@ -2370,17 +2386,17 @@ Toolbox.prototype = {
 
     // 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.updateFrameButton();
     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.
@@ -2622,16 +2638,26 @@ Toolbox.prototype = {
     // provide the `devtools.panels.elements.onSelectionChanged` event).
     this.emit("selection-changed");
   },
 
   _onInspectObject: function(packet) {
     this.inspectObjectActor(packet.objectActor, packet.inspectFromAnnotation);
   },
 
+  _onToolSelected: function() {
+    this._refreshHostTitle();
+
+    this.updatePickerButton();
+    this.updateFrameButton();
+
+    // Calling setToolboxButtons in case the visibility of a button changed.
+    this.component.setToolboxButtons(this.toolbarButtons);
+  },
+
   inspectObjectActor: async function(objectActor, inspectFromAnnotation) {
     if (objectActor.preview &&
         objectActor.preview.nodeType === domNodeConstants.ELEMENT_NODE) {
       // Open the inspector and select the DOM Element.
       await this.loadTool("inspector");
       const inspector = this.getPanel("inspector");
       const nodeFound = await inspector.inspectNodeActor(objectActor.actor,
                                                          inspectFromAnnotation);
@@ -2727,18 +2753,17 @@ Toolbox.prototype = {
     this._destroyer = deferred.promise;
 
     this.emit("destroy");
 
     this._target.off("inspect-object", this._onInspectObject);
     this._target.off("will-navigate", this._onWillNavigate);
     this._target.off("navigate", this._refreshHostTitle);
     this._target.off("frame-update", this._updateFrames);
-    this.off("select", this._refreshHostTitle);
-    this.off("select", this.updatePickerButton);
+    this.off("select", this._onToolSelected);
     this.off("host-changed", this._refreshHostTitle);
     this.off("ready", this._showDevEditionPromo);
 
     gDevTools.off("tool-registered", this._toolRegistered);
     gDevTools.off("tool-unregistered", this._toolUnregistered);
 
     Services.prefs.removeObserver("devtools.cache.disabled", this._applyCacheSettings);
     Services.prefs.removeObserver("devtools.serviceWorkers.testing.enabled",
--- a/devtools/client/locales/en-US/toolbox.properties
+++ b/devtools/client/locales/en-US/toolbox.properties
@@ -133,16 +133,22 @@ toolbox.forceReload2.key=CmdOrCtrl+F5
 # Key shortcut used to move the toolbox in bottom or side of the browser window
 toolbox.toggleHost.key=CmdOrCtrl+Shift+D
 
 # LOCALIZATION NOTE (toolbox.frames.tooltip): This is the label for
 # the iframes menu list that appears only when the document has some.
 # It allows you to switch the context of the whole toolbox.
 toolbox.frames.tooltip=Select an iframe as the currently targeted document
 
+# LOCALIZATION NOTE (toolbox.frames.disabled.tooltip): This is the title
+# displayed as a tooltip of the iframes menu button, when disabled. The button
+# is normally hidden when no frames are available. But if the user is on the
+# DevTools Options panel, the button is always shown for discoverability.
+toolbox.frames.disabled.tooltip=This button is only available on pages with several iframes
+
 # LOCALIZATION NOTE (toolbox.showFrames.key)
 # Key shortcut used to show frames menu when 'frames' button is focused
 toolbox.showFrames.key=Alt+Down
 
 # LOCALIZATION NOTE (toolbox.meatballMenu.button.tooltip): This is the tooltip
 # for the "..." button on the developer tools toolbox.
 toolbox.meatballMenu.button.tooltip=Customize Developer Tools and get help