- Revision
- 152303
- Author
- [email protected]
- Date
- 2013-07-02 11:14:47 -0700 (Tue, 02 Jul 2013)
Log Message
constrainScrollPositionForOverhang needs to handle scrollOrigin correctly
https://bugs.webkit.org/show_bug.cgi?id=118176
<rdar://problem/14301271>
Reviewed by Anders Carlsson.
Test: compositing/geometry/fixed-position-flipped-writing-mode.html
WebCore makes use of constrainScrollPositionForOverhang not only for
constraining fixed- and sticky-positioned elements to the viewport, but
also for clamping the tile cache's visible rect.
Therefore, constrainScrollPositionForOverhang needs to correctly take
the scrollOrigin into account. The easiest way I saw to do this was to
reimplement the function in terms of a pair of rect intersections
between a virtual scrollable "viewport" and the document (with a bit
of complication from headers and footers).
The first intersection is performed, then if the viewport doesn't fit,
it is pushed down and to the right, from the origin. Next, we intersect
again, this time pushing the rect up by the amount it overflowed the document
rect from the bottom right. This performs effectively the same constraint
as previously, but handles the scrollOrigin correctly and is also somewhat
easier to read and understand (with pictures).
* page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:
(WebCore::ScrollingTreeScrollingNodeMac::setScrollLayerPosition):
Subtract the scrollOrigin out of the offset passed to scrollOffsetForFixedPosition,
it's expecting an offset without the origin included.
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::constrainScrollPositionForOverhang):
Reimplement the function as described above.
Add a test that ensures that fixed position works with a scrollable document
and direction: rtl, which is one case where scrollOrigin is set.
Disable the test on Mac WebKit1 because of https://bugs.webkit.org/show_bug.cgi?id=118269.
* compositing/geometry/fixed-position-flipped-writing-mode-expected.txt: Added.
* compositing/geometry/fixed-position-flipped-writing-mode.html: Added.
* platform/mac/compositing/geometry/fixed-position-flipped-writing-mode-expected.png: Added.
* platform/mac/TestExpectations:
* platform/mac-wk2/TestExpectations:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (152302 => 152303)
--- trunk/LayoutTests/ChangeLog 2013-07-02 18:00:22 UTC (rev 152302)
+++ trunk/LayoutTests/ChangeLog 2013-07-02 18:14:47 UTC (rev 152303)
@@ -1,3 +1,22 @@
+2013-07-02 Tim Horton <[email protected]>
+
+ constrainScrollPositionForOverhang needs to handle scrollOrigin correctly
+ https://bugs.webkit.org/show_bug.cgi?id=118176
+ <rdar://problem/14301271>
+
+ Reviewed by Anders Carlsson.
+
+ Add a test that ensures that fixed position works with a scrollable document
+ and direction: rtl, which is one case where scrollOrigin is set.
+
+ Disable the test on Mac WebKit1 because of https://bugs.webkit.org/show_bug.cgi?id=118269.
+
+ * compositing/geometry/fixed-position-flipped-writing-mode-expected.txt: Added.
+ * compositing/geometry/fixed-position-flipped-writing-mode.html: Added.
+ * platform/mac/compositing/geometry/fixed-position-flipped-writing-mode-expected.png: Added.
+ * platform/mac/TestExpectations:
+ * platform/mac-wk2/TestExpectations:
+
2013-07-02 Mario Sanchez Prada <[email protected]>
Unreviewed gardening. Added new common expectations for test after r151665.
Added: trunk/LayoutTests/compositing/geometry/fixed-position-flipped-writing-mode-expected.txt (0 => 152303)
--- trunk/LayoutTests/compositing/geometry/fixed-position-flipped-writing-mode-expected.txt (rev 0)
+++ trunk/LayoutTests/compositing/geometry/fixed-position-flipped-writing-mode-expected.txt 2013-07-02 18:14:47 UTC (rev 152303)
@@ -0,0 +1,25 @@
+(GraphicsLayer
+ (position -4208.00 0.00)
+ (bounds 5008.00 585.00)
+ (children 1
+ (GraphicsLayer
+ (bounds 5008.00 585.00)
+ (contentsOpaque 1)
+ (children 2
+ (GraphicsLayer
+ (position 4308.00 0.00)
+ (bounds 200.00 585.00)
+ (contentsOpaque 1)
+ )
+ (GraphicsLayer
+ (position 0.00 13.00)
+ (bounds 5000.00 15.00)
+ (opacity 0.00)
+ (usingTiledLayer 1)
+ (drawsContent 1)
+ )
+ )
+ )
+ )
+)
+
Added: trunk/LayoutTests/compositing/geometry/fixed-position-flipped-writing-mode.html (0 => 152303)
--- trunk/LayoutTests/compositing/geometry/fixed-position-flipped-writing-mode.html (rev 0)
+++ trunk/LayoutTests/compositing/geometry/fixed-position-flipped-writing-mode.html 2013-07-02 18:14:47 UTC (rev 152303)
@@ -0,0 +1,57 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+ <title>It should be possible to cover the red box with the green box by scrolling.</title>
+ <style>
+ html {
+ direction: rtl;
+ }
+
+ body {
+ width: 5000px;
+ }
+
+ .box {
+ width: 200px;
+ height: 100%;
+ top: 0px;
+ }
+
+ #background {
+ position: absolute;
+ background-color: red;
+ right: 500px;
+ }
+
+ #cover {
+ position: fixed;
+ background-color: green;
+ left: 50%;
+ }
+
+ #layers {
+ opacity: 0;
+ }
+ </style>
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText(true);
+
+ function runTest()
+ {
+ window.scrollTo(-300, 0);
+ if (window.testRunner)
+ document.getElementById('layers').innerText = window.internals.layerTreeAsText(document);
+ }
+
+ window.addEventListener('load', runTest, false);
+ </script>
+</head>
+<body>
+ <div id="background" class="box"></div>
+ <div id="cover" class="box"></div>
+
+ <pre id="layers">Layer tree goes here.</pre>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/mac/TestExpectations (152302 => 152303)
--- trunk/LayoutTests/platform/mac/TestExpectations 2013-07-02 18:00:22 UTC (rev 152302)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2013-07-02 18:14:47 UTC (rev 152303)
@@ -1336,3 +1336,6 @@
webkit.org/b/116582 [ Debug ] inspector/styles/import-pseudoclass-crash.html [ Pass Crash ]
webkit.org/b/116592 inspector/geolocation-error.html [ Pass Failure ]
+
+# Asserts in WebKit1-debug due to a preexisting issue with overflow rect computation
+webkit.org/b/118269 compositing/geometry/fixed-position-flipped-writing-mode.html
Added: trunk/LayoutTests/platform/mac/compositing/geometry/fixed-position-flipped-writing-mode-expected.png
(Binary files differ)
Added: svn:mime-type
Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (152302 => 152303)
--- trunk/LayoutTests/platform/mac-wk2/TestExpectations 2013-07-02 18:00:22 UTC (rev 152302)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations 2013-07-02 18:14:47 UTC (rev 152303)
@@ -398,6 +398,9 @@
webaudio/mediastreamaudiosourcenode.html
webaudio/codec-tests/vorbis/
+# Asserts in WebKit1-debug only, so reenabling.
+compositing/geometry/fixed-position-flipped-writing-mode.html [ Pass ]
+
### END OF (5) Features that are not supported in WebKit1, so skipped in mac/TestExpectations then re-enabled here
########################################
Modified: trunk/Source/WebCore/ChangeLog (152302 => 152303)
--- trunk/Source/WebCore/ChangeLog 2013-07-02 18:00:22 UTC (rev 152302)
+++ trunk/Source/WebCore/ChangeLog 2013-07-02 18:14:47 UTC (rev 152303)
@@ -1,3 +1,39 @@
+2013-07-02 Tim Horton <[email protected]>
+
+ constrainScrollPositionForOverhang needs to handle scrollOrigin correctly
+ https://bugs.webkit.org/show_bug.cgi?id=118176
+ <rdar://problem/14301271>
+
+ Reviewed by Anders Carlsson.
+
+ Test: compositing/geometry/fixed-position-flipped-writing-mode.html
+
+ WebCore makes use of constrainScrollPositionForOverhang not only for
+ constraining fixed- and sticky-positioned elements to the viewport, but
+ also for clamping the tile cache's visible rect.
+
+ Therefore, constrainScrollPositionForOverhang needs to correctly take
+ the scrollOrigin into account. The easiest way I saw to do this was to
+ reimplement the function in terms of a pair of rect intersections
+ between a virtual scrollable "viewport" and the document (with a bit
+ of complication from headers and footers).
+
+ The first intersection is performed, then if the viewport doesn't fit,
+ it is pushed down and to the right, from the origin. Next, we intersect
+ again, this time pushing the rect up by the amount it overflowed the document
+ rect from the bottom right. This performs effectively the same constraint
+ as previously, but handles the scrollOrigin correctly and is also somewhat
+ easier to read and understand (with pictures).
+
+ * page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:
+ (WebCore::ScrollingTreeScrollingNodeMac::setScrollLayerPosition):
+ Subtract the scrollOrigin out of the offset passed to scrollOffsetForFixedPosition,
+ it's expecting an offset without the origin included.
+
+ * platform/ScrollableArea.cpp:
+ (WebCore::ScrollableArea::constrainScrollPositionForOverhang):
+ Reimplement the function as described above.
+
2013-07-02 Christophe Dumez <[email protected]>
Remove SVGStyledLocatableElement class
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm (152302 => 152303)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm 2013-07-02 18:00:22 UTC (rev 152302)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm 2013-07-02 18:14:47 UTC (rev 152303)
@@ -311,7 +311,8 @@
ASSERT(!shouldUpdateScrollLayerPositionOnMainThread());
m_scrollLayer.get().position = CGPointMake(-position.x() + scrollOrigin().x(), -position.y() + scrollOrigin().y());
- IntSize scrollOffsetForFixedChildren = FrameView::scrollOffsetForFixedPosition(viewportRect(), totalContentsSize(), position, scrollOrigin(), frameScaleFactor(), false, headerHeight(), footerHeight());
+ IntPoint scrollOffset = position - toIntSize(scrollOrigin());
+ IntSize scrollOffsetForFixedChildren = FrameView::scrollOffsetForFixedPosition(viewportRect(), totalContentsSize(), scrollOffset, scrollOrigin(), frameScaleFactor(), false, headerHeight(), footerHeight());
if (m_counterScrollingLayer)
m_counterScrollingLayer.get().position = FloatPoint(scrollOffsetForFixedChildren);
@@ -320,7 +321,7 @@
// then we should recompute scrollOffsetForFixedChildren for the banner with a scale factor of 1.
float horizontalScrollOffsetForBanner = scrollOffsetForFixedChildren.width();
if (frameScaleFactor() != 1)
- horizontalScrollOffsetForBanner = FrameView::scrollOffsetForFixedPosition(viewportRect(), totalContentsSize(), position, scrollOrigin(), 1, false, headerHeight(), footerHeight()).width();
+ horizontalScrollOffsetForBanner = FrameView::scrollOffsetForFixedPosition(viewportRect(), totalContentsSize(), scrollOffset, scrollOrigin(), 1, false, headerHeight(), footerHeight()).width();
if (m_headerLayer)
m_headerLayer.get().position = FloatPoint(horizontalScrollOffsetForBanner, 0);
Modified: trunk/Source/WebCore/platform/ScrollableArea.cpp (152302 => 152303)
--- trunk/Source/WebCore/platform/ScrollableArea.cpp 2013-07-02 18:00:22 UTC (rev 152302)
+++ trunk/Source/WebCore/platform/ScrollableArea.cpp 2013-07-02 18:14:47 UTC (rev 152303)
@@ -412,38 +412,32 @@
std::max(0, visibleHeight() + horizontalScrollbarHeight));
}
-static int constrainedScrollPosition(int visibleContentSize, int totalContentsSize, int scrollPosition, int scrollOrigin, int headerHeight, int footerHeight)
+IntPoint ScrollableArea::constrainScrollPositionForOverhang(const IntRect& visibleContentRect, const IntSize& totalContentsSize, const IntPoint& scrollPosition, const IntPoint& scrollOrigin, int headerHeight, int footerHeight)
{
- int maxValue = totalContentsSize - visibleContentSize - footerHeight;
- if (maxValue <= 0)
- return 0;
+ // The viewport rect that we're scrolling shouldn't be larger than our document.
+ IntSize idealScrollRectSize(std::min(visibleContentRect.width(), totalContentsSize.width()), std::min(visibleContentRect.height(), totalContentsSize.height()));
+
+ IntRect scrollRect(scrollPosition + scrollOrigin - IntSize(0, headerHeight), idealScrollRectSize);
+ IntRect documentRect(IntPoint(), IntSize(totalContentsSize.width(), totalContentsSize.height() - headerHeight - footerHeight));
- if (!scrollOrigin) {
- if (scrollPosition <= headerHeight)
- return 0;
- if (scrollPosition > maxValue)
- scrollPosition = maxValue - headerHeight;
- else
- scrollPosition -= headerHeight;
- } else {
- // FIXME: position:fixed elements are currently broken when there is a non-zero y-value in the scroll origin
- // such as when -webkit-writing-mode:horizontal-bt; is set. But when we fix that, we need to make such
- // pages work correctly with headers and footers as well. https://bugs.webkit.org/show_bug.cgi?id=113741
- if (scrollPosition >= 0)
- return 0;
- if (scrollPosition < -maxValue)
- scrollPosition = -maxValue;
+ // Use intersection to constrain our ideal scroll rect by the document rect.
+ scrollRect.intersect(documentRect);
+
+ if (scrollRect.size() != idealScrollRectSize) {
+ // If the rect was clipped, restore its size, effectively pushing it "down" from the top left.
+ scrollRect.setSize(idealScrollRectSize);
+
+ // If we still clip, push our rect "up" from the bottom right.
+ scrollRect.intersect(documentRect);
+ if (scrollRect.width() < idealScrollRectSize.width())
+ scrollRect.move(-(idealScrollRectSize.width() - scrollRect.width()), 0);
+ if (scrollRect.height() < idealScrollRectSize.height())
+ scrollRect.move(0, -(idealScrollRectSize.height() - scrollRect.height()));
}
- return scrollPosition;
+ return scrollRect.location() - toIntSize(scrollOrigin);
}
-IntPoint ScrollableArea::constrainScrollPositionForOverhang(const IntRect& visibleContentRect, const IntSize& totalContentsSize, const IntPoint& scrollPosition, const IntPoint& scrollOrigin, int headerHeight, int footerHeight)
-{
- return IntPoint(constrainedScrollPosition(visibleContentRect.width(), totalContentsSize.width(), scrollPosition.x(), scrollOrigin.x(), 0, 0),
- constrainedScrollPosition(visibleContentRect.height(), totalContentsSize.height(), scrollPosition.y(), scrollOrigin.y(), headerHeight, footerHeight));
-}
-
IntPoint ScrollableArea::constrainScrollPositionForOverhang(const IntPoint& scrollPosition)
{
return constrainScrollPositionForOverhang(visibleContentRect(), totalContentsSize(), scrollPosition, scrollOrigin(), headerHeight(), footerHeight());
