Bug 670467. Correctly reresolve style on non-first continuations and non-first parts of {ib} splits even when we're restyling both an ancestor of the element and the element itself. r=dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 11 Aug 2011 23:52:21 -0400
changeset 74299 77a0d9f09820956bce5c4e1818ed540762fd6c23
parent 74298 1f84f5eb5079bffc321c115f167db8402841de8e
child 74300 c7c13a66ad615d19d2dd91484fa4a3288868a772
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
reviewersdbaron
bugs670467
milestone8.0a1
Bug 670467. Correctly reresolve style on non-first continuations and non-first parts of {ib} splits even when we're restyling both an ancestor of the element and the element itself. r=dbaron
layout/base/nsFrameManager.cpp
layout/reftests/bugs/670467-1-ref.html
layout/reftests/bugs/670467-1.html
layout/reftests/bugs/670467-2-ref.html
layout/reftests/bugs/670467-2.html
layout/reftests/bugs/reftest.list
--- a/layout/base/nsFrameManager.cpp
+++ b/layout/base/nsFrameManager.cpp
@@ -761,16 +761,44 @@ ElementForStyleContext(nsIContent* aPare
     nsBlockFrame* block = nsBlockFrame::GetNearestAncestorBlock(aFrame);
     return block->GetContent()->AsElement();
   }
 
   nsIContent* content = aParentContent ? aParentContent : aFrame->GetContent();
   return content->AsElement();
 }
 
+static nsIFrame*
+GetPrevContinuationWithPossiblySameStyle(nsIFrame* aFrame)
+{
+  // Account for {ib} splits when looking for "prevContinuation".  In
+  // particular, for the first-continuation of a part of an {ib} split we
+  // want to use the special prevsibling of the special prevsibling of
+  // aFrame, which should have the same style context as aFrame itself.
+  // In particular, if aFrame is the first continuation of an inline part
+  // of an {ib} split then its special prevsibling is a block, and the
+  // special prevsibling of _that_ is an inline, just like aFrame.
+  // Similarly, if aFrame is the first continuation of a block part of an
+  // {ib} split (an {ib} wrapper block), then its special prevsibling is
+  // an inline and the special prevsibling of that is either another {ib}
+  // wrapper block block or null.
+  nsIFrame *prevContinuation = aFrame->GetPrevContinuation();
+  if (!prevContinuation && (aFrame->GetStateBits() & NS_FRAME_IS_SPECIAL)) {
+    // We're the first continuation, so we can just get the frame
+    // property directly
+    prevContinuation = static_cast<nsIFrame*>(
+      aFrame->Properties().Get(nsIFrame::IBSplitSpecialPrevSibling()));
+    if (prevContinuation) {
+      prevContinuation = static_cast<nsIFrame*>(
+        prevContinuation->Properties().Get(nsIFrame::IBSplitSpecialPrevSibling()));
+    }
+  }
+  return prevContinuation;
+}
+
 nsresult
 nsFrameManager::ReparentStyleContext(nsIFrame* aFrame)
 {
   if (nsGkAtoms::placeholderFrame == aFrame->GetType()) {
     // Also reparent the out-of-flow and all its continuations.
     nsIFrame* outOfFlow =
       nsPlaceholderFrame::GetRealFrameForPlaceholder(aFrame);
     NS_ASSERTION(outOfFlow, "no out-of-flow frame");
@@ -826,17 +854,18 @@ nsFrameManager::ReparentStyleContext(nsI
                        nextContinuationContext->GetPseudo() ||
                      oldContext->GetParent() !=
                        nextContinuationContext->GetParent(),
                      "continuations should have the same style context");
       }
     }
 #endif
 
-    nsIFrame *prevContinuation = aFrame->GetPrevContinuation();
+    nsIFrame *prevContinuation =
+      GetPrevContinuationWithPossiblySameStyle(aFrame);
     nsStyleContext *prevContinuationContext;
     PRBool copyFromContinuation =
       prevContinuation &&
       (prevContinuationContext = prevContinuation->GetStyleContext())
         ->GetPseudo() == oldContext->GetPseudo() &&
        prevContinuationContext->GetParent() == newParentContext;
     if (copyFromContinuation) {
       // Just use the style context from the frame's previous
@@ -1142,22 +1171,45 @@ nsFrameManager::ReResolveStyleContext(ns
           nextContinuation->GetStyleContext();
         NS_ASSERTION(oldContext == nextContinuationContext ||
                      oldContext->GetPseudo() !=
                        nextContinuationContext->GetPseudo() ||
                      oldContext->GetParent() !=
                        nextContinuationContext->GetParent(),
                      "continuations should have the same style context");
       }
+      // And assert the same thing for {ib} splits.  See the comments in
+      // GetPrevContinuationWithPossiblySameStyle for an explanation of
+      // why we step two forward in the special sibling chain.
+      if ((aFrame->GetStateBits() & NS_FRAME_IS_SPECIAL) &&
+          !aFrame->GetPrevContinuation()) {
+        nsIFrame *nextIBSibling = static_cast<nsIFrame*>(
+          aFrame->Properties().Get(nsIFrame::IBSplitSpecialSibling()));
+        if (nextIBSibling) {
+          nextIBSibling = static_cast<nsIFrame*>(
+            nextIBSibling->Properties().Get(nsIFrame::IBSplitSpecialSibling()));
+        }
+        if (nextIBSibling) {
+          nsStyleContext *nextIBSiblingContext =
+            nextIBSibling->GetStyleContext();
+          NS_ASSERTION(oldContext == nextIBSiblingContext ||
+                       oldContext->GetPseudo() !=
+                         nextIBSiblingContext->GetPseudo() ||
+                       oldContext->GetParent() !=
+                         nextIBSiblingContext->GetParent(),
+                       "continuations should have the same style context");
+        }
+      }
     }
 #endif
 
     // do primary context
     nsRefPtr<nsStyleContext> newContext;
-    nsIFrame *prevContinuation = aFrame->GetPrevContinuation();
+    nsIFrame *prevContinuation =
+      GetPrevContinuationWithPossiblySameStyle(aFrame);
     nsStyleContext *prevContinuationContext;
     PRBool copyFromContinuation =
       prevContinuation &&
       (prevContinuationContext = prevContinuation->GetStyleContext())
         ->GetPseudo() == oldContext->GetPseudo() &&
        prevContinuationContext->GetParent() == parentContext;
     if (copyFromContinuation) {
       // Just use the style context from the frame's previous
@@ -1166,17 +1218,25 @@ nsFrameManager::ReResolveStyleContext(ns
       // continuation).
       newContext = prevContinuationContext;
     }
     else if (pseudoTag == nsCSSAnonBoxes::mozNonElement) {
       NS_ASSERTION(localContent,
                    "non pseudo-element frame without content node");
       newContext = styleSet->ResolveStyleForNonElement(parentContext);
     }
-    else if (!aRestyleHint) {
+    else if (!aRestyleHint && !prevContinuation) {
+      // Unfortunately, if prevContinuation is non-null then we may have
+      // already stolen the restyle tracker entry for this element while
+      // processing prevContinuation.  So we don't know whether aRestyleHint
+      // should really be 0 here or whether it should be eRestyle_Self.  Be
+      // pessimistic and force an actual reresolve in that situation.  The good
+      // news is that in the common case when prevContinuation is non-null we
+      // just used prevContinuationContext anyway and aren't reaching this code
+      // to start with.
       newContext =
         styleSet->ReparentStyleContext(oldContext, parentContext,
                                        ElementForStyleContext(aParentContent,
                                                               aFrame,
                                                               pseudoType));
     } else if (pseudoType == nsCSSPseudoElements::ePseudo_AnonBox) {
       newContext = styleSet->ResolveAnonymousBoxStyle(pseudoTag,
                                                       parentContext);
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/670467-1-ref.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<body style="color: green">
+  <div style="display: inline;">
+    Line 1
+    <div>Line 2</div>
+    Line 3
+    <div>Line 4</div>
+    Line 5
+  </div>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/670467-1.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<body style="color: red">
+  <div id="x" style="display: inline; visibility: hidden">
+    Line 1
+    <div>Line 2</div>
+    Line 3
+    <div>Line 4</div>
+    Line 5
+  </div><script type="text/javascript">
+    document.body.offsetWidth;
+    document.getElementById("x").style.visibility = "visible";
+    document.body.style.color = "green";
+  </script>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/670467-2-ref.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<style>
+  body::first-line { color: purple; }
+</style>
+<body style="width: 0; color: green"><span>This is some text</span>
+</body>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/670467-2.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<style>
+  body::first-line { color: purple; }
+</style>
+<body style="width: 0; color: red"><span id="x" style="visibility: hidden">This is some text</span>
+  <script type="text/javascript">
+    document.body.offsetWidth;
+    document.getElementById("x").style.visibility = "visible";
+    document.body.style.color = "green";
+  </script>
+</body>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1649,8 +1649,10 @@ fails-if(layersGPUAccelerated&&cocoaWidg
 == 652775-1.html 652775-1-ref.html
 == 655549-1.html 655549-1-ref.html
 == 658952.html 658952-ref.html
 == 664127-1.xul 664127-1-ref.xul
 == 660682-1.html 660682-1-ref.html
 != 669015-1.xul 669015-1-notref.xul
 == 668319-1.xul about:blank
 == 670442-1.html 670442-1-ref.html
+== 670467-1.html 670467-1-ref.html
+== 670467-2.html 670467-2-ref.html