Bug 1053898 - Update WalkerActor to return light DOM nodes as children of host element;r=bgrins draft
authorJulian Descottes <jdescottes@mozilla.com>
Mon, 05 Mar 2018 20:06:56 +0100
changeset 773668 bf3a2d42ddb07b5bac08fc95226e9751874c1f7a
parent 773667 b42b7e42c86d5ad1e4e3d435a3f2908dda08d1f1
child 773669 31e8cb68519893a371504885fe34ee3e58372cfd
push id104269
push userjdescottes@mozilla.com
push dateWed, 28 Mar 2018 08:16:27 +0000
reviewersbgrins
bugs1053898
milestone61.0a1
Bug 1053898 - Update WalkerActor to return light DOM nodes as children of host element;r=bgrins Light DOM nodes are now returned next to the shadow-root, however they are skipped if they are found under a slot element, because the markup view can not handle several containers for a single nodeFront yet. MozReview-Commit-ID: 1df3VBPT2HX
devtools/server/actors/inspector/node.js
devtools/server/actors/inspector/walker.js
--- a/devtools/server/actors/inspector/node.js
+++ b/devtools/server/actors/inspector/node.js
@@ -192,16 +192,26 @@ const NodeActor = protocol.ActorClassWit
     return this.rawNode.nodeName === "_moz_generated_content_after";
   },
 
   get isShadowRoot() {
     let isFragment = this.rawNode.nodeType === Ci.nsIDOMNode.DOCUMENT_FRAGMENT_NODE;
     return isFragment && this.rawNode.host;
   },
 
+  get isDirectShadowHostChild() {
+    // Pseudo elements are always part of the anonymous tree.
+    if (this.isBeforePseudoElement || this.isAfterPseudoElement) {
+      return false;
+    }
+
+    let parentNode = this.rawNode.parentNode;
+    return parentNode && parentNode.shadowRoot;
+  },
+
   // Estimate the number of children that the walker will return without making
   // a call to children() if possible.
   get numChildren() {
     // For pseudo elements, childNodes.length returns 1, but the walker
     // will return 0.
     if (this.isBeforePseudoElement || this.isAfterPseudoElement) {
       return 0;
     }
@@ -215,17 +225,17 @@ const NodeActor = protocol.ActorClassWit
     let hasSVGDocument = rawNode.getSVGDocument && rawNode.getSVGDocument();
     if (numChildren === 0 && (hasContentDocument || hasSVGDocument)) {
       // This might be an iframe with virtual children.
       numChildren = 1;
     }
 
     // Normal counting misses ::before/::after.  Also, some anonymous children
     // may ultimately be skipped, so we have to consult with the walker.
-    if (numChildren === 0 || hasAnonChildren) {
+    if (numChildren === 0 || hasAnonChildren || this.isShadowHost) {
       numChildren = this.walker.children(this).nodes.length;
     }
 
     return numChildren;
   },
 
   get computedStyle() {
     if (!this._computedStyle) {
--- a/devtools/server/actors/inspector/walker.js
+++ b/devtools/server/actors/inspector/walker.js
@@ -421,21 +421,35 @@ var WalkerActor = protocol.ActorClassWit
   documentElement: function(node) {
     let elt = isNodeDead(node)
               ? this.rootDoc.documentElement
               : nodeDocument(node.rawNode).documentElement;
     return this._ref(elt);
   },
 
   parentNode: function(node) {
-    let walker = this.getDocumentWalker(node.rawNode);
-    let parent = walker.parentNode();
+    let parent;
+    try {
+      // If the node is the child of a shadow host, we can not use an anonymous walker to
+      // get the shadow host parent.
+      let walker = node.isDirectShadowHostChild ? this.getNonAnonymousWalker(node.rawNode)
+                                                : this.getDocumentWalker(node.rawNode);
+      parent = walker.parentNode();
+    } catch (e) {
+      // When getting the parent node for a child of a non-slotted shadow host child,
+      // walker.parentNode() will throw if the walker is anonymous, because non-slotted
+      // shadow host children are not accessible anywhere in the anonymous tree.
+      let walker = this.getNonAnonymousWalker(node.rawNode);
+      parent = walker.parentNode();
+    }
+
     if (parent) {
       return this._ref(parent);
     }
+
     return null;
   },
 
   /**
    * If the given NodeActor only has a single text node as a child with a text
    * content small enough to be inlined, return that child's NodeActor.
    *
    * @param NodeActor node
@@ -444,25 +458,26 @@ var WalkerActor = protocol.ActorClassWit
     // Quick checks to prevent creating a new walker if possible.
     if (node.isBeforePseudoElement ||
         node.isAfterPseudoElement ||
         node.rawNode.nodeType != Ci.nsIDOMNode.ELEMENT_NODE ||
         node.rawNode.children.length > 0) {
       return undefined;
     }
 
-    let docWalker = this.getDocumentWalker(node.rawNode);
-    let firstChild = docWalker.firstChild();
+    let walker = node.isDirectShadowHostChild ? this.getNonAnonymousWalker(node.rawNode)
+                                              : this.getDocumentWalker(node.rawNode);
+    let firstChild = walker.firstChild();
 
     // Bail out if:
     // - more than one child
     // - unique child is not a text node
     // - unique child is a text node, but is too long to be inlined
     if (!firstChild ||
-        docWalker.nextSibling() ||
+        walker.nextSibling() ||
         firstChild.nodeType !== Ci.nsIDOMNode.TEXT_NODE ||
         firstChild.nodeValue.length > gValueSummaryLength
         ) {
       return undefined;
     }
 
     return this._ref(firstChild);
   },
@@ -588,94 +603,130 @@ var WalkerActor = protocol.ActorClassWit
       throw Error("Can't specify both 'center' and 'start' options.");
     }
     let maxNodes = options.maxNodes || -1;
     if (maxNodes == -1) {
       maxNodes = Number.MAX_VALUE;
     }
 
     let isShadowHost = !!node.rawNode.shadowRoot;
+    let isShadowRoot = !!node.rawNode.host;
 
-    if (isShadowHost) {
-      let shadowRoot = this._ref(node.rawNode.shadowRoot);
-      return {
-        hasFirst: true,
-        hasLast: true,
-        nodes: [shadowRoot],
-      };
+    // Detect special case of unslotted shadow host children that cannot rely on a
+    // regular anonymous walker.
+    let isUnslottedHostChild = false;
+    if (node.isDirectShadowHostChild) {
+      try {
+        this.getDocumentWalker(node.rawNode, options.whatToShow, SKIP_TO_SIBLING);
+      } catch (e) {
+        isUnslottedHostChild = true;
+      }
     }
 
-    let isShadowRoot = !!node.rawNode.host;
     // We're going to create a few document walkers with the same filter,
     // make it easier.
     let getFilteredWalker = documentWalkerNode => {
       let { whatToShow } = options;
+
       // Use SKIP_TO_SIBLING to force the walker to use a sibling of the provided node
       // in case this one is incompatible with the walker's filter function.
-      if (isShadowRoot) {
-        // Do not fetch anonymous children for shadow roots. If the host element has an
-        // ::after pseudo element, a walker on the last child of the shadow root will
-        // jump to the ::after element, which is not a child of the shadow root.
-        // TODO: Should rather use an anonymous walker with a new dedicated filter.
-        return this.getNonAnonymousWalker(documentWalkerNode, whatToShow,
-          SKIP_TO_SIBLING);
+      let skipTo = SKIP_TO_SIBLING;
+
+      let useAnonymousWalker = !(isShadowRoot || isShadowHost || isUnslottedHostChild);
+      if (!useAnonymousWalker) {
+        // Do not use an anonymous walker for :
+        // - shadow roots: if the host element has an ::after pseudo element, a walker on
+        //   the last child of the shadow root will jump to the ::after element, which is
+        //   not a child of the shadow root.
+        //   TODO: For this case, should rather use an anonymous walker with a new
+        //         dedicated filter.
+        // - shadow hosts: anonymous children of host elements make up the shadow dom,
+        //   while we want to return the direct children of the shadow host.
+        // - unslotted host child: if a shadow host child is not slotted, it is not part
+        //   of any anonymous tree and cannot be used with anonymous tree walkers.
+        return this.getNonAnonymousWalker(documentWalkerNode, whatToShow, skipTo);
       }
-      return this.getDocumentWalker(documentWalkerNode, whatToShow, SKIP_TO_SIBLING);
+      return this.getDocumentWalker(documentWalkerNode, whatToShow, skipTo);
     };
 
     // Need to know the first and last child.
     let rawNode = node.rawNode;
     let firstChild = getFilteredWalker(rawNode).firstChild();
     let lastChild = getFilteredWalker(rawNode).lastChild();
 
-    if (!firstChild) {
+    if (!firstChild && !isShadowHost) {
       // No children, we're done.
       return { hasFirst: true, hasLast: true, nodes: [] };
     }
 
-    let start;
-    if (options.center) {
-      start = options.center.rawNode;
-    } else if (options.start) {
-      start = options.start.rawNode;
-    } else {
-      start = firstChild;
-    }
-
     let nodes = [];
 
-    // Start by reading backward from the starting point if we're centering...
-    let backwardWalker = getFilteredWalker(start);
-    if (backwardWalker.currentNode != firstChild && options.center) {
-      backwardWalker.previousSibling();
-      let backwardCount = Math.floor(maxNodes / 2);
-      let backwardNodes = this._readBackward(backwardWalker, backwardCount);
-      nodes = backwardNodes;
+    if (firstChild) {
+      let start;
+      if (options.center) {
+        start = options.center.rawNode;
+      } else if (options.start) {
+        start = options.start.rawNode;
+      } else {
+        start = firstChild;
+      }
+
+      // Start by reading backward from the starting point if we're centering...
+      let backwardWalker = getFilteredWalker(start);
+      if (backwardWalker.currentNode != firstChild && options.center) {
+        backwardWalker.previousSibling();
+        let backwardCount = Math.floor(maxNodes / 2);
+        let backwardNodes = this._readBackward(backwardWalker, backwardCount);
+        nodes = backwardNodes;
+      }
+
+      // Then read forward by any slack left in the max children...
+      let forwardWalker = getFilteredWalker(start);
+      let forwardCount = maxNodes - nodes.length;
+      nodes = nodes.concat(this._readForward(forwardWalker, forwardCount));
+
+      // If there's any room left, it means we've run all the way to the end.
+      // If we're centering, check if there are more items to read at the front.
+      let remaining = maxNodes - nodes.length;
+      if (options.center && remaining > 0 && nodes[0].rawNode != firstChild) {
+        let firstNodes = this._readBackward(backwardWalker, remaining);
+
+        // Then put it all back together.
+        nodes = firstNodes.concat(nodes);
+      }
     }
 
-    // Then read forward by any slack left in the max children...
-    let forwardWalker = getFilteredWalker(start);
-    let forwardCount = maxNodes - nodes.length;
-    nodes = nodes.concat(this._readForward(forwardWalker, forwardCount));
-
-    // If there's any room left, it means we've run all the way to the end.
-    // If we're centering, check if there are more items to read at the front.
-    let remaining = maxNodes - nodes.length;
-    if (options.center && remaining > 0 && nodes[0].rawNode != firstChild) {
-      let firstNodes = this._readBackward(backwardWalker, remaining);
-
-      // Then put it all back together.
-      nodes = firstNodes.concat(nodes);
+    // Temporarily filter out shadow host children when a walker returns them in a <slot>.
+    if (!isShadowHost) {
+      // Shadow host children should only be displayed under the host.
+      nodes = nodes.filter(n => !n.isDirectShadowHostChild);
     }
 
-    return {
-      hasFirst: nodes[0].rawNode == firstChild,
-      hasLast: nodes[nodes.length - 1].rawNode == lastChild,
-      nodes: nodes
-    };
+    let hasFirst, hasLast;
+    if (nodes.length > 0) {
+      // Compare first/last with expected nodes before modifying the nodes array in case
+      // this is a shadow host.
+      hasFirst = nodes[0].rawNode == firstChild;
+      hasLast = nodes[nodes.length - 1].rawNode == lastChild;
+    } else {
+      // If nodes is still an empty array, we are on a host element with a shadow root but
+      // no direct children.
+      hasFirst = hasLast = true;
+    }
+
+    if (isShadowHost) {
+      nodes = [
+        // #shadow-root
+        this._ref(node.rawNode.shadowRoot),
+        // shadow host direct children
+        ...nodes,
+      ];
+    }
+
+    return { hasFirst, hasLast, nodes };
   },
 
   /**
    * Get the next sibling of a given node.  Getting nodes one at a time
    * might be inefficient, be careful.
    *
    * @param object options
    *    Named options: