Correct overflow propagation in BTT and RTL writing-modes

Source/core/rendering/RenderBox.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  *           (C) 2005, 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2013 Adobe Systems Incorporated. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 4190 matching lines @@
     // overflow if we are clipping our own overflow.
     if (child->hasSelfPaintingLayer() || hasOverflowClip())
         return;
     LayoutRect childVisualOverflowRect = child->visualOverflowRectForPropagation(style());
     childVisualOverflowRect.move(delta);
     addVisualOverflow(childVisualOverflowRect);
 }
 
 void RenderBox::addLayoutOverflow(const LayoutRect& rect)
 {
-    LayoutRect clientBox = clientBoxRect();
+    LayoutRect clientBox = noOverflowRect();
     if (clientBox.contains(rect) || rect.isEmpty())
         return;
     
     // For overflow clip objects, we don't want to propagate overflow into unreachable areas.
     LayoutRect overflowRect(rect);
     if (hasOverflowClip() || isRenderView()) {
         // Overflow is in the block's coordinate space and thus is flipped for horizontal-bt and vertical-rl 
         // writing modes.  At this stage that is actually a simplification, since we can treat horizontal-tb/bt as the same
         // and vertical-lr/rl as the same.
         bool hasTopOverflow = !style()->isLeftToRightDirection() && !isHorizontalWritingMode();
@@ skipping 34 matching lines @@
     m_overflow->addLayoutOverflow(overflowRect);
 }
 
 void RenderBox::addVisualOverflow(const LayoutRect& rect)
 {
     LayoutRect borderBox = borderBoxRect();
     if (borderBox.contains(rect) || rect.isEmpty())
         return;
         
     if (!m_overflow)
-        m_overflow = adoptPtr(new RenderOverflow(clientBoxRect(), borderBox));
+        m_overflow = adoptPtr(new RenderOverflow(noOverflowRect(), borderBox));
     
     m_overflow->addVisualOverflow(rect);
 }
 
 void RenderBox::clearLayoutOverflow()
 {
     if (!m_overflow)
         return;
-    
-    if (visualOverflowRect() == borderBoxRect()) {
+
+    LayoutRect noOverflowRect = this->noOverflowRect();
+    if (visualOverflowRect() == noOverflowRect) {
         m_overflow.clear();
         return;
     }
-    
-    m_overflow->setLayoutOverflow(borderBoxRect());
+
+    m_overflow->setLayoutOverflow(noOverflowRect);
 }
 
 inline static bool percentageLogicalHeightIsResolvable(const RenderBox* box)
 {
     return RenderBox::percentageLogicalHeightIsResolvableFromBlock(box->containingBlock(), box->isOutOfFlowPositioned());
 }
 
 bool RenderBox::percentageLogicalHeightIsResolvableFromBlock(const RenderBlock* containingBlock, bool isOutOfFlowPositioned)
 {
     // In quirks mode, blocks with auto height are skipped, and we keep looking for an enclosing
@@ skipping 158 matching lines @@
     // We are putting ourselves into our parent's coordinate space.  If there is a flipped block mismatch
     // in a particular axis, then we have to flip the rect along that axis.
     if (style()->writingMode() == RightToLeftWritingMode || parentStyle->writingMode() == RightToLeftWritingMode)
         rect.setX(width() - rect.maxX());
     else if (style()->writingMode() == BottomToTopWritingMode || parentStyle->writingMode() == BottomToTopWritingMode)
         rect.setY(height() - rect.maxY());
 
     return rect;
 }
 
+LayoutRect RenderBox::noOverflowRect() const

[Julien - ping for review, 2013/07/24 22:22:28]

Not totally satisfied with this name as it doesn't explicitly state which box we
return or which coordinate system it is in but I couldn't come with a better
one. Though the box is easy (client box), I don't think we have a name for this
coordinate system so it would be tricky to put it in the name.

Btw, this requires a FIXME in this function as we would really like to infer
this box from clientBoxRect.

[mstensho (USE GERRIT), 2013/07/25 12:36:39]

I'm not impressed by the name myself. :) But yeah, I couldn't come up with
anything that sucked less. This type of coordinate system certainly deserves a
name. It's widely used throughout the code, e.g. in RenderBox::m_frameRect.
Well... or from paddingBoxRect(). I don't understand why we use clientBoxRect()
and other methods marked with "More IE extensions" internally. But in order to
be able to use any of these, we probably have to fix
https://bugs.webkit.org/show_bug.cgi?id=76129 first.

I've added a FIXME.

+{
+    // Because of the special coodinate system used for overflow rectangles and many other
+    // rectangles (not quite logical, not quite physical), we need to flip the block progression
+    // coordinate in vertical-rl and horizontal-bt writing modes. In other words, the rectangle
+    // returned is physical, except for the block direction progression coordinate (y in horizontal
+    // writing modes, x in vertical writing modes), which is always "logical top". Apart from the
+    // flipping, this method does the same as clientBoxRect().
+
+    LayoutUnit left = borderLeft();
+    LayoutUnit top = borderTop();
+    LayoutUnit right = borderRight();
+    LayoutUnit bottom = borderBottom();
+    LayoutRect rect(left, top, width() - left - right, height() - top - bottom);
+    flipForWritingMode(rect);
+    // Subtract space occupied by scrollbars. Order is important here: first flip, then subtract
+    // scrollbars. This may seem backwards and weird, since one would think that a horizontal
+    // scrollbar at the physical bottom in horizontal-bt ought to be at the logical top (physical
+    // bottom), between the logical top (physical bottom) border and the logical top (physical
+    // bottom) padding. But this is how the rest of the code expects us to behave. This is highly
+    // related to https://bugs.webkit.org/show_bug.cgi?id=76129
+    rect.contract(verticalScrollbarWidth(), horizontalScrollbarHeight());
+    return rect;
+}
+
 LayoutRect RenderBox::overflowRectForPaintRejection() const
 {
     LayoutRect overflowRect = visualOverflowRect();
     if (!m_overflow || !usesCompositedScrolling())
         return overflowRect;
 
     overflowRect.unite(layoutOverflowRect());
     overflowRect.move(-scrolledContentOffset());
     return overflowRect;
 }
@@ skipping 158 matching lines @@
     }
 
     if (didSplitParentAnonymousBoxes)
         markBoxForRelayoutAfterSplit(this);
 
     ASSERT(beforeChild->parent() == this);
     return beforeChild;
 }
 
 } // namespace WebCore