Correct overflow propagation in BTT and RTL writing-modes
Overflow rectangles are not quite physical, not quite logical. This
means that we cannot use clientBoxRect() directly to represent a
rectangle that expresses exactly no overflow. This rectangle is the
padding box (relative to the border box) in vertical-lr and
horizontal-tb, but the block-direction borders need to be flipped in
vertical-rl and horizontal-bt.
fast/multicol/vertical-rl/rules-with-border-before.html now needs a
rebaseline, because it now renders differently, but correctly.
R=
BUG=236326
Review URL: https://chromiumcodereview.appspot.com/18720003
git-svn-id: svn://svn.chromium.org/blink/trunk@154945 bbb929c8-8fbe-4397-9dbb-9b2b20218538
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations
index 9591b6c..a936f92 100644
--- a/third_party/WebKit/LayoutTests/TestExpectations
+++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -507,6 +507,7 @@
webkit.org/b/110296 [ Linux MountainLion ] fast/multicol/newmulticol/column-rules-fixed-height.html [ ImageOnlyFailure ]
crbug.com/250935 [ Win ] fast/multicol/shrink-to-column-height-for-pagination.html [ ImageOnlyFailure Pass ]
+crbug.com/236326 fast/multicol/vertical-rl/rules-with-border-before.html [ NeedsRebaseline ]
# There seems to be some endemic MountainLion reftest failure.
# In theory, this shouldn't happen with reftests. Indicates a likely genuine
diff --git a/third_party/WebKit/LayoutTests/fast/css/overflow-btt-border-after-expected.txt b/third_party/WebKit/LayoutTests/fast/css/overflow-btt-border-after-expected.txt
new file mode 100644
index 0000000..24b6717
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/css/overflow-btt-border-after-expected.txt
@@ -0,0 +1,4 @@
+The word 'HEST' should be seen below, and there should be no red.
+
+HEST
+PASS
diff --git a/third_party/WebKit/LayoutTests/fast/css/overflow-btt-border-after.html b/third_party/WebKit/LayoutTests/fast/css/overflow-btt-border-after.html
new file mode 100644
index 0000000..c07c6256
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/css/overflow-btt-border-after.html
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>Border-after in horizontal-bt mode, non-visible overflow</title>
+ <style>
+ body { color:black; background:white; }
+ #container {
+ -webkit-writing-mode: horizontal-bt;
+ overflow:scroll;
+ height:100px;
+ border-top:150px solid white;
+ background:red;
+ }
+ #inner { height:200px; background:white; }
+ </style>
+ <script src="../../resources/check-layout.js"></script>
+ </head>
+ <body onload="checkLayout('#container'); inner.scrollIntoView();">
+ <!-- We have to set the scroll position, because of https://bugs.webkit.org/show_bug.cgi?id=76129
+ This is just to make it look good; checkLayout() doesn't care. -->
+ <p>The word 'HEST' should be seen below, and there should be no red.</p>
+ <div id="container" data-expected-scroll-height="200">
+ <div id="inner">HEST</div>
+ </div>
+ </body>
+</html>
diff --git a/third_party/WebKit/LayoutTests/fast/css/overflow-rtl-border-after-expected.txt b/third_party/WebKit/LayoutTests/fast/css/overflow-rtl-border-after-expected.txt
new file mode 100644
index 0000000..24b6717
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/css/overflow-rtl-border-after-expected.txt
@@ -0,0 +1,4 @@
+The word 'HEST' should be seen below, and there should be no red.
+
+HEST
+PASS
diff --git a/third_party/WebKit/LayoutTests/fast/css/overflow-rtl-border-after.html b/third_party/WebKit/LayoutTests/fast/css/overflow-rtl-border-after.html
new file mode 100644
index 0000000..7f2066e2
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/css/overflow-rtl-border-after.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>Border-after in vertical-rl mode, non-visible overflow</title>
+ <style>
+ body { color:black; background:white; }
+ #container {
+ -webkit-writing-mode: vertical-rl;
+ writing-mode: vertical-rl;
+ overflow:scroll;
+ width:100px;
+ border-left:150px solid white;
+ background:red;
+ }
+ #inner { width:200px; background:white; }
+ </style>
+ <script src="../../resources/check-layout.js"></script>
+ </head>
+ <body onload="checkLayout('#container'); container.scrollLeft = 1000;">
+ <!-- We have to set the scroll position, because of https://bugs.webkit.org/show_bug.cgi?id=76129
+ Furthermore, inner.scrollIntoView() doesn't work, so just set some insane left value.
+ This is just to make it look good; checkLayout() doesn't care. -->
+ <p>The word 'HEST' should be seen below, and there should be no red.</p>
+ <div id="container" data-expected-scroll-width="200">
+ <div id="inner">HEST</div>
+ </div>
+ </body>
+</html>
diff --git a/third_party/WebKit/Source/core/rendering/RenderBlock.cpp b/third_party/WebKit/Source/core/rendering/RenderBlock.cpp
index 117dad0..c7edc42 100644
--- a/third_party/WebKit/Source/core/rendering/RenderBlock.cpp
+++ b/third_party/WebKit/Source/core/rendering/RenderBlock.cpp
@@ -1784,7 +1784,7 @@
// When we have overflow clip, propagate the original spillout since it will include collapsed bottom margins
// and bottom padding. Set the axis we don't care about to be 1, since we want this overflow to always
// be considered reachable.
- LayoutRect clientRect(clientBoxRect());
+ LayoutRect clientRect(noOverflowRect());
LayoutRect rectToApply;
if (isHorizontalWritingMode())
rectToApply = LayoutRect(clientRect.x(), clientRect.y(), 1, max<LayoutUnit>(0, oldClientAfterEdge - clientRect.y()));
@@ -1798,7 +1798,7 @@
// Allow our overflow to catch cases where the caret in an empty editable element with negative text indent needs to get painted.
LayoutUnit textIndent = textIndentOffset();
if (textIndent < 0) {
- LayoutRect clientRect(clientBoxRect());
+ LayoutRect clientRect(noOverflowRect());
LayoutRect rectToApply = LayoutRect(clientRect.x() + min<LayoutUnit>(0, textIndent), clientRect.y(), clientRect.width() - min<LayoutUnit>(0, textIndent), clientRect.height());
addVisualOverflow(rectToApply);
}
diff --git a/third_party/WebKit/Source/core/rendering/RenderBox.cpp b/third_party/WebKit/Source/core/rendering/RenderBox.cpp
index 12360edf..5a7811c 100644
--- a/third_party/WebKit/Source/core/rendering/RenderBox.cpp
+++ b/third_party/WebKit/Source/core/rendering/RenderBox.cpp
@@ -4221,7 +4221,7 @@
void RenderBox::addLayoutOverflow(const LayoutRect& rect)
{
- LayoutRect clientBox = clientBoxRect();
+ LayoutRect clientBox = noOverflowRect();
if (clientBox.contains(rect) || rect.isEmpty())
return;
@@ -4276,7 +4276,7 @@
return;
if (!m_overflow)
- m_overflow = adoptPtr(new RenderOverflow(clientBoxRect(), borderBox));
+ m_overflow = adoptPtr(new RenderOverflow(noOverflowRect(), borderBox));
m_overflow->addVisualOverflow(rect);
}
@@ -4285,13 +4285,14 @@
{
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)
@@ -4470,6 +4471,34 @@
return rect;
}
+LayoutRect RenderBox::noOverflowRect() const
+{
+ // 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
+ // FIXME: when the above mentioned bug is fixed, it should hopefully be possible to call
+ // clientBoxRect() or paddingBoxRect() in this method, rather than fiddling with the edges on
+ // our own.
+ rect.contract(verticalScrollbarWidth(), horizontalScrollbarHeight());
+ return rect;
+}
+
LayoutRect RenderBox::overflowRectForPaintRejection() const
{
LayoutRect overflowRect = visualOverflowRect();
diff --git a/third_party/WebKit/Source/core/rendering/RenderBox.h b/third_party/WebKit/Source/core/rendering/RenderBox.h
index ad38279..ca05984 100644
--- a/third_party/WebKit/Source/core/rendering/RenderBox.h
+++ b/third_party/WebKit/Source/core/rendering/RenderBox.h
@@ -173,7 +173,8 @@
// For horizontal-tb and vertical-lr they will match physical directions, but for horizontal-bt and vertical-rl, the top/bottom and left/right
// respectively are flipped when compared to their physical counterparts. For example minX is on the left in vertical-lr,
// but it is on the right in vertical-rl.
- LayoutRect layoutOverflowRect() const { return m_overflow ? m_overflow->layoutOverflowRect() : clientBoxRect(); }
+ LayoutRect noOverflowRect() const;
+ LayoutRect layoutOverflowRect() const { return m_overflow ? m_overflow->layoutOverflowRect() : noOverflowRect(); }
IntRect pixelSnappedLayoutOverflowRect() const { return pixelSnappedIntRect(layoutOverflowRect()); }
LayoutSize maxLayoutOverflow() const { return LayoutSize(layoutOverflowRect().maxX(), layoutOverflowRect().maxY()); }
LayoutUnit logicalLeftLayoutOverflow() const { return style()->isHorizontalWritingMode() ? layoutOverflowRect().x() : layoutOverflowRect().y(); }
