Bug 1590050 - Preserve markup view selection in iframes after reload r=rcaliman,gl

Depends on D49940

To support this feature we perform two main changes
- the node actor exposes a getAllSelectors method, and the inspector now stores all selectors rather than just one
- the node actor exposes a waitForFrameLoad method, and the walkerFront findNodeFront helper uses it to make sure frames are loaded before querying a selector

Also added a test

Differential Revision: https://phabricator.services.mozilla.com/D49941

--HG--
extra : moz-landing-system : lando

devtools/client/inspector/inspector.js

@@ -459,8 +459,8 @@ Inspector.prototype = {
         }
 
         rootNode = node;
-        if (this.selectionCssSelector) {
-          return walker.querySelector(rootNode, this.selectionCssSelector);
+        if (this.selectionCssSelectors.length) {
+          return walker.findNodeFront(this.selectionCssSelectors);
         }
         return null;
       })
@@ -1400,46 +1400,63 @@ Inspector.prototype = {
     }
   },
 
-  _selectionCssSelector: null,
+  _selectionCssSelectors: null,
 
   /**
-   * Set the currently selected node unique css selector.
+   * Set the array of CSS selectors for the currently selected node.
+   * We use an array of selectors in case the element is in iframes.
    * Will store the current target url along with it to allow pre-selection at
    * reload
    */
-  set selectionCssSelector(cssSelector = null) {
+  set selectionCssSelectors(cssSelectors = []) {
     if (this._destroyed) {
       return;
     }
 
-    this._selectionCssSelector = {
-      selector: cssSelector,
+    this._selectionCssSelectors = {
+      selectors: cssSelectors,
       url: this._target.url,
     };
   },
 
   /**
-   * Get the current selection unique css selector if any, that is, if a node
+   * Get the CSS selectors for the current selection if any, that is, if a node
    * is actually selected and that node has been selected while on the same url
    */
-  get selectionCssSelector() {
+  get selectionCssSelectors() {
     if (
-      this._selectionCssSelector &&
-      this._selectionCssSelector.url === this._target.url
+      this._selectionCssSelectors &&
+      this._selectionCssSelectors.url === this._target.url
     ) {
-      return this._selectionCssSelector.selector;
+      return this._selectionCssSelectors.selectors;
     }
+    return [];
+  },
+
+  /**
+   * Some inspector ruleview helpers rely on the selectionCssSelector to get the
+   * unique CSS selector of the selected element only within its host document,
+   * disregarding ancestor iframes.
+   * They should not care about the complete array of CSS selectors, only
+   * relevant in order to reselect the proper node when reloading pages with
+   * frames.
+   */
+  get selectionCssSelector() {
+    if (this.selectionCssSelectors.length) {
+      return this.selectionCssSelectors[this.selectionCssSelectors.length - 1];
+    }
+
     return null;
   },
 
   /**
-   * On any new selection made by the user, store the unique css selector
+   * On any new selection made by the user, store the array of css selectors
    * of the selected node so it can be restored after reload of the same page
    */
-  updateSelectionCssSelector() {
+  updateSelectionCssSelectors() {
     if (this.selection.isElementNode()) {
-      this.selection.nodeFront.getUniqueSelector().then(selector => {
-        this.selectionCssSelector = selector;
+      this.selection.nodeFront.getAllSelectors().then(selectors => {
+        this.selectionCssSelectors = selectors;
       }, this._handleRejectionIfNotDestroyed);
     }
   },
@@ -1509,7 +1526,7 @@ Inspector.prototype = {
     }
 
     this.updateAddElementButton();
-    this.updateSelectionCssSelector();
+    this.updateSelectionCssSelectors();
 
     const selfUpdate = this.updating("inspector-panel");
     executeSoon(() => {

devtools/client/inspector/test/browser.ini

@@ -186,6 +186,7 @@ skip-if = fission
 [browser_inspector_pseudoclass-menu.js]
 [browser_inspector_reload-01.js]
 [browser_inspector_reload-02.js]
+[browser_inspector_reload_iframe.js]
 [browser_inspector_reload_xul.js]
 [browser_inspector_remove-iframe-during-load.js]
 fail-if = fission

devtools/client/inspector/test/browser_inspector_reload_iframe.js

@@ -0,0 +1,43 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+"use strict";
+
+// Check that the markup view selection is preserved even if the selection is
+// in an iframe.
+
+const FRAME_URI =
+  "data:text/html;charset=utf-8," +
+  encodeURI(`<div id="in-frame">div in the iframe</div>`);
+const HTML = `
+  <iframe src="${FRAME_URI}"></iframe>
+`;
+
+const TEST_URI = "data:text/html;charset=utf-8," + encodeURI(HTML);
+
+add_task(async function() {
+  const { inspector, testActor } = await openInspectorForURL(TEST_URI);
+
+  const nodeFront = await getNodeFrontInFrame("#in-frame", "iframe", inspector);
+  await selectNode(nodeFront, inspector);
+
+  const markupLoaded = inspector.once("markuploaded");
+
+  info("Reloading page.");
+  await testActor.eval("location.reload()");
+
+  info("Waiting for markupview to load after reload.");
+  await markupLoaded;
+
+  const reloadedNodeFront = await getNodeFrontInFrame(
+    "#in-frame",
+    "iframe",
+    inspector
+  );
+
+  is(
+    inspector.selection.nodeFront,
+    reloadedNodeFront,
+    "#in-frame selected after reload."
+  );
+});

devtools/server/actors/inspector/node.js

@@ -28,6 +28,12 @@ loader.lazyRequireGetter(
   "devtools/shared/inspector/css-logic",
   true
 );
+loader.lazyRequireGetter(
+  this,
+  "findAllCssSelectors",
+  "devtools/shared/inspector/css-logic",
+  true
+);
 
 loader.lazyRequireGetter(
   this,
@@ -137,6 +143,12 @@ loader.lazyRequireGetter(
   "devtools/server/actors/inspector/utils",
   true
 );
+loader.lazyRequireGetter(
+  this,
+  "DOMHelpers",
+  "resource://devtools/client/shared/DOMHelpers.jsm",
+  true
+);
 
 const SUBGRID_ENABLED = Services.prefs.getBoolPref(
   "layout.css.grid-template-subgrid-value.enabled"
@@ -255,6 +267,12 @@ const NodeActor = protocol.ActorClassWithSpec(nodeSpec, {
         this.rawNode.ownerDocument &&
         this.rawNode.ownerDocument.contentType === "text/html",
       hasEventListeners: this._hasEventListeners,
+      traits: {
+        // Added in FF72
+        supportsGetAllSelectors: true,
+        // Added in FF72
+        supportsWaitForFrameLoad: true,
+      },
     };
 
     if (this.isDocumentElement()) {
@@ -543,11 +561,22 @@ const NodeActor = protocol.ActorClassWithSpec(nodeSpec, {
    */
   getUniqueSelector: function() {
     if (Cu.isDeadWrapper(this.rawNode)) {
-      return "";
+      return [];
     }
     return findCssSelector(this.rawNode);
   },
 
+  /**
+   * Get the full array of selectors from the topmost document, going through
+   * iframes.
+   */
+  getAllSelectors: function() {
+    if (Cu.isDeadWrapper(this.rawNode)) {
+      return "";
+    }
+    return findAllCssSelectors(this.rawNode);
+  },
+
   /**
    * Get the full CSS path for this node.
    *
@@ -701,6 +730,23 @@ const NodeActor = protocol.ActorClassWithSpec(nodeSpec, {
       innerHeight: win.innerHeight,
     };
   },
+
+  /**
+   * If the current node is an iframe, wait for the content window to be loaded.
+   */
+  async waitForFrameLoad() {
+    if (Cu.isDeadWrapper(this.rawNode)) {
+      return;
+    }
+
+    const { contentDocument, contentWindow } = this.rawNode;
+    if (contentDocument && contentDocument.readyState !== "complete") {
+      await new Promise(resolve => {
+        const domHelper = new DOMHelpers(contentWindow);
+        domHelper.onceDOMReady(resolve);
+      });
+    }
+  },
 });
 
 /**

devtools/shared/fronts/node.js

@@ -142,6 +142,8 @@ class NodeFront extends FrontClassWithSpec(nodeSpec) {
       form.nodeValue = form.incompleteValue ? null : form.shortValue;
     }
 
+    this.traits = form.traits || {};
+
     // Shallow copy of the form.  We could just store a reference, but
     // eventually we'll want to update some of the data.
     this._form = Object.assign({}, form);
@@ -533,6 +535,17 @@ class NodeFront extends FrontClassWithSpec(nodeSpec) {
     this._remoteFrameTarget = await descriptor.getTarget();
     return this._remoteFrameTarget;
   }
+
+  async getAllSelectors() {
+    if (!this.traits.supportsGetAllSelectors) {
+      // Backward compatibility: if the server does not support getAllSelectors
+      // fallback on getUniqueSelector and wrap the response in an array.
+      // getAllSelectors was added in FF72.
+      const selector = await super.getUniqueSelector();
+      return [selector];
+    }
+    return super.getAllSelectors();
+  }
 }
 
 exports.NodeFront = NodeFront;

devtools/shared/fronts/walker.js

@@ -491,7 +491,15 @@ class WalkerFront extends FrontClassWithSpec(walkerSpec) {
       }
       nodeFront = await this.querySelector(nodeFront, selector);
       if (nodeSelectors.length > 0) {
+        if (nodeFront.traits.supportsWaitForFrameLoad) {
+          // Backward compatibility: only FF72 or newer are able to wait for
+          // iframes to load. After FF72 reaches release we can unconditionally
+          // call waitForFrameLoad.
+          await nodeFront.waitForFrameLoad();
+        }
+
         const { nodes } = await this.children(nodeFront);
+
         // If there are remaining selectors to process, they will target a document or a
         // document-fragment under the current node. Whether the element is a frame or
         // a web component, it can only contain one document/document-fragment, so just

devtools/shared/inspector/css-logic.js

@@ -24,6 +24,11 @@ loader.lazyImporter(
   "findCssSelector",
   "resource://gre/modules/css-selector.js"
 );
+loader.lazyImporter(
+  this,
+  "findAllCssSelectors",
+  "resource://gre/modules/css-selector.js"
+);
 loader.lazyImporter(
   this,
   "getCssPath",
@@ -497,6 +502,16 @@ exports.prettifyCSS = prettifyCSS;
  */
 exports.findCssSelector = findCssSelector;
 
+/**
+ * Retrieve the array of CSS selectors corresponding to the provided node.
+ *
+ * The selectors are ordered starting with the root document and ending with the deepest
+ * nested frame. Additional items are used if the node is inside a frame or a shadow root,
+ * each representing the CSS selector for finding the frame or root element in its parent
+ * document.
+ */
+exports.findAllCssSelectors = findAllCssSelectors;
+
 /**
  * Get the full CSS path for a given element.
  * @returns a string that can be used as a CSS selector for the element. It might not

devtools/shared/specs/node.js

@@ -88,6 +88,12 @@ const nodeSpec = generateActorSpec({
         value: RetVal("string"),
       },
     },
+    getAllSelectors: {
+      request: {},
+      response: {
+        value: RetVal("array:string"),
+      },
+    },
     getCssPath: {
       request: {},
       response: {
@@ -148,6 +154,10 @@ const nodeSpec = generateActorSpec({
       // Alex: Can we do something to address that??
       response: RetVal("json"),
     },
+    waitForFrameLoad: {
+      request: {},
+      response: {},
+    },
   },
 });