diff --git a/devtools/client/webconsole/components/GripMessageBody.js b/devtools/client/webconsole/components/GripMessageBody.js index fc2eb7919d86..ec6b768a3c6d 100644 --- a/devtools/client/webconsole/components/GripMessageBody.js +++ b/devtools/client/webconsole/components/GripMessageBody.js @@ -59,6 +59,10 @@ function GripMessageBody(props) { 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")) { diff --git a/devtools/client/webconsole/components/SideBar.js b/devtools/client/webconsole/components/SideBar.js index 857b7a2dfeaa..bfeeb72ced2d 100644 --- a/devtools/client/webconsole/components/SideBar.js +++ b/devtools/client/webconsole/components/SideBar.js @@ -29,13 +29,26 @@ class SideBar extends Component { 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; @@ -43,6 +56,7 @@ class SideBar extends Component { let objectInspector = getObjectInspector(grip, serviceContainer, { autoExpandDepth: 1, mode: MODE.SHORT, + autoFocusRoot: true, }); let endPanel = dom.aside({ @@ -61,18 +75,14 @@ class SideBar extends Component { }, 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, + }); } } diff --git a/devtools/client/webconsole/test/mochitest/browser.ini b/devtools/client/webconsole/test/mochitest/browser.ini index cb21005a3654..0a5d60588822 100644 --- a/devtools/client/webconsole/test/mochitest/browser.ini +++ b/devtools/client/webconsole/test/mochitest/browser.ini @@ -314,6 +314,7 @@ skip-if = (os == 'linux') || (os == 'win' && os_version == '10.0' && debug && bi [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] diff --git a/devtools/client/webconsole/test/mochitest/browser_webconsole_object_in_sidebar_keyboard_nav.js b/devtools/client/webconsole/test/mochitest/browser_webconsole_object_in_sidebar_keyboard_nav.js new file mode 100644 index 000000000000..52a7982242af --- /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, + `; +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; +} diff --git a/devtools/client/webconsole/utils/object-inspector.js b/devtools/client/webconsole/utils/object-inspector.js index 42453caa482d..029970ebfdd2 100644 --- a/devtools/client/webconsole/utils/object-inspector.js +++ b/devtools/client/webconsole/utils/object-inspector.js @@ -26,7 +26,7 @@ const { Grip } = REPS; * @returns {ObjectInspector} * An ObjectInspector for the given grip. */ -function getObjectInspector(grip, serviceContainer, override) { +function getObjectInspector(grip, serviceContainer, override = {}) { let onDOMNodeMouseOver; let onDOMNodeMouseOut; let onInspectIconClick; @@ -44,18 +44,12 @@ function getObjectInspector(grip, serviceContainer, override) { : 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 => @@ -79,7 +73,22 @@ function getObjectInspector(grip, serviceContainer, override) { }); } + 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, +};