- Revision
- 198560
- Author
- mmaxfi...@apple.com
- Date
- 2016-03-22 15:15:45 -0700 (Tue, 22 Mar 2016)
Log Message
[RTL Scrollbars] Position: absolute divs are covered by vertical scrollbar
https://bugs.webkit.org/show_bug.cgi?id=155533
Reviewed by Darin Adler.
Source/WebCore:
This patch changes the behavior of position: absolute elements when their
containing block has overflow: scroll in RTL scrollbar mode. Previously, we
were only adjusting the overflow calculation for such elements (but not
their position calculation). This patch updates the position calculation,
which automatically makes the overflow calculation work propertly, so the
old calculation is no longer necessary.
This patch also updates iframes to appropriately move their dirty rects
and their painting CTM by the scrollbar width when traversing frame
boundaries. This fixes all our existing RTL scrollbar RTL tests.
The RTL scrollbar tests are only marked as passing on certain OSes, so these
tests are transitioning from failing to passing in that other repository.
Test: fast/scrolling/rtl-scrollbars-positioning.html
fast/scrolling/rtl-scrollbars-overflow-elementFromPoint.html
fast/scrolling/rtl-scrollbars-overflow-position-absolute.html
fast/scrolling/rtl-scrollbars-iframe-offset.html
fast/scrolling/rtl-scrollbars-iframe-position-absolute.html
fast/scrolling/rtl-scrollbars-iframe-scrolled.html
fast/scrolling/rtl-scrollbars-iframe.html
* platform/ScrollView.cpp:
(WebCore::ScrollView::paint):
(WebCore::ScrollView::locationOfContents):
* platform/ScrollView.h:
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::repaintLayerDirtyRects):
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::addOverflowFromPositionedObjects):
* rendering/RenderBox.cpp:
(WebCore::RenderBox::computePositionedLogicalWidth):
* rendering/RenderView.cpp:
(WebCore::RenderView::repaintViewRectangle):
LayoutTests:
* TestExpectations:
* fast/scrolling/rtl-scrollbars-positioning-expected.html: Added.
* fast/scrolling/rtl-scrollbars-positioning.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (198559 => 198560)
--- trunk/LayoutTests/ChangeLog 2016-03-22 22:14:38 UTC (rev 198559)
+++ trunk/LayoutTests/ChangeLog 2016-03-22 22:15:45 UTC (rev 198560)
@@ -1,3 +1,14 @@
+2016-03-22 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ [RTL Scrollbars] Position: absolute divs are covered by vertical scrollbar
+ https://bugs.webkit.org/show_bug.cgi?id=155533
+
+ Reviewed by Darin Adler.
+
+ * TestExpectations:
+ * fast/scrolling/rtl-scrollbars-positioning-expected.html: Added.
+ * fast/scrolling/rtl-scrollbars-positioning.html: Added.
+
2016-03-22 Ryan Haddad <ryanhad...@apple.com>
Marking inspector/console/console-api.html as flaky on Mac
Modified: trunk/LayoutTests/TestExpectations (198559 => 198560)
--- trunk/LayoutTests/TestExpectations 2016-03-22 22:14:38 UTC (rev 198559)
+++ trunk/LayoutTests/TestExpectations 2016-03-22 22:15:45 UTC (rev 198560)
@@ -1013,3 +1013,4 @@
fast/scrolling/rtl-scrollbars-iframe-offset.html [ ImageOnlyFailure ]
fast/scrolling/rtl-scrollbars-elementFromPoint-static.html [ Failure ]
fast/scrolling/rtl-scrollbars-iframe-scrolled.html [ ImageOnlyFailure ]
+fast/scrolling/rtl-scrollbars-positioning.html [ ImageOnlyFailure ]
Added: trunk/LayoutTests/fast/scrolling/rtl-scrollbars-positioning-expected.html (0 => 198560)
--- trunk/LayoutTests/fast/scrolling/rtl-scrollbars-positioning-expected.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/rtl-scrollbars-positioning-expected.html 2016-03-22 22:15:45 UTC (rev 198560)
@@ -0,0 +1,21 @@
+<!DOCTYPE html><!-- webkit-test-runner [ rtlScrollbars=true ] -->
+<html>
+<head>
+<style>
+div {
+ font: 20px Ahem;
+}
+</style>
+</head>
+<body style="margin: 0px;">
+<div style="position: absolute; left: 40px; top: 8px; width: 600px; font: 16px Times;">This test makes sure that static and absolutely positioned divs work correctly with main frame scrolling, overflow: scroll, and iframes. The test passes if you see 7 equally sized and equally spaced black boxes (20px x 20px) below, stacked vertically, with 10px of vertical space between them</div>
+<div style="position: absolute; left: 0px; top: 0px;">m</div>
+<div style="position: absolute; left: 0px; top: 30px;">m</div>
+<div style="position: absolute; left: 0px; top: 60px;">m</div>
+<div style="position: absolute; left: 0px; top: 90px;">m</div>
+<div style="position: absolute; left: 0px; top: 120px;">m</div>
+<div style="position: absolute; left: 0px; top: 150px;">m</div>
+<div style="position: absolute; left: 0px; top: 180px;">m</div>
+<div style="position: absolute; left: 0px; top: 0px; width: 1px; height: 2000px;"></div>
+</body>
+</html>
\ No newline at end of file
Added: trunk/LayoutTests/fast/scrolling/rtl-scrollbars-positioning.html (0 => 198560)
--- trunk/LayoutTests/fast/scrolling/rtl-scrollbars-positioning.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/rtl-scrollbars-positioning.html 2016-03-22 22:15:45 UTC (rev 198560)
@@ -0,0 +1,37 @@
+<!DOCTYPE html><!-- webkit-test-runner [ rtlScrollbars=true ] -->
+<html>
+<head>
+<style>
+div {
+ font: 20px Ahem;
+}
+</style>
+</head>
+<body style="margin: 0px;">
+<div style="position: absolute; left: 40px; top: 8px; width: 600px; font: 16px Times;">This test makes sure that static and absolutely positioned divs work correctly with main frame scrolling, overflow: scroll, and iframes. The test passes if you see 7 equally sized and equally spaced black boxes (20px x 20px) below, stacked vertically, with 10px of vertical space between them</div>
+<div>m</div>
+<div style="position: absolute; left: 0px; top: 30px;">m</div>
+<div style="position: fixed; left: 0px; top: 60px;">m</div>
+<div style="position: absolute; left: -15px; top: 90px; width: 200px; height: 60px; overflow-y: scroll">
+ <div>m</div>
+ <div style="position: absolute; left: 0px; top: 30px;">m</div>
+ <div style="position: absolute; left: 0px; top: 0px; width: 1px; height: 2000px;"></div>
+</div>
+<iframe style="position: absolute; left: -15px; top: 150px; width: 200px; height: 100px; border: 0px solid black;" srcdoc="<!DOCTYPE html>
+<html>
+<head>
+<style>
+div {
+ font: 20px Ahem;
+}
+</style>
+</head>
+<body style='margin: 0px;'>
+<div>m</div>
+<div style='position: absolute; left: 0px; top: 30px;'>m</div>
+<div style='position: absolute; left: 0px; top: 0px; width: 1px; height: 2000px;'></div>
+</body>
+</html>"></iframe>
+<div style="position: absolute; left: 0px; top: 0px; width: 1px; height: 2000px;"></div>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (198559 => 198560)
--- trunk/Source/WebCore/ChangeLog 2016-03-22 22:14:38 UTC (rev 198559)
+++ trunk/Source/WebCore/ChangeLog 2016-03-22 22:15:45 UTC (rev 198560)
@@ -1,3 +1,45 @@
+2016-03-22 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ [RTL Scrollbars] Position: absolute divs are covered by vertical scrollbar
+ https://bugs.webkit.org/show_bug.cgi?id=155533
+
+ Reviewed by Darin Adler.
+
+ This patch changes the behavior of position: absolute elements when their
+ containing block has overflow: scroll in RTL scrollbar mode. Previously, we
+ were only adjusting the overflow calculation for such elements (but not
+ their position calculation). This patch updates the position calculation,
+ which automatically makes the overflow calculation work propertly, so the
+ old calculation is no longer necessary.
+
+ This patch also updates iframes to appropriately move their dirty rects
+ and their painting CTM by the scrollbar width when traversing frame
+ boundaries. This fixes all our existing RTL scrollbar RTL tests.
+
+ The RTL scrollbar tests are only marked as passing on certain OSes, so these
+ tests are transitioning from failing to passing in that other repository.
+
+ Test: fast/scrolling/rtl-scrollbars-positioning.html
+ fast/scrolling/rtl-scrollbars-overflow-elementFromPoint.html
+ fast/scrolling/rtl-scrollbars-overflow-position-absolute.html
+ fast/scrolling/rtl-scrollbars-iframe-offset.html
+ fast/scrolling/rtl-scrollbars-iframe-position-absolute.html
+ fast/scrolling/rtl-scrollbars-iframe-scrolled.html
+ fast/scrolling/rtl-scrollbars-iframe.html
+
+ * platform/ScrollView.cpp:
+ (WebCore::ScrollView::paint):
+ (WebCore::ScrollView::locationOfContents):
+ * platform/ScrollView.h:
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::GraphicsLayerCA::repaintLayerDirtyRects):
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::addOverflowFromPositionedObjects):
+ * rendering/RenderBox.cpp:
+ (WebCore::RenderBox::computePositionedLogicalWidth):
+ * rendering/RenderView.cpp:
+ (WebCore::RenderView::repaintViewRectangle):
+
2016-03-22 Antti Koivisto <an...@apple.com>
Non-const DocumentRuleSets::features() does not check default style version
Modified: trunk/Source/WebCore/platform/ScrollView.cpp (198559 => 198560)
--- trunk/Source/WebCore/platform/ScrollView.cpp 2016-03-22 22:14:38 UTC (rev 198559)
+++ trunk/Source/WebCore/platform/ScrollView.cpp 2016-03-22 22:15:45 UTC (rev 198560)
@@ -1222,15 +1222,16 @@
IntRect documentDirtyRect = rect;
if (!paintsEntireContents()) {
- IntRect visibleAreaWithoutScrollbars(location(), visibleContentRect(LegacyIOSDocumentVisibleRect).size());
+ IntRect visibleAreaWithoutScrollbars(locationOfContents(), visibleContentRect(LegacyIOSDocumentVisibleRect).size());
documentDirtyRect.intersect(visibleAreaWithoutScrollbars);
}
if (!documentDirtyRect.isEmpty()) {
GraphicsContextStateSaver stateSaver(context);
- context.translate(x(), y());
- documentDirtyRect.moveBy(-location());
+ IntPoint locationOfContents = this->locationOfContents();
+ context.translate(locationOfContents.x(), locationOfContents.y());
+ documentDirtyRect.moveBy(-locationOfContents);
if (!paintsEntireContents()) {
context.translate(-scrollX(), -scrollY());
@@ -1498,6 +1499,14 @@
m_verticalScrollbar->styleChanged();
}
+IntPoint ScrollView::locationOfContents() const
+{
+ IntPoint result = location();
+ if (verticalScrollbarIsOnLeft() && m_verticalScrollbar)
+ result.move(m_verticalScrollbar->occupiedWidth(), 0);
+ return result;
+}
+
#if !PLATFORM(COCOA)
void ScrollView::platformAddChild(Widget*)
Modified: trunk/Source/WebCore/platform/ScrollView.h (198559 => 198560)
--- trunk/Source/WebCore/platform/ScrollView.h 2016-03-22 22:14:38 UTC (rev 198559)
+++ trunk/Source/WebCore/platform/ScrollView.h 2016-03-22 22:15:45 UTC (rev 198560)
@@ -73,6 +73,8 @@
virtual void notifyPageThatContentAreaWillPaint() const;
+ IntPoint locationOfContents() const;
+
// NOTE: This should only be called by the overriden setScrollOffset from ScrollableArea.
virtual void scrollTo(const ScrollPosition&);
Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (198559 => 198560)
--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2016-03-22 22:14:38 UTC (rev 198559)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2016-03-22 22:15:45 UTC (rev 198560)
@@ -2675,11 +2675,8 @@
return;
}
- if (!m_dirtyRects.size())
- return;
-
- for (size_t i = 0; i < m_dirtyRects.size(); ++i)
- m_layer->setNeedsDisplayInRect(m_dirtyRects[i]);
+ for (auto& dirtyRect : m_dirtyRects)
+ m_layer->setNeedsDisplayInRect(dirtyRect);
m_dirtyRects.clear();
}
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (198559 => 198560)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2016-03-22 22:14:38 UTC (rev 198559)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2016-03-22 22:15:45 UTC (rev 198560)
@@ -1063,12 +1063,8 @@
RenderBox* positionedObject = *it;
// Fixed positioned elements don't contribute to layout overflow, since they don't scroll with the content.
- if (positionedObject->style().position() != FixedPosition) {
- LayoutUnit x = positionedObject->x();
- if (style().shouldPlaceBlockDirectionScrollbarOnLeft())
- x += (style().isLeftToRightDirection() ? 1 : -1) * verticalScrollbarWidth();
- addOverflowFromChild(positionedObject, LayoutSize(x, positionedObject->y()));
- }
+ if (positionedObject->style().position() != FixedPosition)
+ addOverflowFromChild(positionedObject, { positionedObject->x(), positionedObject->y() });
}
}
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (198559 => 198560)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2016-03-22 22:14:38 UTC (rev 198559)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2016-03-22 22:15:45 UTC (rev 198560)
@@ -3522,6 +3522,11 @@
#endif
computedValues.m_extent += bordersPlusPadding;
+ if (is<RenderBox>(*containerBlock)) {
+ auto& containingBox = downcast<RenderBox>(*containerBlock);
+ if (containingBox.layer() && containingBox.layer()->verticalScrollbarIsOnLeft())
+ computedValues.m_position += containingBox.verticalScrollbarWidth();
+ }
// Adjust logicalLeft if we need to for the flipped version of our writing mode in regions.
// FIXME: Add support for other types of objects as containerBlock, not only RenderBlock.
Modified: trunk/Source/WebCore/rendering/RenderView.cpp (198559 => 198560)
--- trunk/Source/WebCore/rendering/RenderView.cpp 2016-03-22 22:14:38 UTC (rev 198559)
+++ trunk/Source/WebCore/rendering/RenderView.cpp 2016-03-22 22:15:45 UTC (rev 198560)
@@ -635,6 +635,16 @@
#endif
adjustedRect.moveBy(-viewRect.location());
adjustedRect.moveBy(ownerBox->contentBoxRect().location());
+
+ // A dirty rect in an iframe is relative to the contents of that iframe.
+ // When we traverse between parent frames and child frames, we need to make sure
+ // that the coordinate system is mapped appropriately between the iframe's contents
+ // and the Renderer that contains the iframe. This transformation must account for a
+ // left scrollbar (if one exists).
+ FrameView& frameView = this->frameView();
+ if (frameView.verticalScrollbarIsOnLeft() && frameView.verticalScrollbar())
+ adjustedRect.move(LayoutSize(frameView.verticalScrollbar()->occupiedWidth(), 0));
+
ownerBox->repaintRectangle(adjustedRect);
return;
}
