Bug 372037: CSS clip property should clip the overflow rect as well as the paint rect; r+sr=roc.

layout/base/nsCSSFrameConstructor.cpp

@@ -9598,24 +9598,19 @@ DoApplyRenderingChangeToTree(nsIFrame* aFrame,
 static void
 UpdateViewsForTree(nsIFrame* aFrame, nsIViewManager* aViewManager,
                    nsFrameManager* aFrameManager,
-                   nsRect& aBoundsRect, nsChangeHint aChange)
+                   nsChangeHint aChange)
 {
   NS_PRECONDITION(gInApplyRenderingChangeToTree,
                   "should only be called within ApplyRenderingChangeToTree");
 
   nsIView* view = aFrame->GetView();
   if (view) {
-    if (aChange & nsChangeHint_RepaintFrame) {
-      aViewManager->UpdateView(view, NS_VMREFRESH_NO_SYNC);
-    }
     if (aChange & nsChangeHint_SyncFrameView) {
       nsContainerFrame::SyncFrameViewProperties(aFrame->GetPresContext(),
                                                 aFrame, nsnull, view);
     }
   }
 
-  nsRect bounds = aFrame->GetOverflowRect();
-
   // now do children of frame
   PRInt32 listIndex = 0;
   nsIAtom* childList = nsnull;
@@ -9634,19 +9629,13 @@ UpdateViewsForTree(nsIFrame* aFrame, nsIViewManager* aViewManager,
                                        aFrameManager, aChange);
         }
         else {  // regular frame
-          nsRect  childBounds;
-          UpdateViewsForTree(child, aViewManager, aFrameManager, childBounds,
-                             aChange);
-          bounds.UnionRect(bounds, childBounds);
+          UpdateViewsForTree(child, aViewManager, aFrameManager, aChange);
         }
       }
       child = child->GetNextSibling();
     }
     childList = aFrame->GetAdditionalChildListName(listIndex++);
   } while (childList);
-
-  nsPoint parentOffset = aFrame->GetPosition();
-  aBoundsRect = bounds + parentOffset;
 }
 
 static void
@@ -9663,21 +9652,11 @@ DoApplyRenderingChangeToTree(nsIFrame* aFrame,
     // frame doesn't have a view, find the nearest containing view
     // (adjusting r's coordinate system to reflect the nesting) and
     // update there.
-    nsRect invalidRect;
-    UpdateViewsForTree(aFrame, aViewManager, aFrameManager, invalidRect,
-                       aChange);
+    UpdateViewsForTree(aFrame, aViewManager, aFrameManager, aChange);
 
     // if frame has view, will already be invalidated
-    if (!aFrame->HasView()
-        && (aChange & nsChangeHint_RepaintFrame)) {
-      if (aFrame->GetParent()) {
-        // Tell the parent to invalidate instead of the frame itself in case
-        // the child is clipping invalidates
-        aFrame->GetParent()->Invalidate(invalidRect);
-      } else {
-        invalidRect -= aFrame->GetPosition();
-        aFrame->Invalidate(invalidRect, PR_FALSE);
-      }
+    if (aChange & nsChangeHint_RepaintFrame) {
+      aFrame->Invalidate(aFrame->GetOverflowRect());
     }
   }
 }

layout/generic/nsAbsoluteContainingBlock.cpp

@@ -425,11 +425,14 @@ nsAbsoluteContainingBlock::ReflowAbsoluteFrame(nsIFrame*                aDelegat
                                              &kidDesiredSize.mOverflowArea);
 
   if (oldRect.TopLeft() != rect.TopLeft() || 
-      (aDelegatingFrame->GetStateBits() & NS_FRAME_FIRST_REFLOW)) {
+      (aDelegatingFrame->GetStateBits() & NS_FRAME_FIRST_REFLOW) ||
+      ((aKidFrame->GetStyleDisplay()->mClipFlags & NS_STYLE_CLIP_RECT) && 
+       (kidDesiredSize.mOverflowArea != oldOverflowRect))) {
     // The frame moved; we have to invalidate the whole frame
     // because the children may have moved after they were reflowed
-    // XXX This could be optimized in some cases, like repositioning and
-    // clipping
+    // We also have to invalidate when we're clipping and the overflow
+    // changes; the style code doesn't know how to deal with this case
+    // XXX This could be optimized in some cases, especially clipping changes
     aKidFrame->GetParent()->Invalidate(oldOverflowRect);
     aKidFrame->GetParent()->Invalidate(kidDesiredSize.mOverflowArea +
                                        rect.TopLeft());
@@ -447,18 +450,6 @@ nsAbsoluteContainingBlock::ReflowAbsoluteFrame(nsIFrame*                aDelegat
   }
   aKidFrame->DidReflow(aPresContext, &kidReflowState, NS_FRAME_REFLOW_FINISHED);
 
-  // If the frame has visible overflow, then store it as a property on the
-  // frame. This allows us to be able to recover it without having to reflow
-  // the frame
-  if (aKidFrame->GetStateBits() & NS_FRAME_OUTSIDE_CHILDREN) {
-    // Get the property (creating a rect struct if necessary)
-    nsRect* overflowArea = aKidFrame->GetOverflowAreaProperty(PR_TRUE);
-
-    NS_ASSERTION(overflowArea, "should have created rect");
-    if (overflowArea) {
-      *overflowArea = kidDesiredSize.mOverflowArea;
-    }
-  }
 #ifdef DEBUG
   if (nsBlockFrame::gNoisyReflow) {
     nsFrame::IndentBy(stdout,nsBlockFrame::gNoiseIndent - 1);

layout/generic/nsFrame.cpp

@@ -5341,11 +5341,11 @@ nsIFrame::FinishAndStoreOverflow(nsRect* aOverflowArea, nsSize aNewSize)
     aOverflowArea->x < 0 || aOverflowArea->y < 0 ||
     aOverflowArea->XMost() > aNewSize.width || aOverflowArea->YMost() > aNewSize.height;
   // Clear geometric overflow area if we clip our children
-  NS_ASSERTION((GetStyleDisplay()->mOverflowY == NS_STYLE_OVERFLOW_CLIP) ==
-               (GetStyleDisplay()->mOverflowX == NS_STYLE_OVERFLOW_CLIP),
+  NS_ASSERTION((disp->mOverflowY == NS_STYLE_OVERFLOW_CLIP) ==
+               (disp->mOverflowX == NS_STYLE_OVERFLOW_CLIP),
                "If one overflow is clip, the other should be too");
   if (geometricOverflow &&
-      GetStyleDisplay()->mOverflowX == NS_STYLE_OVERFLOW_CLIP) {
+      disp->mOverflowX == NS_STYLE_OVERFLOW_CLIP) {
     *aOverflowArea = nsRect(nsPoint(0, 0), aNewSize);
     geometricOverflow = PR_FALSE;
   }
@@ -5353,7 +5353,14 @@ nsIFrame::FinishAndStoreOverflow(nsRect* aOverflowArea, nsSize aNewSize)
   PRBool hasOutline;
   nsRect outlineRect(ComputeOutlineRect(this, &hasOutline, *aOverflowArea));
 
-  if (hasOutline || geometricOverflow) {
+  PRBool hasAbsPosClip;
+  nsRect absPosClipRect;
+  hasAbsPosClip = GetAbsPosClipRect(disp, &absPosClipRect);
+  if (hasAbsPosClip) {
+    outlineRect.IntersectRect(outlineRect, absPosClipRect);
+  }
+
+  if (outlineRect != nsRect(nsPoint(0, 0), aNewSize)) {
     // Throw out any overflow if we're -moz-hidden-unscrollable
     mState |= NS_FRAME_OUTSIDE_CHILDREN;
     nsRect* overflowArea = GetOverflowAreaProperty(PR_TRUE); 

layout/generic/nsIFrame.h

@@ -154,11 +154,8 @@ typedef PRUint32 nsFrameState;
 // continuation, e.g. a bidi continuation.
 #define NS_FRAME_IS_FLUID_CONTINUATION                0x00000004
 
-// If this bit is set, then there is a child frame in the frame that
-// extends outside this frame's bounding box. The implication is that
-// the frame's rect does not completely cover its children and
-// therefore operations like rendering and hit testing (for example)
-// must operate differently.
+// This bit is set when the frame's overflow rect is
+// different from its border rect (i.e. GetOverflowRect() != GetRect())
 #define NS_FRAME_OUTSIDE_CHILDREN                     0x00000008
 
 // If this bit is set, then a reference to the frame is being held
@@ -1516,10 +1513,10 @@ public:
                                   nsIFrame* aForChild, PRBool aImmediate);
 
   /**
-   * Computes a rect that includes this frame, all its descendant
-   * frames, this frame's outline (if any), and all descendant frames'
-   * outlines (if any). This is the union of everything that might be painted by
-   * this frame subtree.
+   * Computes a rect that encompasses everything that might be painted by
+   * this frame.  This includes this frame, all its descendent frames, this
+   * frame's outline, and descentant frames' outline, but does not include
+   * areas clipped out by the CSS "overflow" and "clip" properties.
    *
    * @return the rect relative to this frame's origin
    */

layout/reftests/Makefile.in

@@ -51,4 +51,4 @@ include $(topsrcdir)/config/rules.mk
 lcheck::
 	# XXX profiles?
 	# XXX do something with the result!
-	MOZ_NO_REMOTE=1 $(DIST)/bin/$(MOZ_APP_NAME) -reftest $(srcdir)/reftest.list
+	MOZ_NO_REMOTE=1 $(DIST)/bin/$(MOZ_APP_NAME) -P Debug -reftest file:///C:/mozilla/mozilla/layout/reftests/reftest.list

layout/reftests/bugs/372037-1-ref.html

@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+
+<style>
+html, body {margin: 0; padding:5px}
+</style>
+
+<div style="position: relative; height: 100px; width: 100px;">
+<div style="background: blue; position: absolute; top: 10px; width: 40px; height: 50px;">
+</div>
+</div>
+
+<div style="position: relative; height: 100px; width: 100px;">
+<div style="background: blue; position: fixed; left: 20px; width: 40px; height: 50px;">
+</div>
+</div>
+
+<div style="overflow: auto; position: relative; height: 100px; width: 100px;">
+<div style="background: blue; position: relative; width: 200px; height: 200px;">
+</div>
+</div>
+
+<div style="overflow: auto; position: relative; height: 100px; width: 100px;">
+<div style="background: blue; position: absolute; width: 200px; height: 200px;">
+</div>
+</div>

layout/reftests/bugs/372037-1.html

@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+
+<style>
+html, body {margin: 0; padding:5px}
+</style>
+
+<div style="overflow: auto; position: relative; height: 100px; width: 100px;">
+<div style="background: blue; position: absolute; clip: rect(10px, 40px, 60px, 0); width: 200px; height: 200px;">
+</div>
+</div>
+
+<div style="overflow: auto; position: relative; height: 100px; width: 100px;">
+<div style="background: blue; position: fixed; clip: rect(0 50px 50px 10px); width: 200px; height: 200px;">
+</div>
+</div>
+
+<div style="overflow: auto; position: relative; height: 100px; width: 100px;">
+<div style="background: blue; position: relative; clip: rect(0, 50px, 50px, 0); width: 200px; height: 200px;">
+</div>
+</div>
+
+<div style="overflow: auto; position: relative; height: 100px; width: 100px;">
+<div style="background: blue; position: absolute; clip: rect(0, auto, auto, 0); width: 200px; height: 200px;">
+</div>
+</div>

layout/reftests/bugs/reftest.list

@@ -200,3 +200,4 @@ fails == 368504-1.html 368504-1-ref.html # bug 368504
 == 371925-1b.html 371925-1-ref.html
 == 372553-1.html 372553-1-ref.html
 == 373381-1.html 373381-1-ref.html
+== 372037-1.html 372037-1-ref.html

layout/style/nsStyleStruct.cpp

@@ -1155,6 +1155,14 @@ nsChangeHint nsStyleDisplay::CalcDifference(const nsStyleDisplay& aOther) const
   if (mFloats != aOther.mFloats)
     NS_UpdateHint(hint, nsChangeHint_ReflowFrame);    
 
+  if (mClipFlags != aOther.mClipFlags || mClip != aOther.mClip) {
+    NS_UpdateHint(hint, nsChangeHint_ReflowFrame);
+    // The reflow code in naAbsoluteContainingBlock can deal with invalidation
+    // in all cases except changing from non-auto clip to auto clip; when
+    // changing to auto clip, the frame can't tell what happened
+    if (mClipFlags == NS_STYLE_CLIP_AUTO)
+      NS_UpdateHint(hint, nsChangeHint_RepaintFrame);
+  }
   // XXX the following is conservative, for now: changing float breaking shouldn't
   // necessarily require a repaint, reflow should suffice.
   if (mBreakType != aOther.mBreakType
@@ -1163,9 +1171,7 @@ nsChangeHint nsStyleDisplay::CalcDifference(const nsStyleDisplay& aOther) const
       || mAppearance != aOther.mAppearance)
     NS_UpdateHint(hint, NS_CombineHint(nsChangeHint_ReflowFrame, nsChangeHint_RepaintFrame));
 
-  if (mClipFlags != aOther.mClipFlags
-      || mClip != aOther.mClip
-      || mOpacity != aOther.mOpacity)
+  if (mOpacity != aOther.mOpacity)
     NS_UpdateHint(hint, nsChangeHint_RepaintFrame);
 
   return hint;