Fix style context invariants so we can re-resolve page break frames. (Bug 468645) r+sr=bzbarsky
authorL. David Baron <dbaron@dbaron.org>
Mon, 29 Dec 2008 10:07:36 -0500
changeset 23162 43391574f35f23bbe2885f09f20e22c53a8255be
parent 23161 1b8061be91458f4eb2c5530622dcd1c7cb86ae93
child 23163 b80da4a764fc8af170942436ee844d754465ffa8
push idunknown
push userunknown
push dateunknown
bugs468645
milestone1.9.2a1pre
Fix style context invariants so we can re-resolve page break frames. (Bug 468645) r+sr=bzbarsky
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsFrameManager.cpp
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -7290,16 +7290,18 @@ nsCSSFrameConstructor::ConstructSVGFrame
 
 // If page-break-before is set, this function constructs a page break frame,
 // EXCEPT for on these types of elements:
 //  * row groups, rows, cells (these are handled internally by tables)
 //  * fixed- and absolutely-positioned elements (currently, our positioning
 //    code doesn't expect positioned frames to have nsPageBreakFrame siblings)
 //
 // Returns true iff we should construct a page break frame after this element.
+// aStyleContext is the style context of the frame for which we're
+// constructing the page break
 PRBool
 nsCSSFrameConstructor::PageBreakBefore(nsFrameConstructorState& aState,
                                        nsIContent*              aContent,
                                        nsIFrame*                aParentFrame,
                                        nsStyleContext*          aStyleContext,
                                        nsFrameItems&            aFrameItems)
 {
   const nsStyleDisplay* display = aStyleContext->GetStyleDisplay();
@@ -7313,27 +7315,32 @@ nsCSSFrameConstructor::PageBreakBefore(n
       ConstructPageBreakFrame(aState, aContent, aParentFrame, aStyleContext,
                               aFrameItems);
     }
     return display->mBreakAfter;
   }
   return PR_FALSE;
 }
 
+// aStyleContext is the style context of the frame for which we're
+// constructing the page break
 nsresult
 nsCSSFrameConstructor::ConstructPageBreakFrame(nsFrameConstructorState& aState,
                                                nsIContent*              aContent,
                                                nsIFrame*                aParentFrame,
                                                nsStyleContext*          aStyleContext,
                                                nsFrameItems&            aFrameItems)
 {
   nsRefPtr<nsStyleContext> pseudoStyle;
+  // Use the same parent style context that |aStyleContext| has, since
+  // that's easier to re-resolve and it doesn't matter in practice.
+  // (Getting different parents can result in framechange hints, e.g.,
+  // for user-modify.)
   pseudoStyle = mPresShell->StyleSet()->ResolvePseudoStyleFor(nsnull,
-                                                              nsCSSAnonBoxes::pageBreak,
-                                                              aStyleContext);
+                   nsCSSAnonBoxes::pageBreak, aStyleContext->GetParent());
   nsIFrame* pageBreakFrame = NS_NewPageBreakFrame(mPresShell, pseudoStyle);
   if (pageBreakFrame) {
     InitAndRestoreFrame(aState, aContent, aParentFrame, nsnull, pageBreakFrame);
     aFrameItems.AddChild(pageBreakFrame);
 
     return NS_OK;
   }
   else {
@@ -7363,17 +7370,18 @@ nsCSSFrameConstructor::ConstructFrame(ns
     return rv;
 
   nsRefPtr<nsStyleContext> styleContext;
   styleContext = ResolveStyleContext(aParentFrame, aContent);
 
   PRBool pageBreakAfter = PR_FALSE;
 
   if (aState.mPresContext->IsPaginated()) {
-    // See if there is a page break before, if so construct one. Also see if there is one after
+    // Construct a page break frame for page-break-before, and remember if
+    // we need one for page-break-after.
     pageBreakAfter = PageBreakBefore(aState, aContent, aParentFrame,
                                      styleContext, aFrameItems);
   }
 
   // construct the frame
   rv = ConstructFrameInternal(aState, aContent, aParentFrame,
                               aContent->Tag(), aContent->GetNameSpaceID(),
                               styleContext, aFrameItems, PR_FALSE);
--- a/layout/base/nsFrameManager.cpp
+++ b/layout/base/nsFrameManager.cpp
@@ -1166,16 +1166,18 @@ nsFrameManager::ReResolveStyleContext(ns
     // XXXbz newContext should just be an nsRefPtr
     nsStyleContext* newContext = nsnull;
     if (pseudoTag == nsCSSAnonBoxes::mozNonElement) {
       NS_ASSERTION(localContent,
                    "non pseudo-element frame without content node");
       newContext = styleSet->ResolveStyleForNonElement(parentContext).get();
     }
     else if (pseudoTag) {
+      // XXXldb This choice of pseudoContent seems incorrect for anon
+      // boxes and perhaps other cases.
       nsIContent* pseudoContent =
           aParentContent ? aParentContent : localContent;
       if (pseudoTag == nsCSSPseudoElements::before ||
           pseudoTag == nsCSSPseudoElements::after) {
         // XXX what other pseudos do we need to treat like this?
         newContext = styleSet->ProbePseudoStyleFor(pseudoContent,
                                                    pseudoTag,
                                                    parentContext).get();
@@ -1189,17 +1191,19 @@ nsFrameManager::ReResolveStyleContext(ns
           newContext->AddRef();
         }
       } else {
         if (pseudoTag == nsCSSPseudoElements::firstLetter) {
           NS_ASSERTION(aFrame->GetType() == nsGkAtoms::letterFrame, 
                        "firstLetter pseudoTag without a nsFirstLetterFrame");
           nsBlockFrame* block = nsBlockFrame::GetNearestAncestorBlock(aFrame);
           pseudoContent = block->GetContent();
-        }       
+        } else if (pseudoTag == nsCSSAnonBoxes::pageBreak) {
+          pseudoContent = nsnull;
+        }
         newContext = styleSet->ResolvePseudoStyleFor(pseudoContent,
                                                      pseudoTag,
                                                      parentContext).get();
       }
     }
     else {
       NS_ASSERTION(localContent,
                    "non pseudo-element frame without content node");