Bug 1419081 - Cmd/Ctrl + click should put object in the sidebar; r=Honza. draft
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 19 Apr 2018 10:54:36 +0200
changeset 814477 ad40ffb48695e3156a5730a5a41c1e8d9cb73b88
parent 813581 a0e47ebc4c06e652b919dabee711fdbd6bfd31b5
push id115224
push userbmo:nchevobbe@mozilla.com
push dateThu, 05 Jul 2018 12:47:36 +0000
reviewersHonza
bugs1419081
milestone63.0a1
Bug 1419081 - Cmd/Ctrl + click should put object in the sidebar; r=Honza. In order to do that, we pass an onCmdCtrlClick to the ObjectInspector. We also extract the dispatching of the SHOW_OBJECT_IN_SIDEBAR action to its own function so it can be called directly with a grip. browser_webconsole_object_in_sidebar is renamed and repurposed to test this specific behavior, since there's already a test checking the context menu way (browser_webconsole_context_menu_object_in_sidebar.js). MozReview-Commit-ID: GCF1wFPWTH
devtools/client/webconsole/actions/ui.js
devtools/client/webconsole/components/GripMessageBody.js
devtools/client/webconsole/test/mochitest/browser.ini
devtools/client/webconsole/test/mochitest/browser_webconsole_object_ctrl_click.js
devtools/client/webconsole/test/mochitest/browser_webconsole_object_in_sidebar.js
devtools/client/webconsole/test/store/ui.test.js
devtools/client/webconsole/webconsole-output-wrapper.js
--- a/devtools/client/webconsole/actions/ui.js
+++ b/devtools/client/webconsole/actions/ui.js
@@ -69,35 +69,47 @@ function sidebarClose(show) {
 
 function splitConsoleCloseButtonToggle(shouldDisplayButton) {
   return {
     type: SPLIT_CONSOLE_CLOSE_BUTTON_TOGGLE,
     shouldDisplayButton,
   };
 }
 
-function showObjectInSidebar(actorId, messageId) {
+/**
+ * Dispatches a SHOW_OBJECT_IN_SIDEBAR action, with a grip property corresponding to the
+ * {actor} parameter in the {messageId} message.
+ *
+ * @param {String} actor: Actor id of the object we want to place in the sidebar.
+ * @param {String} messageId: id of the message containing the {actor} parameter.
+ */
+function showMessageObjectInSidebar(actor, messageId) {
   return (dispatch, getState) => {
     const { parameters } = getMessage(getState(), messageId);
     if (Array.isArray(parameters)) {
       for (const parameter of parameters) {
-        if (parameter.actor === actorId) {
-          dispatch({
-            type: SHOW_OBJECT_IN_SIDEBAR,
-            grip: parameter
-          });
+        if (parameter.actor === actor) {
+          dispatch(showObjectInSidebar(parameter));
           return;
         }
       }
     }
   };
 }
 
+function showObjectInSidebar(grip) {
+  return {
+    type: SHOW_OBJECT_IN_SIDEBAR,
+    grip,
+  };
+}
+
 module.exports = {
   filterBarToggle,
   initialize,
   persistToggle,
   selectNetworkMessageTab,
   sidebarClose,
+  showMessageObjectInSidebar,
   showObjectInSidebar,
   timestampsToggle,
   splitConsoleCloseButtonToggle,
 };
--- a/devtools/client/webconsole/components/GripMessageBody.js
+++ b/devtools/client/webconsole/components/GripMessageBody.js
@@ -8,19 +8,20 @@
 
 // React
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const {
   MESSAGE_TYPE,
   JSTERM_COMMANDS,
 } = require("../constants");
 const { getObjectInspector } = require("devtools/client/webconsole/utils/object-inspector");
+const actions = require("devtools/client/webconsole/actions/index");
 
 const reps = require("devtools/client/shared/components/reps/reps");
-const { MODE } = reps;
+const { MODE, ObjectInspectorUtils } = reps;
 
 GripMessageBody.displayName = "GripMessageBody";
 
 GripMessageBody.propTypes = {
   grip: PropTypes.oneOfType([
     PropTypes.string,
     PropTypes.number,
     PropTypes.object,
@@ -44,30 +45,37 @@ GripMessageBody.defaultProps = {
 function GripMessageBody(props) {
   const {
     grip,
     userProvidedStyle,
     serviceContainer,
     useQuotes,
     escapeWhitespace,
     mode = MODE.LONG,
+    dispatch,
   } = props;
 
   let styleObject;
   if (userProvidedStyle && userProvidedStyle !== "") {
     styleObject = cleanupStyle(userProvidedStyle, serviceContainer.createElement);
   }
 
   const 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,
+    onCmdCtrlClick: (node, { depth, event, focused, expanded }) => {
+      const value = ObjectInspectorUtils.node.getValue(node);
+      if (value) {
+        dispatch(actions.showObjectInSidebar(value));
+      }
+    }
   };
 
   if (typeof grip === "string" || (grip && grip.type === "longString")) {
     Object.assign(objectInspectorProps, {
       useQuotes,
       escapeWhitespace,
       style: styleObject
     });
--- a/devtools/client/webconsole/test/mochitest/browser.ini
+++ b/devtools/client/webconsole/test/mochitest/browser.ini
@@ -309,17 +309,17 @@ skip-if = true #	Bug 1404382
 [browser_webconsole_network_messages_expand.js]
 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_ctrl_click.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_local_session_storage.js]
 [browser_webconsole_object_inspector_selected_text.js]
 [browser_webconsole_object_inspector_scroll.js]
 [browser_webconsole_object_inspector_while_debugging_and_inspecting.js]
rename from devtools/client/webconsole/test/mochitest/browser_webconsole_object_in_sidebar.js
rename to devtools/client/webconsole/test/mochitest/browser_webconsole_object_ctrl_click.js
--- a/devtools/client/webconsole/test/mochitest/browser_webconsole_object_in_sidebar.js
+++ b/devtools/client/webconsole/test/mochitest/browser_webconsole_object_ctrl_click.js
@@ -2,56 +2,81 @@
 /* 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/ */
 
 // Test that the ObjectInspector is rendered correctly in the sidebar.
 
 "use strict";
 
-const TEST_URI =
-  "data:text/html;charset=utf8," +
-  "<script>console.log({a:1,b:2,c:3});</script>";
+const TEST_URI = `data:text/html;charset=utf8,<script>
+    console.log({
+      a:1,
+      b:2,
+      c:[3]
+    });
+  </script>`;
 
 add_task(async function() {
   // Should be removed when sidebar work is complete
   await pushPref("devtools.webconsole.sidebarToggle", true);
+  const isMacOS = Services.appinfo.OS === "Darwin";
 
   const hud = await openNewTabAndConsole(TEST_URI);
 
   const message = findMessage(hud, "Object");
   const object = message.querySelector(".object-inspector .objectBox-object");
 
-  await showSidebarWithContextMenu(hud, object, true);
+  info("Ctrl+click on an object to put it in the sidebar");
+  const onSidebarShown = waitFor(() => hud.ui.document.querySelector(".sidebar"));
+  EventUtils.sendMouseEvent({
+    type: "click",
+    [isMacOS ? "metaKey" : "ctrlKey"]: true
+  }, object, hud.ui.window);
+  await onSidebarShown;
+  ok(true, "sidebar is displayed after user Ctrl+clicked on it");
 
   const sidebarContents = hud.ui.document.querySelector(".sidebar-contents");
-  const objectInspectors = [...sidebarContents.querySelectorAll(".tree")];
+  let objectInspectors = [...sidebarContents.querySelectorAll(".tree")];
   is(objectInspectors.length, 1, "There is the expected number of object inspectors");
-  const [objectInspector] = objectInspectors;
-  let oiNodes = objectInspector.querySelectorAll(".node");
-  if (oiNodes.length === 1) {
-    // If this is the case, we wait for the properties to be fetched and displayed.
-    await waitForNodeMutation(objectInspector, {
-      childList: true
-    });
-    oiNodes = objectInspector.querySelectorAll(".node");
-  }
+  let [objectInspector] = objectInspectors;
 
-  // There are 5 nodes: the root, a, b, c, and proto.
-  is(oiNodes.length, 5, "There is the expected number of nodes in the tree");
-  const propertiesNodes = [...objectInspector.querySelectorAll(".object-label")]
+  // The object in the sidebar now should look like:
+  // ▼ { … }
+  // |   a: 1
+  // |   b: 2
+  // | ▶︎ c: Array [3]
+  // | ▶︎ <prototype>: Object { … }
+  await waitFor(() => objectInspector.querySelectorAll(".node").length === 5);
+
+  let propertiesNodes = [...objectInspector.querySelectorAll(".object-label")]
     .map(el => el.textContent);
-  const arrayPropertiesNames = ["a", "b", "c", "<prototype>"];
-  is(JSON.stringify(propertiesNodes), JSON.stringify(arrayPropertiesNames));
-});
+  let arrayPropertiesNames = ["a", "b", "c", "<prototype>"];
+  is(JSON.stringify(propertiesNodes), JSON.stringify(arrayPropertiesNames),
+    "The expected nodes are displayed");
 
-async function showSidebarWithContextMenu(hud, node, expectMutation) {
-  const wrapper = hud.ui.document.querySelector(".webconsole-output-wrapper");
-  const onSidebarShown = waitForNodeMutation(wrapper, { childList: true });
+  info("Expand the object in the console output");
+  object.click();
+  await waitFor(() => message.querySelectorAll(".node").length === 5);
+  const cNode = message.querySelectorAll(".node")[3];
+  info("Ctrl+click on the `c` property node to put it in the sidebar");
+  EventUtils.sendMouseEvent({
+    type: "click",
+    [isMacOS ? "metaKey" : "ctrlKey"]: true
+  }, cNode, hud.ui.window);
 
-  const contextMenu = await openContextMenu(hud, node);
-  const openInSidebar = contextMenu.querySelector("#console-menu-open-sidebar");
-  openInSidebar.click();
-  if (expectMutation) {
-    await onSidebarShown;
-  }
-  await hideContextMenu(hud);
-}
+  objectInspectors = [...sidebarContents.querySelectorAll(".tree")];
+  is(objectInspectors.length, 1, "There is still only one object inspector");
+  [objectInspector] = objectInspectors;
+
+  // The object in the sidebar now should look like:
+  // ▼ (1) […]
+  // |   0: 3
+  // |   length: 1
+  // | ▶︎ <prototype>: Array []
+  await waitFor(() => objectInspector.querySelectorAll(".node").length === 4);
+
+  propertiesNodes = [...objectInspector.querySelectorAll(".object-label")]
+    .map(el => el.textContent);
+  arrayPropertiesNames = ["0", "length", "<prototype>"];
+  is(JSON.stringify(propertiesNodes), JSON.stringify(arrayPropertiesNames),
+    "The expected nodes are displayed");
+});
--- a/devtools/client/webconsole/test/store/ui.test.js
+++ b/devtools/client/webconsole/test/store/ui.test.js
@@ -19,33 +19,33 @@ describe("Testing UI", () => {
   describe("Toggle sidebar", () => {
     it("sidebar is toggled on and off", () => {
       const packet = stubPackets.get("inspect({a: 1})");
       const message = stubPreparedMessages.get("inspect({a: 1})");
       store.dispatch(actions.messagesAdd([packet]));
 
       const actorId = message.parameters[0].actor;
       const messageId = getFirstMessage(store.getState()).id;
-      store.dispatch(actions.showObjectInSidebar(actorId, messageId));
+      store.dispatch(actions.showMessageObjectInSidebar(actorId, messageId));
 
       expect(store.getState().ui.sidebarVisible).toEqual(true);
       store.dispatch(actions.sidebarClose());
       expect(store.getState().ui.sidebarVisible).toEqual(false);
     });
   });
 
   describe("Hide sidebar on clear", () => {
     it("sidebar is hidden on clear", () => {
       const packet = stubPackets.get("inspect({a: 1})");
       const message = stubPreparedMessages.get("inspect({a: 1})");
       store.dispatch(actions.messagesAdd([packet]));
 
       const actorId = message.parameters[0].actor;
       const messageId = getFirstMessage(store.getState()).id;
-      store.dispatch(actions.showObjectInSidebar(actorId, messageId));
+      store.dispatch(actions.showMessageObjectInSidebar(actorId, messageId));
 
       expect(store.getState().ui.sidebarVisible).toEqual(true);
       store.dispatch(actions.messagesClear());
       expect(store.getState().ui.sidebarVisible).toEqual(false);
       store.dispatch(actions.messagesClear());
       expect(store.getState().ui.sidebarVisible).toEqual(false);
     });
   });
@@ -53,56 +53,56 @@ describe("Testing UI", () => {
   describe("Show object in sidebar", () => {
     it("sidebar is shown with correct object", () => {
       const packet = stubPackets.get("inspect({a: 1})");
       const message = stubPreparedMessages.get("inspect({a: 1})");
       store.dispatch(actions.messagesAdd([packet]));
 
       const actorId = message.parameters[0].actor;
       const messageId = getFirstMessage(store.getState()).id;
-      store.dispatch(actions.showObjectInSidebar(actorId, messageId));
+      store.dispatch(actions.showMessageObjectInSidebar(actorId, messageId));
 
       expect(store.getState().ui.sidebarVisible).toEqual(true);
       expect(store.getState().ui.gripInSidebar).toEqual(message.parameters[0]);
     });
 
     it("sidebar is not updated for the same object", () => {
       const packet = stubPackets.get("inspect({a: 1})");
       const message = stubPreparedMessages.get("inspect({a: 1})");
       store.dispatch(actions.messagesAdd([packet]));
 
       const actorId = message.parameters[0].actor;
       const messageId = getFirstMessage(store.getState()).id;
-      store.dispatch(actions.showObjectInSidebar(actorId, messageId));
+      store.dispatch(actions.showMessageObjectInSidebar(actorId, messageId));
 
       expect(store.getState().ui.sidebarVisible).toEqual(true);
       expect(store.getState().ui.gripInSidebar).toEqual(message.parameters[0]);
       const state = store.getState().ui;
 
-      store.dispatch(actions.showObjectInSidebar(actorId, messageId));
+      store.dispatch(actions.showMessageObjectInSidebar(actorId, messageId));
       expect(store.getState().ui).toEqual(state);
     });
 
     it("sidebar shown and updated for new object", () => {
       const packet = stubPackets.get("inspect({a: 1})");
       const message = stubPreparedMessages.get("inspect({a: 1})");
       store.dispatch(actions.messagesAdd([packet]));
 
       const actorId = message.parameters[0].actor;
       const messageId = getFirstMessage(store.getState()).id;
-      store.dispatch(actions.showObjectInSidebar(actorId, messageId));
+      store.dispatch(actions.showMessageObjectInSidebar(actorId, messageId));
 
       expect(store.getState().ui.sidebarVisible).toEqual(true);
       expect(store.getState().ui.gripInSidebar).toEqual(message.parameters[0]);
 
       const newPacket = stubPackets.get("new Date(0)");
       const newMessage = stubPreparedMessages.get("new Date(0)");
       store.dispatch(actions.messagesAdd([newPacket]));
 
       const newActorId = newMessage.parameters[0].actor;
       const newMessageId = getLastMessage(store.getState()).id;
-      store.dispatch(actions.showObjectInSidebar(newActorId, newMessageId));
+      store.dispatch(actions.showMessageObjectInSidebar(newActorId, newMessageId));
 
       expect(store.getState().ui.sidebarVisible).toEqual(true);
       expect(store.getState().ui.gripInSidebar).toEqual(newMessage.parameters[0]);
     });
   });
 });
--- a/devtools/client/webconsole/webconsole-output-wrapper.js
+++ b/devtools/client/webconsole/webconsole-output-wrapper.js
@@ -135,17 +135,17 @@ WebConsoleOutputWrapper.prototype = {
 
         const rootObjectInspector = target.closest(".object-inspector");
         const rootActor = rootObjectInspector ?
                         rootObjectInspector.querySelector("[data-link-actor-id]") : null;
         const rootActorId = rootActor ? rootActor.dataset.linkActorId : null;
 
         const sidebarTogglePref = store.getState().prefs.sidebarToggle;
         const openSidebar = sidebarTogglePref ? (messageId) => {
-          store.dispatch(actions.showObjectInSidebar(rootActorId, messageId));
+          store.dispatch(actions.showMessageObjectInSidebar(rootActorId, messageId));
         } : null;
 
         const menu = createContextMenu(this.hud, this.parentNode, {
           actor,
           clipboardText,
           variableText,
           message,
           serviceContainer,