https://bugs.webkit.org/show_bug.cgi?id=106946
authorbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 19 Jan 2013 02:22:41 +0000 (02:22 +0000)
committerbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 19 Jan 2013 02:22:41 +0000 (02:22 +0000)
Sticky-position elements can jump around/hide on rubber-banding

Reviewed by Simon Fraser.

Source/WebCore:

This patch adds a new function called viewportConstrainedVisibleContentRect()
since there are a number of places where we need a visibleContectRect() that does
not allow scroll offsets that indicate the rubber-banding that is happening. And
this patch fixes the bug for sticky by calling this function in two new places.

* page/FrameView.cpp:
(WebCore::FrameView::viewportConstrainedVisibleContentRect):
(WebCore):
* page/FrameView.h:
(FrameView):
* page/scrolling/ScrollingCoordinator.cpp:
(WebCore::ScrollingCoordinator::updateMainFrameScrollPosition):
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::stickyPositionOffset):
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::computeFixedViewportConstraints):
(WebCore::RenderLayerCompositor::computeStickyViewportConstraints):

LayoutTests:

* platform/mac/tiled-drawing/sticky/negative-scroll-offset-expected.txt: Added.
* platform/mac/tiled-drawing/sticky/negative-scroll-offset.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@140229 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/mac/tiled-drawing/sticky/negative-scroll-offset-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/tiled-drawing/sticky/negative-scroll-offset.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
Source/WebCore/rendering/RenderBoxModelObject.cpp
Source/WebCore/rendering/RenderLayerCompositor.cpp

index a79ab53..444076c 100644 (file)
@@ -1,3 +1,13 @@
+2013-01-18  Beth Dakin  <bdakin@apple.com>
+
+        https://bugs.webkit.org/show_bug.cgi?id=106946
+        Sticky-position elements can jump around/hide on rubber-banding
+
+        Reviewed by Simon Fraser.
+
+        * platform/mac/tiled-drawing/sticky/negative-scroll-offset-expected.txt: Added.
+        * platform/mac/tiled-drawing/sticky/negative-scroll-offset.html: Added.
+
 2013-01-18  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r140206.
diff --git a/LayoutTests/platform/mac/tiled-drawing/sticky/negative-scroll-offset-expected.txt b/LayoutTests/platform/mac/tiled-drawing/sticky/negative-scroll-offset-expected.txt
new file mode 100644 (file)
index 0000000..b0c359e
--- /dev/null
@@ -0,0 +1,19 @@
+(Scrolling node
+  (viewport rect 0 0 785 600)
+  (contents size 785 2216)
+  (requested scroll position 0 -20)
+  (children 1
+    (Sticky node
+      (anchor edges: AnchorEdgeLeft AnchorEdgeRight AnchorEdgeBottom)
+      (left offset 0.00)
+      (right offset 0.00)
+      (bottom offset 0.00)
+      (containing block rect 8.00, 8.00 769.00 x 2200.00)
+      (sticky box rect 8.00 8.00 773.00 54.00)
+      (sticky box rect 8.00 8.00 773.00 54.00)
+      (sticky offset at last layout 0.00 0.00)
+      (layer position at last layout 8.00 8.00)
+    )
+  )
+)
+
diff --git a/LayoutTests/platform/mac/tiled-drawing/sticky/negative-scroll-offset.html b/LayoutTests/platform/mac/tiled-drawing/sticky/negative-scroll-offset.html
new file mode 100644 (file)
index 0000000..36acfc5
--- /dev/null
@@ -0,0 +1,49 @@
+<html>
+<head>
+    <style type="text/css" media="screen">
+      body {
+        height: 2200px;
+      }
+
+      .sticky {
+        position: -webkit-sticky;
+        width: 100%;
+        height: 50px;
+        left: 0;
+        right: 0;
+        bottom: 0;
+        background-color: rgba(0, 128, 0, 0.8);
+        border: 2px solid black;
+      }
+      
+    </style>
+    <script type="text/javascript" charset="utf-8">
+      if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+      }
+
+      function doScroll()
+      {
+        window.setTimeout(function() {
+          window.scrollTo(0, -20);
+          if (window.testRunner) {
+            document.getElementById('results').innerText = window.internals.scrollingStateTreeAsText(document);
+            testRunner.notifyDone();
+          }
+        }, 10);
+      }
+      
+      window.addEventListener('load', doScroll, false);
+    </script>
+
+    
+</head>
+<body>
+
+<pre id="results"></pre>
+
+<div class="sticky"></div>
+
+</body>
+</html>
index 5234afe..01bc9fa 100644 (file)
@@ -1,3 +1,28 @@
+2013-01-18  Beth Dakin  <bdakin@apple.com>
+
+        https://bugs.webkit.org/show_bug.cgi?id=106946
+        Sticky-position elements can jump around/hide on rubber-banding
+
+        Reviewed by Simon Fraser.
+
+        This patch adds a new function called viewportConstrainedVisibleContentRect() 
+        since there are a number of places where we need a visibleContectRect() that does 
+        not allow scroll offsets that indicate the rubber-banding that is happening. And 
+        this patch fixes the bug for sticky by calling this function in two new places.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::viewportConstrainedVisibleContentRect):
+        (WebCore):
+        * page/FrameView.h:
+        (FrameView):
+        * page/scrolling/ScrollingCoordinator.cpp:
+        (WebCore::ScrollingCoordinator::updateMainFrameScrollPosition):
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::stickyPositionOffset):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::computeFixedViewportConstraints):
+        (WebCore::RenderLayerCompositor::computeStickyViewportConstraints):
+
 2013-01-18  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r140206.
index c20b038..9669906 100644 (file)
@@ -1494,6 +1494,13 @@ void FrameView::removeViewportConstrainedObject(RenderObject* object)
     }
 }
 
+LayoutRect FrameView::viewportConstrainedVisibleContentRect() const
+{
+    LayoutRect viewportRect = visibleContentRect();
+    viewportRect.setLocation(toPoint(scrollOffsetForFixedPosition()));
+    return viewportRect;
+}
+
 IntSize FrameView::scrollOffsetForFixedPosition() const
 {
     IntRect visibleContentRect = this->visibleContentRect();
index a98e88f..c7d877e 100644 (file)
@@ -191,6 +191,10 @@ public:
     virtual bool shouldRubberBandInDirection(ScrollDirection) const;
     virtual bool requestScrollPositionUpdate(const IntPoint&) OVERRIDE;
 
+    // This is different than visibleContentRect() in that it ignores negative (or overly positive)
+    // offsets from rubber-banding, and it takes zooming into account. 
+    LayoutRect viewportConstrainedVisibleContentRect() const;
+
     String mediaType() const;
     void setMediaType(const String&);
     void adjustMediaTypeForPrinting(bool printing);
index 06fcd7c..3122e56 100644 (file)
@@ -392,8 +392,7 @@ void ScrollingCoordinator::updateMainFrameScrollPosition(const IntPoint& scrollP
             if (counterScrollingLayer)
                 counterScrollingLayer->syncPosition(IntPoint(frameView->scrollOffsetForFixedPosition()));
 
-            LayoutRect viewportRect = frameView->visibleContentRect();
-            viewportRect.setLocation(IntPoint(frameView->scrollOffsetForFixedPosition()));
+            LayoutRect viewportRect = frameView->viewportConstrainedVisibleContentRect();
             syncChildPositions(viewportRect);
         }
     }
index da49c11..9e6f1c6 100644 (file)
@@ -509,7 +509,7 @@ void RenderBoxModelObject::computeStickyPositionConstraints(StickyPositionViewpo
 
 LayoutSize RenderBoxModelObject::stickyPositionOffset() const
 {
-    LayoutRect viewportRect = view()->frameView()->visibleContentRect();
+    LayoutRect viewportRect = view()->frameView()->viewportConstrainedVisibleContentRect();
     float scale = 1;
     if (Frame* frame = view()->frameView()->frame())
         scale = frame->frameScaleFactor();
index 1db8573..309c212 100644 (file)
@@ -2728,9 +2728,7 @@ FixedPositionViewportConstraints RenderLayerCompositor::computeFixedViewportCons
     ASSERT(layer->isComposited());
 
     FrameView* frameView = m_renderView->frameView();
-
-    LayoutRect viewportRect = frameView->visibleContentRect();
-    viewportRect.setLocation(toPoint(frameView->scrollOffsetForFixedPosition()));
+    LayoutRect viewportRect = frameView->viewportConstrainedVisibleContentRect();
 
     FixedPositionViewportConstraints constraints = FixedPositionViewportConstraints();
 
@@ -2768,7 +2766,7 @@ StickyPositionViewportConstraints RenderLayerCompositor::computeStickyViewportCo
     ASSERT(layer->isComposited());
 
     FrameView* frameView = m_renderView->frameView();
-    LayoutRect viewportRect = frameView->visibleContentRect();
+    LayoutRect viewportRect = frameView->viewportConstrainedVisibleContentRect();
 
     StickyPositionViewportConstraints constraints = StickyPositionViewportConstraints();