Bug 1278552 - Make sidebar containers focusable so shortcuts work when they are active; r=jdescottes
authorPatrick Brosset <pbrosset@mozilla.com>
Mon, 18 Jul 2016 14:38:10 +0200
changeset 348404 129c925e216d801c622922a669e720dc1d5fb5a5
parent 348403 a7a882d122e36defe6c2a102a28ae7dfc16311c5
child 348405 e5ebff5eb655ffefbc777d1ca424b56dc8ce0283
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes
bugs1278552
milestone50.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 1278552 - Make sidebar containers focusable so shortcuts work when they are active; r=jdescottes The rule-view and computed-view now have tabindex="-1" on their containers so that clicking anywhere in the views will focus them, so will tabbing through the inspector to them. This means that any user activity on them ends up focusing them (or any focusable element in them, which was the case before anyway). Because of this, pressing ctrl+F (or any other shortcut) will work correctly: the _onShortcut callback in each of these views will detect that the active element is the view itself and will therefore handle the event. Note that the container element that has the tabindex attribute is a child of the main container element that is scrollable (overflow:auto). That is because if we use only one element for both, then grabbing the scrollbar will focus the element and blur any field that's currently focused. MozReview-Commit-ID: 97a1tpdlbmZ
devtools/client/inspector/inspector.xul
devtools/client/inspector/rules/rules.js
devtools/client/inspector/rules/test/browser_rules_edit-selector-click-on-scrollbar.js
devtools/client/inspector/test/browser.ini
devtools/client/inspector/test/browser_inspector_search-sidebar.js
devtools/client/themes/computed.css
devtools/client/themes/rules.css
--- a/devtools/client/inspector/inspector.xul
+++ b/devtools/client/inspector/inspector.xul
@@ -86,16 +86,18 @@
           <html:div id="pseudo-class-panel" hidden="true">
             <html:label><html:input id="pseudo-hover-toggle" type="checkbox" value=":hover" tabindex="-1" />:hover</html:label>
             <html:label><html:input id="pseudo-active-toggle" type="checkbox" value=":active" tabindex="-1" />:active</html:label>
             <html:label><html:input id="pseudo-focus-toggle" type="checkbox" value=":focus" tabindex="-1" />:focus</html:label>
         </html:div>
         </html:div>
 
         <html:div id="ruleview-container" class="ruleview">
+          <html:div id="ruleview-container-focusable" tabindex="-1">
+          </html:div>
         </html:div>
       </html:div>
 
       <html:div id="sidebar-panel-computedview" class="devtools-monospace theme-sidebar inspector-tabpanel">
         <html:div class="devtools-toolbar">
           <html:div class="devtools-searchbox">
             <html:input id="computedview-searchbox"
                         class="devtools-filterinput devtools-rule-searchbox"
@@ -106,73 +108,75 @@
           <html:label id="browser-style-checkbox-label" for="browser-style-checkbox">
             <html:input id="browser-style-checkbox"
                         type="checkbox"
                         class="includebrowserstyles"
                         label="&browserStylesLabel;"/>&browserStylesLabel;</html:label>
         </html:div>
 
         <html:div id="computedview-container">
-          <html:div id="layout-wrapper" class="theme-separator" tabindex="0">
-            <html:div id="layout-header">
-              <html:div id="layout-expander" class="expander theme-twisty expandable" open=""></html:div>
-              <html:span>&layoutViewTitle;</html:span>
-              <html:button class="devtools-button" id="layout-geometry-editor" title="&geometry.button.tooltip;"></html:button>
-            </html:div>
+          <html:div id="computedview-container-focusable" tabindex="-1">
+            <html:div id="layout-wrapper" class="theme-separator" tabindex="0">
+              <html:div id="layout-header">
+                <html:div id="layout-expander" class="expander theme-twisty expandable" open=""></html:div>
+                <html:span>&layoutViewTitle;</html:span>
+                <html:button class="devtools-button" id="layout-geometry-editor" title="&geometry.button.tooltip;"></html:button>
+              </html:div>
 
-            <html:div id="layout-container">
-              <html:div id="layout-main">
-                <html:span class="layout-legend" data-box="margin" title="&margin.tooltip;">&margin.tooltip;</html:span>
-                <html:div id="layout-margins" data-box="margin" title="&margin.tooltip;">
-                  <html:span class="layout-legend" data-box="border" title="&border.tooltip;">&border.tooltip;</html:span>
-                  <html:div id="layout-borders" data-box="border" title="&border.tooltip;">
-                    <html:span class="layout-legend" data-box="padding" title="&padding.tooltip;">&padding.tooltip;</html:span>
-                    <html:div id="layout-padding" data-box="padding" title="&padding.tooltip;">
-                      <html:div id="layout-content" data-box="content" title="&content.tooltip;">
+              <html:div id="layout-container">
+                <html:div id="layout-main">
+                  <html:span class="layout-legend" data-box="margin" title="&margin.tooltip;">&margin.tooltip;</html:span>
+                  <html:div id="layout-margins" data-box="margin" title="&margin.tooltip;">
+                    <html:span class="layout-legend" data-box="border" title="&border.tooltip;">&border.tooltip;</html:span>
+                    <html:div id="layout-borders" data-box="border" title="&border.tooltip;">
+                      <html:span class="layout-legend" data-box="padding" title="&padding.tooltip;">&padding.tooltip;</html:span>
+                      <html:div id="layout-padding" data-box="padding" title="&padding.tooltip;">
+                        <html:div id="layout-content" data-box="content" title="&content.tooltip;">
+                        </html:div>
                       </html:div>
                     </html:div>
                   </html:div>
+
+                  <html:p class="layout-margin layout-top"><html:span data-box="margin" class="layout-editable" title="margin-top"></html:span></html:p>
+                  <html:p class="layout-margin layout-right"><html:span data-box="margin" class="layout-editable" title="margin-right"></html:span></html:p>
+                  <html:p class="layout-margin layout-bottom"><html:span data-box="margin" class="layout-editable" title="margin-bottom"></html:span></html:p>
+                  <html:p class="layout-margin layout-left"><html:span data-box="margin" class="layout-editable" title="margin-left"></html:span></html:p>
+
+                  <html:p class="layout-border layout-top"><html:span data-box="border" class="layout-editable" title="border-top"></html:span></html:p>
+                  <html:p class="layout-border layout-right"><html:span data-box="border" class="layout-editable" title="border-right"></html:span></html:p>
+                  <html:p class="layout-border layout-bottom"><html:span data-box="border" class="layout-editable" title="border-bottom"></html:span></html:p>
+                  <html:p class="layout-border layout-left"><html:span data-box="border" class="layout-editable" title="border-left"></html:span></html:p>
+
+                  <html:p class="layout-padding layout-top"><html:span data-box="padding" class="layout-editable" title="padding-top"></html:span></html:p>
+                  <html:p class="layout-padding layout-right"><html:span data-box="padding" class="layout-editable" title="padding-right"></html:span></html:p>
+                  <html:p class="layout-padding layout-bottom"><html:span data-box="padding" class="layout-editable" title="padding-bottom"></html:span></html:p>
+                  <html:p class="layout-padding layout-left"><html:span data-box="padding" class="layout-editable" title="padding-left"></html:span></html:p>
+
+                  <html:p class="layout-size"><html:span data-box="content" title="&content.tooltip;"></html:span></html:p>
                 </html:div>
 
-                <html:p class="layout-margin layout-top"><html:span data-box="margin" class="layout-editable" title="margin-top"></html:span></html:p>
-                <html:p class="layout-margin layout-right"><html:span data-box="margin" class="layout-editable" title="margin-right"></html:span></html:p>
-                <html:p class="layout-margin layout-bottom"><html:span data-box="margin" class="layout-editable" title="margin-bottom"></html:span></html:p>
-                <html:p class="layout-margin layout-left"><html:span data-box="margin" class="layout-editable" title="margin-left"></html:span></html:p>
-
-                <html:p class="layout-border layout-top"><html:span data-box="border" class="layout-editable" title="border-top"></html:span></html:p>
-                <html:p class="layout-border layout-right"><html:span data-box="border" class="layout-editable" title="border-right"></html:span></html:p>
-                <html:p class="layout-border layout-bottom"><html:span data-box="border" class="layout-editable" title="border-bottom"></html:span></html:p>
-                <html:p class="layout-border layout-left"><html:span data-box="border" class="layout-editable" title="border-left"></html:span></html:p>
+                <html:div id="layout-info">
+                  <html:span id="layout-element-size"></html:span>
+                  <html:section id="layout-position-group">
+                    <html:span id="layout-element-position"></html:span>
+                  </html:section>
+                </html:div>
 
-                <html:p class="layout-padding layout-top"><html:span data-box="padding" class="layout-editable" title="padding-top"></html:span></html:p>
-                <html:p class="layout-padding layout-right"><html:span data-box="padding" class="layout-editable" title="padding-right"></html:span></html:p>
-                <html:p class="layout-padding layout-bottom"><html:span data-box="padding" class="layout-editable" title="padding-bottom"></html:span></html:p>
-                <html:p class="layout-padding layout-left"><html:span data-box="padding" class="layout-editable" title="padding-left"></html:span></html:p>
-
-                <html:p class="layout-size"><html:span data-box="content" title="&content.tooltip;"></html:span></html:p>
-              </html:div>
-
-              <html:div id="layout-info">
-                <html:span id="layout-element-size"></html:span>
-                <html:section id="layout-position-group">
-                  <html:span id="layout-element-position"></html:span>
-                </html:section>
-              </html:div>
-
-              <html:div style="display: none">
-                <html:p id="layout-dummy"></html:p>
+                <html:div style="display: none">
+                  <html:p id="layout-dummy"></html:p>
+                </html:div>
               </html:div>
             </html:div>
-          </html:div>
+
+            <html:div id="propertyContainer" class="theme-separator" tabindex="0">
+            </html:div>
 
-          <html:div id="propertyContainer" class="theme-separator" tabindex="0">
-          </html:div>
-
-          <html:div id="computedview-no-results" hidden="">
-            &noPropertiesFound;
+            <html:div id="computedview-no-results" hidden="">
+              &noPropertiesFound;
+            </html:div>
           </html:div>
         </html:div>
       </html:div>
 
       <html:div id="sidebar-panel-fontinspector" class="devtools-monospace theme-sidebar inspector-tabpanel">
         <html:div class="devtools-toolbar">
           <html:div class="devtools-searchbox">
             <html:input id="font-preview-text-input"
--- a/devtools/client/inspector/rules/rules.js
+++ b/devtools/client/inspector/rules/rules.js
@@ -169,17 +169,17 @@ function CssRuleView(inspector, document
   this._onFilterStyles = this._onFilterStyles.bind(this);
   this._onClearSearch = this._onClearSearch.bind(this);
   this._onFilterTextboxContextMenu =
     this._onFilterTextboxContextMenu.bind(this);
   this._onTogglePseudoClassPanel = this._onTogglePseudoClassPanel.bind(this);
   this._onTogglePseudoClass = this._onTogglePseudoClass.bind(this);
 
   let doc = this.styleDocument;
-  this.element = doc.getElementById("ruleview-container");
+  this.element = doc.getElementById("ruleview-container-focusable");
   this.addRuleButton = doc.getElementById("ruleview-add-rule-button");
   this.searchField = doc.getElementById("ruleview-searchbox");
   this.searchClearButton = doc.getElementById("ruleview-searchinput-clear");
   this.pseudoClassPanel = doc.getElementById("pseudo-class-panel");
   this.pseudoClassToggle = doc.getElementById("pseudo-class-panel-toggle");
   this.hoverCheckbox = doc.getElementById("pseudo-hover-toggle");
   this.activeCheckbox = doc.getElementById("pseudo-active-toggle");
   this.focusCheckbox = doc.getElementById("pseudo-focus-toggle");
--- a/devtools/client/inspector/rules/test/browser_rules_edit-selector-click-on-scrollbar.js
+++ b/devtools/client/inspector/rules/test/browser_rules_edit-selector-click-on-scrollbar.js
@@ -71,17 +71,17 @@ add_task(function* () {
   info("Enter new value and commit.");
   editor.input.value = newValue;
   EventUtils.synthesizeKey("VK_RETURN", {});
   yield onRuleViewChanged;
   ok(getRuleViewRule(view, newValue), "Rule with '" + newValue + " 'exists.");
 });
 
 function* clickOnRuleviewScrollbar(view) {
-  let container = view.element;
+  let container = view.element.parentNode;
   let onScroll = once(container, "scroll");
   let rect = container.getBoundingClientRect();
   // click 5 pixels before the bottom-right corner should hit the scrollbar
   EventUtils.synthesizeMouse(container, rect.width - 5, rect.height - 5,
     {}, view.styleWindow);
   yield onScroll;
 
   ok(true, "The rule view container scrolled after clicking on the scrollbar.");
--- a/devtools/client/inspector/test/browser.ini
+++ b/devtools/client/inspector/test/browser.ini
@@ -133,13 +133,14 @@ skip-if = os == "mac" # Full keyboard na
 [browser_inspector_search-04.js]
 [browser_inspector_search-05.js]
 [browser_inspector_search-06.js]
 [browser_inspector_search-07.js]
 [browser_inspector_search-08.js]
 [browser_inspector_search_keyboard_trap.js]
 [browser_inspector_search-reserved.js]
 [browser_inspector_search-selection.js]
+[browser_inspector_search-sidebar.js]
 [browser_inspector_select-docshell.js]
 [browser_inspector_select-last-selected.js]
 [browser_inspector_search-navigation.js]
 [browser_inspector_sidebarstate.js]
 [browser_inspector_switch-to-inspector-on-pick.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/test/browser_inspector_search-sidebar.js
@@ -0,0 +1,74 @@
+/* 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";
+
+// Test that depending where the user last clicked in the inspector, the right search
+// field is focused when ctrl+F is pressed.
+
+add_task(function* () {
+  let {inspector} = yield openInspectorForURL("data:text/html;charset=utf-8,Search!");
+
+  info("Check that by default, the inspector search field gets focused");
+  pressCtrlF();
+  isInInspectorSearchBox(inspector);
+
+  info("Click somewhere in the rule-view");
+  clickInRuleView(inspector);
+
+  info("Check that the rule-view search field gets focused");
+  pressCtrlF();
+  isInRuleViewSearchBox(inspector);
+
+  info("Click in the inspector again");
+  yield clickContainer("head", inspector);
+
+  info("Check that now we're back in the inspector, its search field gets focused");
+  pressCtrlF();
+  isInInspectorSearchBox(inspector);
+
+  info("Switch to the computed view, and click somewhere inside it");
+  selectComputedView(inspector);
+  clickInComputedView(inspector);
+
+  info("Check that the computed-view search field gets focused");
+  pressCtrlF();
+  isInComputedViewSearchBox(inspector);
+
+  info("Click in the inspector yet again");
+  yield clickContainer("body", inspector);
+
+  info("We're back in the inspector again, check the inspector search field focuses");
+  pressCtrlF();
+  isInInspectorSearchBox(inspector);
+});
+
+function pressCtrlF() {
+  EventUtils.synthesizeKey("f", {accelKey: true});
+}
+
+function clickInRuleView(inspector) {
+  let el = inspector.panelDoc.querySelector("#sidebar-panel-ruleview");
+  EventUtils.synthesizeMouseAtCenter(el, {}, inspector.panelDoc.defaultView);
+}
+
+function clickInComputedView(inspector) {
+  let el = inspector.panelDoc.querySelector("#sidebar-panel-computedview");
+  EventUtils.synthesizeMouseAtCenter(el, {}, inspector.panelDoc.defaultView);
+}
+
+function isInInspectorSearchBox(inspector) {
+  // Focus ends up in an anonymous child of the XUL textbox.
+  ok(inspector.panelDoc.activeElement.closest("#inspector-searchbox"),
+     "The inspector search field is focused when ctrl+F is pressed");
+}
+
+function isInRuleViewSearchBox(inspector) {
+  is(inspector.panelDoc.activeElement, inspector.ruleview.view.searchField,
+     "The rule-view search field is focused when ctrl+F is pressed");
+}
+
+function isInComputedViewSearchBox(inspector) {
+  is(inspector.panelDoc.activeElement, inspector.computedview.computedView.searchField,
+     "The computed-view search field is focused when ctrl+F is pressed");
+}
--- a/devtools/client/themes/computed.css
+++ b/devtools/client/themes/computed.css
@@ -29,27 +29,38 @@
   /* Vertically center the 'Browser styles' checkbox in the
      Computed panel with its label. */
   display: flex;
   align-items: center;
 }
 
 #computedview-container {
   overflow: auto;
+  height: 100%;
 }
 
 #propertyContainer {
   -moz-user-select: text;
   overflow-y: auto;
   overflow-x: hidden;
   flex: auto;
   border-top-width: 1px;
   border-top-style: dotted;
 }
 
+/* This extra wrapper only serves as a way to get the content of the view focusable.
+   So that when the user reaches it either via keyboard or mouse, we know that the view
+   is focused and therefore can handle shortcuts.
+   However, for accessibility reasons, tabindex is set to -1 to avoid having to tab
+   through it, and the outline is hidden. */
+#computedview-container-focusable {
+  height: 100%;
+  outline: none;
+}
+
 .row-striped {
   background: var(--theme-body-background);
 }
 
 .property-view-hidden,
 .property-content-hidden {
   display: none;
 }
--- a/devtools/client/themes/rules.css
+++ b/devtools/client/themes/rules.css
@@ -77,16 +77,26 @@
 
 #ruleview-container {
   -moz-user-select: text;
   overflow: auto;
   flex: auto;
   height: 100%;
 }
 
+/* This extra wrapper only serves as a way to get the content of the view focusable.
+   So that when the user reaches it either via keyboard or mouse, we know that the view
+   is focused and therefore can handle shortcuts.
+   However, for accessibility reasons, tabindex is set to -1 to avoid having to tab
+   through it, and the outline is hidden. */
+#ruleview-container-focusable {
+  height: 100%;
+  outline: none;
+}
+
 #ruleview-container.non-interactive {
   pointer-events: none;
   visibility: collapse;
   transition: visibility 0.25s;
 }
 
 .ruleview-code {
   direction: ltr;