Bug 1414303: Make nsLayoutUtils::CompareTreePosition handle Shadow DOM in a sensible way. r=xidorn
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 14 May 2018 16:34:45 +0200
changeset 418306 06ef43a833cec2f4741f04e4a51e27ced55744d3
parent 418257 2fa8aedae0ffbed270af75dd62489bc33f589b0f
child 418307 9c5ddcb72b1de01a342a9299bd3b90bb697a6092
push id33998
push userdluca@mozilla.com
push dateTue, 15 May 2018 21:53:06 +0000
treeherdermozilla-central@9260cc524bb5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn
bugs1414303
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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;
   }