Bug 1428078 - Enable keyboard navigation in the object sidebar; r=bgrins. draft
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Fri, 13 Apr 2018 17:47:37 +0200
changeset 786598 d3213b0797d8ca2ed6c5f4b5f42b606b323edf92
parent 786478 dfb15917c057f17e5143f7d7c6e1972ba53efc49
push id107538
push userbmo:nchevobbe@mozilla.com
push dateMon, 23 Apr 2018 16:59:09 +0000
reviewersbgrins
bugs1428078
milestone61.0a1
Bug 1428078 - Enable keyboard navigation in the object sidebar; r=bgrins. MozReview-Commit-ID: JuZOX3e9AXy
devtools/client/webconsole/components/GripMessageBody.js
devtools/client/webconsole/components/SideBar.js
devtools/client/webconsole/test/mochitest/browser.ini
devtools/client/webconsole/test/mochitest/browser_webconsole_object_in_sidebar_keyboard_nav.js
devtools/client/webconsole/utils/object-inspector.js
--- a/devtools/client/webconsole/components/GripMessageBody.js
+++ b/devtools/client/webconsole/components/GripMessageBody.js
@@ -54,16 +54,20 @@ function GripMessageBody(props) {
   let styleObject;
   if (userProvidedStyle && userProvidedStyle !== "") {
     styleObject = cleanupStyle(userProvidedStyle, serviceContainer.createElement);
   }
 
   let objectInspectorProps = {
     autoExpandDepth: shouldAutoExpandObjectInspector(props) ? 1 : 0,
     mode,
+    // TODO: we disable focus since the tabbing trail is a bit weird in the output (e.g.
+    // location links are not focused). Let's remove the property below when we found and
+    // fixed the issue (See Bug 1456060).
+    focusable: false,
   };
 
   if (typeof grip === "string" || (grip && grip.type === "longString")) {
     Object.assign(objectInspectorProps, {
       useQuotes,
       escapeWhitespace,
       style: styleObject
     });
--- a/devtools/client/webconsole/components/SideBar.js
+++ b/devtools/client/webconsole/components/SideBar.js
@@ -24,30 +24,44 @@ class SideBar extends Component {
     };
   }
 
   constructor(props) {
     super(props);
     this.onClickSidebarClose = this.onClickSidebarClose.bind(this);
   }
 
+  shouldComponentUpdate(nextProps) {
+    const {
+      grip,
+      sidebarVisible,
+    } = nextProps;
+
+    return sidebarVisible !== this.props.sidebarVisible
+      || grip !== this.props.grip;
+  }
+
   onClickSidebarClose() {
     this.props.dispatch(actions.sidebarClose());
   }
 
   render() {
+    if (!this.props.sidebarVisible) {
+      return null;
+    }
+
     let {
-      sidebarVisible,
       grip,
       serviceContainer,
     } = this.props;
 
     let objectInspector = getObjectInspector(grip, serviceContainer, {
       autoExpandDepth: 1,
       mode: MODE.SHORT,
+      autoFocusRoot: true,
     });
 
     let endPanel = dom.aside({
       className: "sidebar-wrapper"
     },
       dom.header({
         className: "devtools-toolbar webconsole-sidebar-toolbar"
       },
@@ -56,28 +70,24 @@ class SideBar extends Component {
           onClick: this.onClickSidebarClose
         })
       ),
       dom.aside({
         className: "sidebar-contents"
       }, objectInspector)
     );
 
-    return (
-      sidebarVisible ?
-        SplitBox({
-          className: "sidebar",
-          endPanel,
-          endPanelControl: true,
-          initialSize: "200px",
-          minSize: "100px",
-          vert: true,
-        })
-        : null
-    );
+    return SplitBox({
+      className: "sidebar",
+      endPanel,
+      endPanelControl: true,
+      initialSize: "200px",
+      minSize: "100px",
+      vert: true,
+    });
   }
 }
 
 function mapStateToProps(state, props) {
   return {
     sidebarVisible: state.ui.sidebarVisible,
     grip: state.ui.gripInSidebar,
   };
--- a/devtools/client/webconsole/test/mochitest/browser.ini
+++ b/devtools/client/webconsole/test/mochitest/browser.ini
@@ -309,16 +309,17 @@ skip-if = true #	Bug 1403448
 skip-if = (os == 'linux') || (os == 'win' && os_version == '10.0' && debug && bits == 64)  # Bug 1429361, disabled on Linux/Win for frequent failures
 [browser_webconsole_network_messages_openinnet.js]
 [browser_webconsole_network_messages_status_code.js]
 [browser_webconsole_network_requests_from_chrome.js]
 [browser_webconsole_network_reset_filter.js]
 [browser_webconsole_nodes_highlight.js]
 [browser_webconsole_nodes_select.js]
 [browser_webconsole_object_in_sidebar.js]
+[browser_webconsole_object_in_sidebar_keyboard_nav.js]
 [browser_webconsole_object_inspector.js]
 [browser_webconsole_object_inspector_entries.js]
 [browser_webconsole_object_inspector_key_sorting.js]
 [browser_webconsole_object_inspector_while_debugging_and_inspecting.js]
 [browser_webconsole_observer_notifications.js]
 [browser_webconsole_optimized_out_vars.js]
 [browser_webconsole_output_copy.js]
 subsuite = clipboard
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/mochitest/browser_webconsole_object_in_sidebar_keyboard_nav.js
@@ -0,0 +1,98 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that the ObjectInspector in the sidebar can be navigated with the keyboard.
+
+"use strict";
+
+const TEST_URI = `data:text/html;charset=utf8,
+  <script>
+    console.log({
+      a:1,
+      b:2,
+      c: Array.from({length: 100}, (_, i) => i)
+    });
+  </script>`;
+const {ELLIPSIS} = require("devtools/shared/l10n");
+
+add_task(async function() {
+  // Should be removed when sidebar work is complete
+  await pushPref("devtools.webconsole.sidebarToggle", true);
+
+  let hud = await openNewTabAndConsole(TEST_URI);
+
+  let message = await waitFor(() => findMessage(hud, "Object"));
+  let object = message.querySelector(".object-inspector .objectBox-object");
+
+  const onSideBarVisible = waitFor(() =>
+    hud.ui.document.querySelector(".sidebar-contents"));
+
+  await openObjectInSidebar(hud, object);
+  const sidebarContents = await onSideBarVisible;
+
+  let objectInspector = sidebarContents.querySelector(".object-inspector");
+  ok(objectInspector, "The ObjectInspector is displayed");
+
+  // There are 5 nodes: the root, a, b, c, and proto.
+  await waitFor(() => objectInspector.querySelectorAll(".node").length === 5);
+
+  const [
+    root,
+    a,
+    b,
+    c,
+  ] = objectInspector.querySelectorAll(".node");
+
+  ok(root.classList.contains("focused"), "The root node is focused");
+
+  await synthesizeKeyAndWaitForFocus("KEY_ArrowDown", a);
+  ok(true, "`a` node is focused");
+
+  await synthesizeKeyAndWaitForFocus("KEY_ArrowDown", b);
+  ok(true, "`b` node is focused");
+
+  await synthesizeKeyAndWaitForFocus("KEY_ArrowDown", c);
+  ok(true, "`c` node is focused");
+
+  EventUtils.synthesizeKey("KEY_ArrowRight");
+  await waitFor(() => objectInspector.querySelectorAll(".node").length > 5);
+  ok(true, "`c` node is expanded");
+
+  const arrayNodes = objectInspector.querySelectorAll(`[aria-level="3"]`);
+  await synthesizeKeyAndWaitForFocus("KEY_ArrowDown", arrayNodes[0]);
+  ok(true, "First item of the `c` array is focused");
+
+  await synthesizeKeyAndWaitForFocus("KEY_ArrowLeft", c);
+  ok(true, "`c` node is focused again");
+
+  await synthesizeKeyAndWaitForFocus("KEY_ArrowUp", b);
+  ok(true, "`b` node is focused again");
+
+  info("Select another object in the console output");
+  const onArrayMessage = waitForMessage(hud, "Array");
+  await ContentTask.spawn(gBrowser.selectedBrowser, null, () => {
+    content.wrappedJSObject.console.log([4, 5, 6]);
+  });
+
+  const arrayMessage = await onArrayMessage;
+  let array = arrayMessage.node.querySelector(".object-inspector .objectBox-array");
+  await openObjectInSidebar(hud, array);
+
+  await waitFor(() => sidebarContents.querySelector(".tree-node")
+    .textContent.includes(`(3) [${ELLIPSIS}]`));
+  ok(sidebarContents.querySelector(".tree-node").classList.contains("focused"),
+    "The root node of the new object in the sidebar is focused");
+});
+
+async function openObjectInSidebar(hud, objectNode) {
+  let contextMenu = await openContextMenu(hud, objectNode);
+  let openInSidebarEntry = contextMenu.querySelector("#console-menu-open-sidebar");
+  openInSidebarEntry.click();
+  await hideContextMenu(hud);
+}
+
+function synthesizeKeyAndWaitForFocus(keyStr, elementToBeFocused) {
+  let onFocusChanged = waitFor(() => elementToBeFocused.classList.contains("focused"));
+  EventUtils.synthesizeKey(keyStr);
+  return onFocusChanged;
+}
--- a/devtools/client/webconsole/utils/object-inspector.js
+++ b/devtools/client/webconsole/utils/object-inspector.js
@@ -21,17 +21,17 @@ const { Grip } = REPS;
  * @param {Object} serviceContainer
  *        Object containing various utility functions
  * @param {Object} override
  *        Object containing props that should override the default props passed to
  *        ObjectInspector.
  * @returns {ObjectInspector}
  *        An ObjectInspector for the given grip.
  */
-function getObjectInspector(grip, serviceContainer, override) {
+function getObjectInspector(grip, serviceContainer, override = {}) {
   let onDOMNodeMouseOver;
   let onDOMNodeMouseOut;
   let onInspectIconClick;
   if (serviceContainer) {
     onDOMNodeMouseOver = serviceContainer.highlightDomElement
       ? (object) => serviceContainer.highlightDomElement(object)
       : null;
     onDOMNodeMouseOut = serviceContainer.unHighlightDomElement;
@@ -39,28 +39,22 @@ function getObjectInspector(grip, servic
       ? (object, e) => {
         // Stop the event propagation so we don't trigger ObjectInspector expand/collapse.
         e.stopPropagation();
         serviceContainer.openNodeInInspector(object);
       }
       : null;
   }
 
+  const roots = createRootsFromGrip(grip);
+
   const objectInspectorProps = {
     autoExpandDepth: 0,
     mode: MODE.LONG,
-    // TODO: we disable focus since it's not currently working well in ObjectInspector.
-    // Let's remove the property below when problem are fixed in OI.
-    focusable: false,
-    roots: [{
-      path: (grip && grip.actor) || JSON.stringify(grip),
-      contents: {
-        value: grip
-      }
-    }],
+    roots,
     createObjectClient: object =>
       new ObjectClient(serviceContainer.hudProxy.client, object),
     createLongStringClient: object =>
       new LongStringClient(serviceContainer.hudProxy.client, object),
     releaseActor: actor => {
       if (!actor || !serviceContainer.hudProxy.releaseActor) {
         return;
       }
@@ -74,12 +68,27 @@ function getObjectInspector(grip, servic
     Object.assign(objectInspectorProps, {
       onDOMNodeMouseOver,
       onDOMNodeMouseOut,
       onInspectIconClick,
       defaultRep: Grip,
     });
   }
 
+  if (override.autoFocusRoot) {
+    Object.assign(objectInspectorProps, {
+      focusedItem: roots[0]
+    });
+  }
+
   return ObjectInspector({...objectInspectorProps, ...override});
 }
 
-exports.getObjectInspector = getObjectInspector;
+function createRootsFromGrip(grip) {
+  return [{
+    path: (grip && grip.actor) || JSON.stringify(grip),
+    contents: { value: grip }
+  }];
+}
+
+module.exports = {
+  getObjectInspector,
+};