Bug 1053898 - Update framework/selection setNodeFront signature to use object argument;r=gl draft
authorJulian Descottes <jdescottes@mozilla.com>
Mon, 05 Mar 2018 14:15:57 +0100
changeset 773672 e5bde9e74369e1b0fb2e1d1a0a2b31a0131caacd
parent 773671 95259e33e084038d7a71e89b90affb60c38894a3
child 773673 4ee3b857ee43ebe981b2148b0a6a1390c69669aa
push id104269
push userjdescottes@mozilla.com
push dateWed, 28 Mar 2018 08:16:27 +0000
reviewersgl
bugs1053898
milestone61.0a1
Bug 1053898 - Update framework/selection setNodeFront signature to use object argument;r=gl Both reason and isSlotted arguments of setNodeFront are optional, switching to an object argument avoids weird calls such as setNodeFront(front, null, true) in favor of a more explicit setNodeFront(front, { isSlotted: true }); MozReview-Commit-ID: A6nziH3QQYe
devtools/client/framework/devtools.js
devtools/client/framework/selection.js
devtools/client/framework/test/browser_toolbox_selectionchanged_event.js
devtools/client/framework/toolbox-highlighter-utils.js
devtools/client/inspector/animation/components/AnimationTarget.js
devtools/client/inspector/boxmodel/components/ComputedProperty.js
devtools/client/inspector/boxmodel/test/head.js
devtools/client/inspector/breadcrumbs.js
devtools/client/inspector/extensions/extension-sidebar.js
devtools/client/inspector/flexbox/components/FlexboxItem.js
devtools/client/inspector/grids/components/GridItem.js
devtools/client/inspector/inspector-commands.js
devtools/client/inspector/inspector-search.js
devtools/client/inspector/inspector.js
devtools/client/inspector/markup/markup.js
devtools/client/inspector/shared/dom-node-preview.js
devtools/client/inspector/test/shared-head.js
devtools/client/shared/widgets/VariablesView.jsm
devtools/client/webconsole/console-output.js
devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
--- a/devtools/client/framework/devtools.js
+++ b/devtools/client/framework/devtools.js
@@ -640,17 +640,17 @@ DevTools.prototype = {
         // This is the NodeFront for the document node inside the iframe
         nodeFront = nodes[0];
       }
       return querySelectors(nodeFront);
     }
     let nodeFront = await inspector.walker.getRootNode();
     nodeFront = await querySelectors(nodeFront);
     // Select the final node
-    inspector.selection.setNodeFront(nodeFront, "browser-context-menu");
+    inspector.selection.setNodeFront(nodeFront, { reason: "browser-context-menu" });
 
     await onNewNode;
     // Now that the node has been selected, wait until the inspector is
     // fully updated.
     await inspector.once("inspector-updated");
   },
 
   /**
--- a/devtools/client/framework/selection.js
+++ b/devtools/client/framework/selection.js
@@ -120,34 +120,34 @@ Selection.prototype = {
     }
   },
 
   /**
    * Update the currently selected node-front.
    *
    * @param {NodeFront} nodeFront
    *        The NodeFront being selected.
-   * @param {String} reason
-   *        Reason that triggered the selection, will be fired with the "new-node-front"
-   *        event.
-   * @param {Boolean} isSlotted
-   *        Is the selection representing the slotted version of the node.
+   * @param {Object} (optional)
+   *        - {String} reason: Reason that triggered the selection, will be fired with
+   *          the "new-node-front" event.
+   *        - {Boolean} isSlotted: Is the selection representing the slotted version of
+   *          the node.
    */
-  setNodeFront: function(value, reason = "unknown", isSlotted = false) {
+  setNodeFront: function(nodeFront, { reason = "unknown", isSlotted = false} = {}) {
     this.reason = reason;
-    this._isSlotted = isSlotted;
 
     // If an inlineTextChild text node is being set, then set it's parent instead.
-    let parentNode = value && value.parentNode();
-    if (value && parentNode && parentNode.inlineTextChild === value) {
-      value = parentNode;
+    let parentNode = nodeFront && nodeFront.parentNode();
+    if (nodeFront && parentNode && parentNode.inlineTextChild === nodeFront) {
+      nodeFront = parentNode;
     }
 
-    this._nodeFront = value;
-    this.emit("new-node-front", value, this.reason);
+    this._isSlotted = isSlotted;
+    this._nodeFront = nodeFront;
+    this.emit("new-node-front", nodeFront, this.reason);
   },
 
   get documentFront() {
     return this._walker.document(this._nodeFront);
   },
 
   get nodeFront() {
     return this._nodeFront;
--- a/devtools/client/framework/test/browser_toolbox_selectionchanged_event.js
+++ b/devtools/client/framework/test/browser_toolbox_selectionchanged_event.js
@@ -17,25 +17,25 @@ add_task(async function () {
   let node = await inspector.walker.querySelector(root, "div");
 
   is(inspector.selection.nodeFront, body, "Body is selected by default");
 
   // Listen to selection changed
   let onSelectionChanged = toolbox.once("selection-changed");
 
   info("Select the div and wait for the selection-changed event to be fired.");
-  inspector.selection.setNodeFront(node, "browser-context-menu");
+  inspector.selection.setNodeFront(node, { reason: "browser-context-menu" });
 
   await onSelectionChanged;
 
   is(inspector.selection.nodeFront, node, "Div is now selected");
 
   // Listen to cleared selection changed
   let onClearSelectionChanged = toolbox.once("selection-changed");
 
   info("Clear the selection and wait for the selection-changed event to be fired.");
-  inspector.selection.setNodeFront(undefined, "browser-context-menu");
+  inspector.selection.setNodeFront(undefined, { reason: "browser-context-menu" });
 
   await onClearSelectionChanged;
 
   is(inspector.selection.nodeFront, undefined, "The selection is undefined as expected");
 });
 
--- a/devtools/client/framework/toolbox-highlighter-utils.js
+++ b/devtools/client/framework/toolbox-highlighter-utils.js
@@ -186,27 +186,27 @@ exports.getHighlighterUtils = function(t
     toolbox.emit("picker-node-hovered", data.node);
   }
 
   /**
    * When a node has been picked while the highlighter is in picker mode
    * @param {Object} data Information about the picked node
    */
   function onPickerNodePicked(data) {
-    toolbox.selection.setNodeFront(data.node, "picker-node-picked");
+    toolbox.selection.setNodeFront(data.node, { reason: "picker-node-picked" });
     stopPicker();
   }
 
   /**
    * When a node has been shift-clicked (previewed) while the highlighter is in
    * picker mode
    * @param {Object} data Information about the picked node
    */
   function onPickerNodePreviewed(data) {
-    toolbox.selection.setNodeFront(data.node, "picker-node-previewed");
+    toolbox.selection.setNodeFront(data.node, { reason: "picker-node-previewed" });
   }
 
   /**
    * When the picker is canceled, stop the picker, and make sure the toolbox
    * gets the focus.
    */
   function onPickerNodeCanceled() {
     cancelPicker();
--- a/devtools/client/inspector/animation/components/AnimationTarget.js
+++ b/devtools/client/inspector/animation/components/AnimationTarget.js
@@ -93,16 +93,17 @@ class AnimationTarget extends PureCompon
       },
       Rep(
         {
           defaultRep: ElementNode,
           mode: MODE.TINY,
           object: translateNodeFrontToGrip(nodeFront),
           onDOMNodeMouseOut: () => onHideBoxModelHighlighter(),
           onDOMNodeMouseOver: () => onShowBoxModelHighlighterForNode(nodeFront),
-          onInspectIconClick: () => setSelectedNode(nodeFront, "animation-panel"),
+          onInspectIconClick: () => setSelectedNode(nodeFront,
+            { reason: "animation-panel" }),
         }
       )
     );
   }
 }
 
 module.exports = AnimationTarget;
--- a/devtools/client/inspector/boxmodel/components/ComputedProperty.js
+++ b/devtools/client/inspector/boxmodel/components/ComputedProperty.js
@@ -48,17 +48,18 @@ class ComputedProperty extends PureCompo
       {
         className: "reference-element"
       },
       dom.span({ className: "reference-element-type" }, referenceElementType),
       Rep({
         defaultRep: referenceElement,
         mode: MODE.TINY,
         object: translateNodeFrontToGrip(referenceElement),
-        onInspectIconClick: () => setSelectedNode(referenceElement, "box-model"),
+        onInspectIconClick: () => setSelectedNode(referenceElement,
+          { reason: "box-model" }),
         onDOMNodeMouseOver: () => onShowBoxModelHighlighterForNode(referenceElement),
         onDOMNodeMouseOut: () => onHideBoxModelHighlighter(),
       })
     );
   }
 
   onFocus() {
     this.container.focus();
--- a/devtools/client/inspector/boxmodel/test/head.js
+++ b/devtools/client/inspector/boxmodel/test/head.js
@@ -27,17 +27,17 @@ registerCleanupFunction(() => {
  * @return {Promise} a promise that resolves when the inspector is updated with the new
  *         node.
  */
 async function selectAndHighlightNode(selectorOrNodeFront, inspector) {
   info("Highlighting and selecting the node " + selectorOrNodeFront);
 
   let nodeFront = await getNodeFront(selectorOrNodeFront, inspector);
   let updated = inspector.toolbox.once("highlighter-ready");
-  inspector.selection.setNodeFront(nodeFront, "test-highlight");
+  inspector.selection.setNodeFront(nodeFront, { reason: "test-highlight" });
   await updated;
 }
 
 /**
  * Is the given node visible in the page (rendered in the frame tree).
  * @param {DOMNode}
  * @return {Boolean}
  */
--- a/devtools/client/inspector/breadcrumbs.js
+++ b/devtools/client/inspector/breadcrumbs.js
@@ -597,17 +597,17 @@ HTMLBreadcrumbs.prototype = {
         currentnode = this.nodeHierarchy[this.currentIndex - 1];
       } else if (isRight && this.currentIndex < this.nodeHierarchy.length - 1) {
         currentnode = this.nodeHierarchy[this.currentIndex + 1];
       } else {
         return null;
       }
 
       this.outer.setAttribute("aria-activedescendant", currentnode.button.id);
-      return this.selection.setNodeFront(currentnode.node, "breadcrumbs");
+      return this.selection.setNodeFront(currentnode.node, { reason: "breadcrumbs" });
     });
   },
 
   /**
    * Remove nodes and clean up.
    */
   destroy: function() {
     this.selection.off("new-node-front", this.update);
@@ -701,17 +701,17 @@ HTMLBreadcrumbs.prototype = {
     button.setAttribute("tabindex", "-1");
     button.setAttribute("title", this.prettyPrintNodeAsText(node));
 
     button.onclick = () => {
       button.focus();
     };
 
     button.onBreadcrumbsClick = () => {
-      this.selection.setNodeFront(node, "breadcrumbs");
+      this.selection.setNodeFront(node, { reason: "breadcrumbs" });
     };
 
     button.onBreadcrumbsHover = () => {
       this.inspector.toolbox.highlighterUtils.highlightNodeFront(node);
     };
 
     return button;
   },
--- a/devtools/client/inspector/extensions/extension-sidebar.js
+++ b/devtools/client/inspector/extensions/extension-sidebar.js
@@ -86,19 +86,19 @@ class ExtensionSidebar {
             const { highlighterUtils } = this.inspector.toolbox;
 
             if (!highlighterUtils) {
               return null;
             }
 
             let front = await highlighterUtils.gripToNodeFront(grip);
             let onInspectorUpdated = this.inspector.once("inspector-updated");
-            let onNodeFrontSet = this.inspector.toolbox.selection.setNodeFront(
-              front, "inspector-extension-sidebar"
-            );
+            let onNodeFrontSet = this.inspector.toolbox.selection.setNodeFront(front, {
+              reason: "inspector-extension-sidebar"
+            });
 
             return Promise.all([onNodeFrontSet, onInspectorUpdated]);
           }
         },
       }));
     }
 
     return this._provider;
--- a/devtools/client/inspector/flexbox/components/FlexboxItem.js
+++ b/devtools/client/inspector/flexbox/components/FlexboxItem.js
@@ -48,17 +48,17 @@ class FlexboxItem extends PureComponent 
       onToggleFlexboxHighlighter,
     } = this.props;
 
     onToggleFlexboxHighlighter(flexbox.nodeFront);
   }
 
   onFlexboxInspectIconClick(nodeFront) {
     const { setSelectedNode } = this.props;
-    setSelectedNode(nodeFront, "layout-panel").catch(e => console.error(e));
+    setSelectedNode(nodeFront, { reason: "layout-panel" }).catch(e => console.error(e));
     nodeFront.scrollIntoView().catch(e => console.error(e));
   }
 
   render() {
     const {
       flexbox,
       onHideBoxModelHighlighter,
       onShowBoxModelHighlighterForNode,
--- a/devtools/client/inspector/grids/components/GridItem.js
+++ b/devtools/client/inspector/grids/components/GridItem.js
@@ -80,17 +80,17 @@ class GridItem extends PureComponent {
       onToggleGridHighlighter,
     } = this.props;
 
     onToggleGridHighlighter(grid.nodeFront);
   }
 
   onGridInspectIconClick(nodeFront) {
     let { setSelectedNode } = this.props;
-    setSelectedNode(nodeFront, "layout-panel").catch(e => console.error(e));
+    setSelectedNode(nodeFront, { reason: "layout-panel" }).catch(e => console.error(e));
     nodeFront.scrollIntoView().catch(e => console.error(e));
   }
 
   render() {
     let {
       grid,
       onHideBoxModelHighlighter,
       onShowBoxModelHighlighterForNode,
--- a/devtools/client/inspector/inspector-commands.js
+++ b/devtools/client/inspector/inspector-commands.js
@@ -29,17 +29,17 @@ exports.items = [{
     }
   ],
   exec: async function(args, context) {
     let target = context.environment.target;
     let toolbox = await gDevTools.showToolbox(target, "inspector");
     let walker = toolbox.getCurrentPanel().walker;
     let rootNode = await walker.getRootNode();
     let nodeFront = await walker.querySelector(rootNode, args.selector);
-    toolbox.getCurrentPanel().selection.setNodeFront(nodeFront, "gcli");
+    toolbox.getCurrentPanel().selection.setNodeFront(nodeFront, { reason: "gcli" });
   },
 }, {
   item: "command",
   runAt: "client",
   name: "eyedropper",
   description: l10n.lookup("eyedropperDesc"),
   manual: l10n.lookup("eyedropperManual"),
   params: [{
--- a/devtools/client/inspector/inspector-search.js
+++ b/devtools/client/inspector/inspector-search.js
@@ -89,17 +89,17 @@ InspectorSearch.prototype = {
     let res = await this.walker.search(query, { reverse });
 
     // Value has changed since we started this request, we're done.
     if (query !== this.searchBox.value) {
       return;
     }
 
     if (res) {
-      this.inspector.selection.setNodeFront(res.node, "inspectorsearch");
+      this.inspector.selection.setNodeFront(res.node, { reason: "inspectorsearch" });
       this.searchBox.classList.remove("devtools-style-searchbox-no-match");
 
       res.query = query;
       this.emit("search-result", res);
     } else {
       this.searchBox.classList.add("devtools-style-searchbox-no-match");
       this.emit("search-result");
     }
--- a/devtools/client/inspector/inspector.js
+++ b/devtools/client/inspector/inspector.js
@@ -275,17 +275,17 @@ Inspector.prototype = {
     this.setupExtensionSidebars();
 
     await this.once("markuploaded");
     this.isReady = true;
 
     // All the components are initialized. Let's select a node.
     if (defaultSelection) {
       let onAllPanelsUpdated = this.once("inspector-updated");
-      this.selection.setNodeFront(defaultSelection, "inspector-open");
+      this.selection.setNodeFront(defaultSelection, { reason: "inspector-open" });
       await onAllPanelsUpdated;
       await this.markup.expandNode(this.selection.nodeFront);
     }
 
     // And setup the toolbar only now because it may depend on the document.
     await this.setupToolbar();
     this.emit("ready");
     return this;
@@ -1044,17 +1044,17 @@ Inspector.prototype = {
 
     let onNodeSelected = defaultNode => {
       // Cancel this promise resolution as a new one had
       // been queued up.
       if (this._pendingSelection != onNodeSelected) {
         return;
       }
       this._pendingSelection = null;
-      this.selection.setNodeFront(defaultNode, "navigateaway");
+      this.selection.setNodeFront(defaultNode, { reason: "navigateaway" });
 
       this._initMarkup();
       this.once("markuploaded", this.onMarkupLoaded);
 
       // Setup the toolbar again, since its content may depend on the current document.
       this.setupToolbar();
     };
     this._pendingSelection = onNodeSelected;
@@ -1248,17 +1248,18 @@ Inspector.prototype = {
 
   /**
    * When a node is deleted, select its parent node or the defaultNode if no
    * parent is found (may happen when deleting an iframe inside which the
    * node was selected).
    */
   onDetached: function(parentNode) {
     this.breadcrumbs.cutAfter(this.breadcrumbs.indexOf(parentNode));
-    this.selection.setNodeFront(parentNode ? parentNode : this._defaultNode, "detached");
+    let nodeFront = parentNode ? parentNode : this._defaultNode;
+    this.selection.setNodeFront(nodeFront, { reason: "detached" });
   },
 
   /**
    * Destroy the inspector.
    */
   destroy: function() {
     if (this._panelDestroyer) {
       return this._panelDestroyer;
@@ -2320,17 +2321,17 @@ Inspector.prototype = {
     }
 
     let isAttached = await this.walker.isInDOMTree(nodeFront);
     if (!isAttached) {
       console.error("Selected DOMNode is not attached to the document tree.");
       return false;
     }
 
-    await this.selection.setNodeFront(nodeFront, inspectFromAnnotation);
+    await this.selection.setNodeFront(nodeFront, { reason: inspectFromAnnotation });
     return true;
   },
 };
 
 /**
  * Create a fake toolbox when running the inspector standalone, either in a chrome tab or
  * in a content tab.
  *
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -1493,17 +1493,17 @@ MarkupView.prototype = {
    *
    * @param  {NodeFront} aNode
    *         The NodeFront to mark as selected.
    * @param  {String} reason
    *         The reason for marking the node as selected.
    * @return {Boolean} False if the node is already marked as selected, true
    *         otherwise.
    */
-  markNodeAsSelected: function(node, reason) {
+  markNodeAsSelected: function(node, reason = "nodeselected") {
     let container = this.getContainer(node);
 
     if (this._selectedContainer === container) {
       return false;
     }
 
     // Un-select and remove focus from the previous container.
     if (this._selectedContainer) {
@@ -1514,17 +1514,17 @@ MarkupView.prototype = {
     // Select the new container.
     this._selectedContainer = container;
     if (node) {
       this._selectedContainer.selected = true;
     }
 
     // Change the current selection if needed.
     if (this.inspector.selection.nodeFront !== node) {
-      this.inspector.selection.setNodeFront(node, reason || "nodeselected");
+      this.inspector.selection.setNodeFront(node, { reason });
     }
 
     return true;
   },
 
   /**
    * Make sure that every ancestor of the selection are updated
    * and included in the list of visible children.
--- a/devtools/client/inspector/shared/dom-node-preview.js
+++ b/devtools/client/inspector/shared/dom-node-preview.js
@@ -227,17 +227,17 @@ DomNodePreview.prototype = {
     this.highlighterUtils.unhighlight()
                          .catch(console.error);
   },
 
   onSelectElClick: function() {
     if (!this.nodeFront) {
       return;
     }
-    this.inspector.selection.setNodeFront(this.nodeFront, "dom-node-preview");
+    this.inspector.selection.setNodeFront(this.nodeFront, { reason: "dom-node-preview" });
   },
 
   onHighlightElClick: function(e) {
     e.stopPropagation();
 
     let classList = this.highlightNodeEl.classList;
     let isHighlighted = classList.contains("selected");
 
--- a/devtools/client/inspector/test/shared-head.js
+++ b/devtools/client/inspector/test/shared-head.js
@@ -200,17 +200,17 @@ function getNodeFront(selector, {walker}
  * @param {String} reason Defaults to "test" which instructs the inspector not
  * to highlight the node upon selection
  * @return {Promise} Resolves when the inspector is updated with the new node
  */
 var selectNode = async function(selector, inspector, reason = "test") {
   info("Selecting the node for '" + selector + "'");
   let nodeFront = await getNodeFront(selector, inspector);
   let updated = inspector.once("inspector-updated");
-  inspector.selection.setNodeFront(nodeFront, reason);
+  inspector.selection.setNodeFront(nodeFront, { reason });
   await updated;
 };
 
 /**
  * Create a throttling function that can be manually "flushed". This is to replace the
  * use of the `debounce` function from `devtools/client/inspector/shared/utils.js`, which
  * has a setTimeout that can cause intermittents.
  * @return {Function} This function has the same function signature as debounce, but
--- a/devtools/client/shared/widgets/VariablesView.jsm
+++ b/devtools/client/shared/widgets/VariablesView.jsm
@@ -2783,17 +2783,17 @@ Variable.prototype = extend(Scope.protot
         nodeFront = await this.toolbox.walker.getNodeActorFromObjectActor(this._valueGrip.actor);
       }
 
       if (nodeFront) {
         await this.toolbox.selectTool("inspector");
 
         let inspectorReady = defer();
         this.toolbox.getPanel("inspector").once("inspector-updated", inspectorReady.resolve);
-        await this.toolbox.selection.setNodeFront(nodeFront, "variables-view");
+        await this.toolbox.selection.setNodeFront(nodeFront, { reason: "variables-view" });
         await inspectorReady.promise;
       }
     }.bind(this))();
   },
 
   /**
    * In case this variable is a DOMNode and part of a variablesview that has been
    * linked to the toolbox's inspector, then highlight the corresponding node
--- a/devtools/client/webconsole/console-output.js
+++ b/devtools/client/webconsole/console-output.js
@@ -3172,17 +3172,17 @@ Widgets.ObjectRenderers.add({
   async openNodeInInspector() {
     await this.linkToInspector();
     await this.toolbox.selectTool("inspector");
 
     let isAttached = await this.toolbox.walker.isInDOMTree(this._nodeFront);
     if (isAttached) {
       let onReady = defer();
       this.toolbox.inspector.once("inspector-updated", onReady.resolve);
-      await this.toolbox.selection.setNodeFront(this._nodeFront, "console");
+      await this.toolbox.selection.setNodeFront(this._nodeFront, { reason: "console" });
       await onReady.promise;
     } else {
       throw new Error("Node is not attached.");
     }
   },
 
   destroy: function() {
     if (this.toolbox && this._nodeFront) {
--- a/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
+++ b/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
@@ -186,17 +186,18 @@ NewConsoleOutputWrapper.prototype = {
             let onSelectInspector = this.toolbox.selectTool("inspector");
             let onGripNodeToFront = this.toolbox.highlighterUtils.gripToNodeFront(grip);
             let [
               front,
               inspector
             ] = await Promise.all([onGripNodeToFront, onSelectInspector]);
 
             let onInspectorUpdated = inspector.once("inspector-updated");
-            let onNodeFrontSet = this.toolbox.selection.setNodeFront(front, "console");
+            let onNodeFrontSet = this.toolbox.selection
+              .setNodeFront(front, { reason: "console" });
 
             return Promise.all([onNodeFrontSet, onInspectorUpdated]);
           }
         });
       }
 
       let consoleOutput = ConsoleOutput({
         serviceContainer,