Bug 609272. Make sure to not apply clipping of overflow when printing to the root element, because it doesn't really make use of its overflow style itself. r=dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 30 Nov 2010 13:19:57 -0500
changeset 58471 5f664ece27590bed20d50c8d7cb752b15ecf9567
parent 58470 80426f61cb135ab784c37b22a93dccd2f6ff9267
child 58472 43437276f9f174878ac212b6c03eb4efa831ac20
push id17312
push userbzbarsky@mozilla.com
push dateThu, 02 Dec 2010 12:19:10 +0000
treeherdermozilla-central@251cd89364a8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs609272
milestone2.0b8pre
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 609272. Make sure to not apply clipping of overflow when printing to the root element, because it doesn't really make use of its overflow style itself. r=dbaron
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/generic/nsBlockFrame.h
layout/generic/nsFrame.cpp
layout/generic/nsHTMLParts.h
layout/reftests/bugs/609272-1-ref.html
layout/reftests/bugs/609272-1.html
layout/reftests/bugs/reftest.list
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -2463,18 +2463,17 @@ nsCSSFrameConstructor::ConstructDocEleme
                           frameItems, &contentFrame);
       if (NS_FAILED(rv))
         return rv;
       if (!contentFrame || frameItems.IsEmpty())
         return NS_ERROR_FAILURE;
       *aNewFrame = frameItems.FirstChild();
       NS_ASSERTION(frameItems.OnlyChild(), "multiple root element frames");
     } else {
-      contentFrame = NS_NewBlockFrame(mPresShell, styleContext,
-        NS_BLOCK_FLOAT_MGR|NS_BLOCK_MARGIN_ROOT);
+      contentFrame = NS_NewBlockFormattingContext(mPresShell, styleContext);
       if (!contentFrame)
         return NS_ERROR_OUT_OF_MEMORY;
       nsFrameItems frameItems;
       // Use a null PendingBinding, since our binding is not in fact pending.
       rv = ConstructBlock(state, display, aDocElement,
                           state.GetGeometricParent(display,
                                                    mDocElementContainingBlock),
                           mDocElementContainingBlock, styleContext,
@@ -3704,16 +3703,21 @@ nsCSSFrameConstructor::ConstructFrameFro
   CHECK_ONLY_ONE_BIT(FCDATA_FUNC_IS_FULL_CTOR, FCDATA_IS_POPUP);
   CHECK_ONLY_ONE_BIT(FCDATA_FUNC_IS_FULL_CTOR, FCDATA_SKIP_ABSPOS_PUSH);
   CHECK_ONLY_ONE_BIT(FCDATA_FUNC_IS_FULL_CTOR, FCDATA_FORCE_VIEW);
   CHECK_ONLY_ONE_BIT(FCDATA_FUNC_IS_FULL_CTOR,
                      FCDATA_DISALLOW_GENERATED_CONTENT);
   CHECK_ONLY_ONE_BIT(FCDATA_FUNC_IS_FULL_CTOR, FCDATA_ALLOW_BLOCK_STYLES);
   CHECK_ONLY_ONE_BIT(FCDATA_MAY_NEED_SCROLLFRAME, FCDATA_FORCE_VIEW);
 #undef CHECK_ONLY_ONE_BIT
+  NS_ASSERTION(!(bits & FCDATA_FORCED_NON_SCROLLABLE_BLOCK) ||
+               ((bits & FCDATA_FUNC_IS_FULL_CTOR) &&
+                data->mFullConstructor ==
+                  &nsCSSFrameConstructor::ConstructNonScrollableBlock),
+               "Unexpected FCDATA_FORCED_NON_SCROLLABLE_BLOCK flag");
 
   // Don't create a subdocument frame for iframes if we're creating extra frames
   if (aState.mCreatingExtraFrames && aItem.mContent->IsHTML() &&
       aItem.mContent->Tag() == nsGkAtoms::iframe)
   {
     return NS_OK;
   }
 
@@ -4360,30 +4364,35 @@ nsCSSFrameConstructor::FindDisplayData(c
       PropagateScrollToViewport() == aContent;
   }
 
   NS_ASSERTION(!propagatedScrollToViewport ||
                !mPresShell->GetPresContext()->IsPaginated(),
                "Shouldn't propagate scroll in paginated contexts");
 
   // If the frame is a block-level frame and is scrollable, then wrap it in a
-  // scroll frame.  Except we don't want to do that for paginated contexts for
-  // frames that are block-outside and aren't frames for native anonymous stuff.
-  // The condition on skipping scrollframe construction in the
-  // paginated case needs to match code in ConstructNonScrollableBlock
-  // and in nsFrame::ApplyPaginatedOverflowClipping.
+  // scroll frame.
   // XXX Ignore tables for the time being
   // XXXbz it would be nice to combine this with the other block
   // case... Think about how do do this?
   if (aDisplay->IsBlockInside() &&
       aDisplay->IsScrollableOverflow() &&
-      !propagatedScrollToViewport &&
-      (!mPresShell->GetPresContext()->IsPaginated() ||
-       !aDisplay->IsBlockOutside() ||
-       aContent->IsInNativeAnonymousSubtree())) {
+      !propagatedScrollToViewport) {
+    // Except we don't want to do that for paginated contexts for
+    // frames that are block-outside and aren't frames for native
+    // anonymous stuff.
+    if (mPresShell->GetPresContext()->IsPaginated() &&
+        aDisplay->IsBlockOutside() &&
+        !aContent->IsInNativeAnonymousSubtree()) {
+      static const FrameConstructionData sForcedNonScrollableBlockData =
+        FULL_CTOR_FCDATA(FCDATA_FORCED_NON_SCROLLABLE_BLOCK,
+                         &nsCSSFrameConstructor::ConstructNonScrollableBlock);
+      return &sForcedNonScrollableBlockData;
+    }
+
     static const FrameConstructionData sScrollableBlockData =
       FULL_CTOR_FCDATA(0, &nsCSSFrameConstructor::ConstructScrollableBlock);
     return &sScrollableBlockData;
   }
 
   // Handle various non-scrollable blocks
   if (aDisplay->IsBlockInside() ||
       NS_STYLE_DISPLAY_RUN_IN == aDisplay->mDisplay ||
@@ -4499,32 +4508,30 @@ nsCSSFrameConstructor::ConstructNonScrol
                                                    FrameConstructionItem&   aItem,
                                                    nsIFrame*                aParentFrame,
                                                    const nsStyleDisplay*    aDisplay,
                                                    nsFrameItems&            aFrameItems,
                                                    nsIFrame**               aNewFrame)
 {
   nsStyleContext* const styleContext = aItem.mStyleContext;
 
+  // We want a block formatting context root in paginated contexts for
+  // every block that would be scrollable in a non-paginated context.
+  // We mark our blocks with a bit here if this condition is true, so
+  // we can check it later in nsFrame::ApplyPaginatedOverflowClipping.
+  PRBool clipPaginatedOverflow =
+    (aItem.mFCData->mBits & FCDATA_FORCED_NON_SCROLLABLE_BLOCK) != 0;
   if (aDisplay->IsAbsolutelyPositioned() ||
       aDisplay->IsFloating() ||
       NS_STYLE_DISPLAY_INLINE_BLOCK == aDisplay->mDisplay ||
-      // This check just needs to be the same as the check for using scrollable
-      // blocks in FindDisplayData and the check for clipping in
-      // nsFrame::ApplyPaginatedOverflowClipping; we want a block formatting
-      // context root in paginated contexts for every block that would be
-      // scrollable in a non-paginated context.  Note that IsPaginated()
-      // implies that no propagation to viewport has taken place, so we don't
-      // need to check for propagation here.
-      (mPresShell->GetPresContext()->IsPaginated() &&
-       aDisplay->IsBlockInside() &&
-       aDisplay->IsScrollableOverflow() &&
-       aDisplay->IsBlockOutside() &&
-       !aItem.mContent->IsInNativeAnonymousSubtree())) {
+      clipPaginatedOverflow) {
     *aNewFrame = NS_NewBlockFormattingContext(mPresShell, styleContext);
+    if (clipPaginatedOverflow) {
+      (*aNewFrame)->AddStateBits(NS_BLOCK_CLIP_PAGINATED_OVERFLOW);
+    }
   } else {
     *aNewFrame = NS_NewBlockFrame(mPresShell, styleContext);
   }
 
   return ConstructBlock(aState, aDisplay, aItem.mContent,
                         aState.GetGeometricParent(aDisplay, aParentFrame),
                         aParentFrame, styleContext, aNewFrame,
                         aFrameItems, aDisplay->IsPositioned(),
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -723,16 +723,20 @@ private:
   /* If FCDATA_ALLOW_BLOCK_STYLES is set, allow block styles when processing
      children.  This should not be used with FCDATA_FUNC_IS_FULL_CTOR. */
 #define FCDATA_ALLOW_BLOCK_STYLES 0x10000
   /* If FCDATA_USE_CHILD_ITEMS is set, then use the mChildItems in the relevant
      FrameConstructionItem instead of trying to process the content's children.
      This can be used with or without FCDATA_FUNC_IS_FULL_CTOR.
      The child items might still need table pseudo processing. */
 #define FCDATA_USE_CHILD_ITEMS 0x20000
+  /* If FCDATA_FORCED_NON_SCROLLABLE_BLOCK is set, then this block
+     would have been scrollable but has been forced to be
+     non-scrollable due to being in a paginated context. */
+#define FCDATA_FORCED_NON_SCROLLABLE_BLOCK 0x40000
 
   /* Structure representing information about how a frame should be
      constructed.  */
   struct FrameConstructionData {
     // Flag bits that can modify the way the construction happens
     PRUint32 mBits;
     // We have exactly one of three types of functions, so use a union for
     // better cache locality for the ones that aren't pointer-to-member.  That
--- a/layout/generic/nsBlockFrame.h
+++ b/layout/generic/nsBlockFrame.h
@@ -373,17 +373,17 @@ protected:
                                    nscoord& start,
                                    nscoord& width);
 
   void TryAllLines(nsLineList::iterator* aIterator,
                    nsLineList::iterator* aStartIterator,
                    nsLineList::iterator* aEndIterator,
                    PRBool* aInOverflowLines);
 
-  void SetFlags(PRUint32 aFlags) {
+  void SetFlags(nsFrameState aFlags) {
     mState &= ~NS_BLOCK_FLAGS_MASK;
     mState |= aFlags;
   }
 
   PRBool HaveOutsideBullet() const {
 #if defined(DEBUG) && !defined(DEBUG_rods)
     if(mState & NS_BLOCK_FRAME_HAS_OUTSIDE_BULLET) {
       NS_ASSERTION(mBullet,"NS_BLOCK_FRAME_HAS_OUTSIDE_BULLET flag set and no mBullet");
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -1226,30 +1226,23 @@ static inline PRBool ApplyOverflowHidden
   return type == nsGkAtoms::tableFrame ||
        type == nsGkAtoms::tableCellFrame ||
        type == nsGkAtoms::bcTableCellFrame;
 }
 
 static inline PRBool ApplyPaginatedOverflowClipping(nsIFrame* aFrame,
                                                     const nsStyleDisplay* aDisp)
 {
-  // These conditions on aDisp need to match the conditions for which in
-  // non-paginated contexts we'd create a scrollframe for a block but in a
-  // paginated context we don't.  See nsCSSFrameConstructor::FindDisplayData
-  // for the relevant conditions.  These conditions must also match those in
-  // nsCSSFrameConstructor::ConstructNonScrollableBlock for creating block
-  // formatting context roots for forced-to-be-no-longer scrollable blocks in
-  // paginated contexts.
+  // If we're paginated and aFrame is a block, and it has
+  // NS_BLOCK_CLIP_PAGINATED_OVERFLOW set, then we want to clip our
+  // overflow.
   return
     aFrame->PresContext()->IsPaginated() &&
-    aDisp->IsBlockInside() &&
-    aDisp->IsScrollableOverflow() &&
-    aDisp->IsBlockOutside() &&
     aFrame->GetType() == nsGkAtoms::blockFrame &&
-    !aFrame->GetContent()->IsInNativeAnonymousSubtree();
+    (aFrame->GetStateBits() & NS_BLOCK_CLIP_PAGINATED_OVERFLOW) != 0;
 }
 
 static PRBool ApplyOverflowClipping(nsDisplayListBuilder* aBuilder,
                                     nsIFrame* aFrame,
                                     const nsStyleDisplay* aDisp, nsRect* aRect) {
   // REVIEW: from nsContainerFrame.cpp SyncFrameViewGeometryDependentProperties,
   // except that that function used the border-edge for
   // -moz-hidden-unscrollable which I don't think is correct... Also I've
--- a/layout/generic/nsHTMLParts.h
+++ b/layout/generic/nsHTMLParts.h
@@ -57,33 +57,39 @@ class nsString;
 class nsIPresShell;
 class nsIChannel;
 class nsTableColFrame;
 
 /**
  * Additional frame-state bits used by nsBlockFrame
  * See the meanings at http://www.mozilla.org/newlayout/doc/block-and-line.html
  *
+ * NS_BLOCK_CLIP_PAGINATED_OVERFLOW is only set in paginated prescontexts, on
+ *  blocks which were forced to not have scrollframes but still need to clip
+ *  the display of their kids.
+ *
  * NS_BLOCK_HAS_FIRST_LETTER_STYLE means that the block has first-letter style,
  *  even if it has no actual first-letter frame among its descendants.
  *
  * NS_BLOCK_HAS_FIRST_LETTER_CHILD means that there is an inflow first-letter
  *  frame among the block's descendants. If there is a floating first-letter
  *  frame, or the block has first-letter style but has no first letter, this
  *  bit is not set. This bit is set on the first continuation only.
  */
 #define NS_BLOCK_MARGIN_ROOT              NS_FRAME_STATE_BIT(22)
 #define NS_BLOCK_FLOAT_MGR                NS_FRAME_STATE_BIT(23)
+#define NS_BLOCK_CLIP_PAGINATED_OVERFLOW  NS_FRAME_STATE_BIT(28)
 #define NS_BLOCK_HAS_FIRST_LETTER_STYLE   NS_FRAME_STATE_BIT(29)
 #define NS_BLOCK_FRAME_HAS_OUTSIDE_BULLET NS_FRAME_STATE_BIT(30)
 #define NS_BLOCK_HAS_FIRST_LETTER_CHILD   NS_FRAME_STATE_BIT(31)
 // These are the bits that get inherited from a block frame to its
 // next-in-flows and are not private to blocks
 #define NS_BLOCK_FLAGS_MASK               (NS_BLOCK_MARGIN_ROOT | \
                                            NS_BLOCK_FLOAT_MGR | \
+                                           NS_BLOCK_CLIP_PAGINATED_OVERFLOW | \
                                            NS_BLOCK_HAS_FIRST_LETTER_STYLE | \
                                            NS_BLOCK_FRAME_HAS_OUTSIDE_BULLET | \
                                            NS_BLOCK_HAS_FIRST_LETTER_CHILD)
 
 // Factory methods for creating html layout objects
 
 // Create a frame that supports "display: block" layout behavior
 nsIFrame*
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/609272-1-ref.html
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<html class="reftest-print">
+  This text should show up
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/609272-1.html
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<html class="reftest-print" style="height: 0; overflow: hidden">
+  This text should show up
+</html>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1536,16 +1536,17 @@ fails-if(!haveTestPlugin) == 599476.html
 == 600974-3.html 600974-1-ref.html
 == 602200-1.html 602200-1-ref.html
 == 602200-2.html 602200-2-ref.html
 == 602200-3.html 602200-3-ref.html
 == 602200-4.html 602200-4-ref.html
 == 604737.html 604737-ref.html
 == 605138-1.html 605138-1-ref.html
 == 605157-1.xhtml 605157-1-ref.xhtml
+== 609272-1.html 609272-1-ref.html
 == 608636-1.html 608636-1-ref.html
 == 613433-1.html 613433-1-ref.html
 == 613433-1.html 613433-2-ref.html
 == 613433-1.html 613433-3-ref.html
 == 613433-2.html 613433-1-ref.html
 == 613433-2.html 613433-2-ref.html
 == 613433-2.html 613433-3-ref.html
 == 613433-3.html 613433-1-ref.html