Comcast website displays bottom of page when loaded
authorandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 12 May 2012 00:56:13 +0000 (00:56 +0000)
committerandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 12 May 2012 00:56:13 +0000 (00:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=86277
<rdar://problem/11426887>

Reviewed by Beth Dakin.

There were two bugs here. The first bug was that FrameView::setScrollPosition didn't end up calling into the scrolling coordinator
to update the scroll position. The second bug was that ScrollingTreeNodeMac::setScrollPosition didn't constrain the scroll position
to the edge of the page.

* page/FrameView.cpp:
(WebCore::FrameView::setScrollPosition):
Call requestScrollPositionUpdate.

* page/scrolling/ScrollingTree.cpp:
* page/scrolling/ScrollingTree.h:
Remove setMainFrameScrollPosition, it is not called by anyone.

* page/scrolling/mac/ScrollingTreeNodeMac.h:
* page/scrolling/mac/ScrollingTreeNodeMac.mm:
(WebCore::ScrollingTreeNodeMac::setScrollPosition):
Clamp to the page size and call setScrollPositionWithoutContentEdgeConstraints.

(WebCore::ScrollingTreeNodeMac::setScrollPositionWithoutContentEdgeConstraints):
Update the scroll layer position and call back to the main thread.

(WebCore::ScrollingTreeNodeMac::scrollBy):
Call setScrollPosition.

(WebCore::ScrollingTreeNodeMac::scrollByWithoutContentEdgeConstraints):
Call setScrollPositionWithoutContentEdgeConstraints.

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

Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/scrolling/ScrollingTree.cpp
Source/WebCore/page/scrolling/ScrollingTree.h
Source/WebCore/page/scrolling/mac/ScrollingTreeNodeMac.h
Source/WebCore/page/scrolling/mac/ScrollingTreeNodeMac.mm

index a14f093..3d4ab26 100644 (file)
@@ -1,3 +1,37 @@
+2012-05-11  Anders Carlsson  <andersca@apple.com>
+
+        Comcast website displays bottom of page when loaded
+        https://bugs.webkit.org/show_bug.cgi?id=86277
+        <rdar://problem/11426887>
+
+        Reviewed by Beth Dakin.
+
+        There were two bugs here. The first bug was that FrameView::setScrollPosition didn't end up calling into the scrolling coordinator
+        to update the scroll position. The second bug was that ScrollingTreeNodeMac::setScrollPosition didn't constrain the scroll position
+        to the edge of the page.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::setScrollPosition):
+        Call requestScrollPositionUpdate.
+
+        * page/scrolling/ScrollingTree.cpp:
+        * page/scrolling/ScrollingTree.h:
+        Remove setMainFrameScrollPosition, it is not called by anyone.
+
+        * page/scrolling/mac/ScrollingTreeNodeMac.h:
+        * page/scrolling/mac/ScrollingTreeNodeMac.mm:
+        (WebCore::ScrollingTreeNodeMac::setScrollPosition):
+        Clamp to the page size and call setScrollPositionWithoutContentEdgeConstraints.
+
+        (WebCore::ScrollingTreeNodeMac::setScrollPositionWithoutContentEdgeConstraints):
+        Update the scroll layer position and call back to the main thread.
+
+        (WebCore::ScrollingTreeNodeMac::scrollBy):
+        Call setScrollPosition.
+
+        (WebCore::ScrollingTreeNodeMac::scrollByWithoutContentEdgeConstraints):
+        Call setScrollPositionWithoutContentEdgeConstraints.
+
 2012-05-11  Gavin Barraclough  <barraclough@apple.com>
 
         Introduce PropertyName class
index 25531b0..a9cc220 100644 (file)
@@ -1704,6 +1704,10 @@ void FrameView::setScrollPosition(const IntPoint& scrollPoint)
 {
     TemporaryChange<bool> changeInProgrammaticScroll(m_inProgrammaticScroll, true);
     m_maintainScrollPositionAnchor = 0;
+
+    if (requestScrollPositionUpdate(scrollPoint))
+        return;
+
     ScrollView::setScrollPosition(scrollPoint);
 }
 
index 5516a71..36e6aae 100644 (file)
@@ -97,13 +97,6 @@ void ScrollingTree::handleWheelEvent(const PlatformWheelEvent& wheelEvent)
     m_rootNode->handleWheelEvent(wheelEvent);
 }
 
-void ScrollingTree::setMainFrameScrollPosition(const IntPoint& scrollPosition)
-{
-    ASSERT(ScrollingThread::isCurrentThread());
-
-    m_rootNode->setScrollPosition(scrollPosition);
-}
-
 static void derefScrollingCoordinator(ScrollingCoordinator* scrollingCoordinator)
 {
     ASSERT(isMainThread());
index 82b7c24..ef77926 100644 (file)
@@ -75,8 +75,6 @@ public:
     // Must be called from the scrolling thread. Handles the wheel event.
     void handleWheelEvent(const PlatformWheelEvent&);
 
-    void setMainFrameScrollPosition(const IntPoint&);
-
     void invalidate();
     void commitNewTreeState(PassOwnPtr<ScrollingTreeState>);
 
index f056ff9..8a1576e 100644 (file)
@@ -45,7 +45,6 @@ private:
     // ScrollingTreeNode member functions.
     virtual void update(ScrollingTreeState*) OVERRIDE;
     virtual void handleWheelEvent(const PlatformWheelEvent&) OVERRIDE;
-    virtual void setScrollPosition(const IntPoint&) OVERRIDE;
 
     // ScrollElasticityController member functions.
     virtual bool allowsHorizontalStretching() OVERRIDE;
@@ -62,6 +61,9 @@ private:
     virtual void stopSnapRubberbandTimer() OVERRIDE;
 
     IntPoint scrollPosition() const;
+    void setScrollPosition(const IntPoint&);
+    void setScrollPositionWithoutContentEdgeConstraints(const IntPoint&);
+
     void setScrollLayerPosition(const IntPoint&);
 
     IntPoint minimumScrollPosition() const;
index b87e446..a37bf40 100644 (file)
@@ -85,20 +85,6 @@ void ScrollingTreeNodeMac::handleWheelEvent(const PlatformWheelEvent& wheelEvent
     scrollingTree()->handleWheelEventPhase(wheelEvent.phase());
 }
 
-void ScrollingTreeNodeMac::setScrollPosition(const IntPoint& scrollPosition)
-{
-    updateMainFramePinState(scrollPosition);
-
-    if (shouldUpdateScrollLayerPositionOnMainThread()) {
-        m_probableMainThreadScrollPosition = scrollPosition;
-        scrollingTree()->updateMainFrameScrollPositionAndScrollLayerPosition(scrollPosition);
-        return;
-    }
-
-    setScrollLayerPosition(scrollPosition);
-    scrollingTree()->updateMainFrameScrollPosition(scrollPosition);
-}
-
 bool ScrollingTreeNodeMac::allowsHorizontalStretching()
 {
     switch (horizontalScrollElasticity()) {
@@ -240,6 +226,29 @@ IntPoint ScrollingTreeNodeMac::scrollPosition() const
     return IntPoint(-scrollLayerPosition.x + scrollOrigin().x(), -scrollLayerPosition.y + scrollOrigin().y());
 }
 
+void ScrollingTreeNodeMac::setScrollPosition(const IntPoint& scrollPosition)
+{
+    IntPoint newScrollPosition = scrollPosition;
+    newScrollPosition = newScrollPosition.shrunkTo(maximumScrollPosition());
+    newScrollPosition = newScrollPosition.expandedTo(minimumScrollPosition());
+
+    setScrollPositionWithoutContentEdgeConstraints(newScrollPosition);
+}
+
+void ScrollingTreeNodeMac::setScrollPositionWithoutContentEdgeConstraints(const IntPoint& scrollPosition)
+{
+    updateMainFramePinState(scrollPosition);
+
+    if (shouldUpdateScrollLayerPositionOnMainThread()) {
+        m_probableMainThreadScrollPosition = scrollPosition;
+        scrollingTree()->updateMainFrameScrollPositionAndScrollLayerPosition(scrollPosition);
+        return;
+    }
+
+    setScrollLayerPosition(scrollPosition);
+    scrollingTree()->updateMainFrameScrollPosition(scrollPosition);
+}
+
 void ScrollingTreeNodeMac::setScrollLayerPosition(const IntPoint& position)
 {
     ASSERT(!shouldUpdateScrollLayerPositionOnMainThread());
@@ -263,16 +272,12 @@ IntPoint ScrollingTreeNodeMac::maximumScrollPosition() const
 
 void ScrollingTreeNodeMac::scrollBy(const IntSize& offset)
 {
-    IntPoint newScrollPosition = scrollPosition() + offset;
-    newScrollPosition = newScrollPosition.shrunkTo(maximumScrollPosition());
-    newScrollPosition = newScrollPosition.expandedTo(minimumScrollPosition());
-
-    setScrollPosition(newScrollPosition);
+    setScrollPosition(scrollPosition() + offset);
 }
 
 void ScrollingTreeNodeMac::scrollByWithoutContentEdgeConstraints(const IntSize& offset)
 {
-    setScrollPosition(scrollPosition() + offset);
+    setScrollPositionWithoutContentEdgeConstraints(scrollPosition() + offset);
 }
 
 void ScrollingTreeNodeMac::updateMainFramePinState(const IntPoint& scrollPosition)