Bug 1414303: Make nsLayoutUtils::CompareTreePosition handle Shadow DOM in a sensible way. r?xidorn draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 14 May 2018 16:34:45 +0200
changeset 794842 7a2159f7315192e150c1aaddcd44071e306c9d1c
parent 794800 6cecfe800b0eec66f74d41afa1941fda8140f8a5
push id109793
push userbmo:emilio@crisal.io
push dateMon, 14 May 2018 17:18:24 +0000
reviewersxidorn
bugs1414303
milestone62.0a1
Bug 1414303: Make nsLayoutUtils::CompareTreePosition handle Shadow DOM in a sensible way. r?xidorn We probably need more fixes for counters and Shadow DOM. The spec only mentions "document order", and this is the most reasonable thing to do accounting for shadow DOM in that regard... This ensures a reasonable behavior for all callers which pretty much expect otherwise for all children to be connected. MozReview-Commit-ID: YEQIKdjRTK
layout/base/crashtests/1414303.html
layout/base/crashtests/crashtests.list
layout/base/nsGenConList.cpp
layout/base/nsLayoutUtils.cpp
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1414303.html
@@ -0,0 +1,11 @@
+<style>
+  * { counter-reset: c; }
+</style>
+<script>
+function go() {
+  host.attachShadow({ mode: "open" }).innerHTML = form.outerHTML;
+}
+</script>
+<body onload=go()>
+<form id="form" style="counter-reset: c">
+  <div id="host">
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -525,8 +525,9 @@ pref(dom.webcomponents.shadowdom.enabled
 load 1442506.html
 load 1437155.html
 load 1443027-1.html
 load 1448841-1.html
 load 1452839.html
 load 1453702.html
 load 1453342.html
 load 1453196.html
+pref(dom.webcomponents.shadowdom.enabled,true) load 1414303.html
--- a/layout/base/nsGenConList.cpp
+++ b/layout/base/nsGenConList.cpp
@@ -69,24 +69,24 @@ inline int32_t PseudoCompareType(nsIFram
   }
   *aContent = aFrame->GetContent();
   return 0;
 }
 
 /* static */ bool
 nsGenConList::NodeAfter(const nsGenConNode* aNode1, const nsGenConNode* aNode2)
 {
-  nsIFrame *frame1 = aNode1->mPseudoFrame;
-  nsIFrame *frame2 = aNode2->mPseudoFrame;
+  nsIFrame* frame1 = aNode1->mPseudoFrame;
+  nsIFrame* frame2 = aNode2->mPseudoFrame;
   if (frame1 == frame2) {
     NS_ASSERTION(aNode2->mContentIndex != aNode1->mContentIndex, "identical");
     return aNode1->mContentIndex > aNode2->mContentIndex;
   }
-  nsIContent *content1;
-  nsIContent *content2;
+  nsIContent* content1;
+  nsIContent* content2;
   int32_t pseudoType1 = PseudoCompareType(frame1, &content1);
   int32_t pseudoType2 = PseudoCompareType(frame2, &content2);
   if (pseudoType1 == 0 || pseudoType2 == 0) {
     if (content1 == content2) {
       NS_ASSERTION(pseudoType1 != pseudoType2, "identical");
       return pseudoType2 == 0;
     }
     // We want to treat an element as coming before its :before (preorder
@@ -94,17 +94,17 @@ nsGenConList::NodeAfter(const nsGenConNo
     if (pseudoType1 == 0) pseudoType1 = -1;
     if (pseudoType2 == 0) pseudoType2 = -1;
   } else {
     if (content1 == content2) {
       NS_ASSERTION(pseudoType1 != pseudoType2, "identical");
       return pseudoType1 == 1;
     }
   }
-  // XXX Switch to the frame version of DoCompareTreePosition?
+
   int32_t cmp = nsLayoutUtils::DoCompareTreePosition(content1, content2,
                                                      pseudoType1, -pseudoType2);
   MOZ_ASSERT(cmp != 0, "same content, different frames");
   return cmp > 0;
 }
 
 void
 nsGenConList::Insert(nsGenConNode* aNode)
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -1623,28 +1623,32 @@ nsLayoutUtils::DoCompareTreePosition(nsI
                                      int32_t aIf2Ancestor,
                                      const nsIContent* aCommonAncestor)
 {
   MOZ_ASSERT(aContent1, "aContent1 must not be null");
   MOZ_ASSERT(aContent2, "aContent2 must not be null");
 
   AutoTArray<nsINode*, 32> content1Ancestors;
   nsINode* c1;
-  for (c1 = aContent1; c1 && c1 != aCommonAncestor; c1 = c1->GetParentNode()) {
+  for (c1 = aContent1;
+       c1 && c1 != aCommonAncestor;
+       c1 = c1->GetParentOrHostNode()) {
     content1Ancestors.AppendElement(c1);
   }
   if (!c1 && aCommonAncestor) {
     // So, it turns out aCommonAncestor was not an ancestor of c1. Oops.
     // Never mind. We can continue as if aCommonAncestor was null.
     aCommonAncestor = nullptr;
   }
 
   AutoTArray<nsINode*, 32> content2Ancestors;
   nsINode* c2;
-  for (c2 = aContent2; c2 && c2 != aCommonAncestor; c2 = c2->GetParentNode()) {
+  for (c2 = aContent2;
+       c2 && c2 != aCommonAncestor;
+       c2 = c2->GetParentOrHostNode()) {
     content2Ancestors.AppendElement(c2);
   }
   if (!c2 && aCommonAncestor) {
     // So, it turns out aCommonAncestor was not an ancestor of c2.
     // We need to retry with no common ancestor hint.
     return DoCompareTreePosition(aContent1, aContent2,
                                  aIf1Ancestor, aIf2Ancestor, nullptr);
   }
@@ -1670,17 +1674,17 @@ nsLayoutUtils::DoCompareTreePosition(nsI
   }
 
   if (last2 < 0) {
     // aContent2 is an ancestor of aContent1
     return aIf2Ancestor;
   }
 
   // content1Ancestor != content2Ancestor, so they must be siblings with the same parent
-  nsINode* parent = content1Ancestor->GetParentNode();
+  nsINode* parent = content1Ancestor->GetParentOrHostNode();
 #ifdef DEBUG
   // TODO: remove the uglyness, see bug 598468.
   NS_ASSERTION(gPreventAssertInCompareTreePosition || parent,
                "no common ancestor at all???");
 #endif // DEBUG
   if (!parent) { // different documents??
     return 0;
   }