setPageScaleFactor should setScrollPosition if scale is unchanged
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 May 2012 23:44:03 +0000 (23:44 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 May 2012 23:44:03 +0000 (23:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=84400

Patch by Alexandre Elias <aelias@google.com> on 2012-05-09
Reviewed by Adam Barth.

Previously, setPageScaleFactor forgot about its "origin" argument if
the page scale factor is unchanged.  This has proven undesirable in
practice because, for example, a single pinch gesture may zoom in and
back out to the original page scale factor, but at a different scroll
offset.

New test case added to scale-and-scroll-body-expected.txt

* page/Page.cpp:
(WebCore::Page::setPageScaleFactor):

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

LayoutTests/fast/events/scale-and-scroll-body-expected.txt
LayoutTests/fast/events/scale-and-scroll-body.html
Source/WebCore/ChangeLog
Source/WebCore/page/Page.cpp

index c1b847d..2d7c5e1 100644 (file)
@@ -1,3 +1,5 @@
+PASS window.document.body.scrollTop is 30
+PASS window.document.body.scrollLeft is 30
 PASS window.document.body.scrollTop is 100
 PASS window.document.body.scrollLeft is 100
 PASS window.scrollX is 100
index 9e6e005..d952906 100644 (file)
@@ -9,7 +9,12 @@
     <script>
       window.enablePixelTesting = true;
 
-      function scroll() {
+      function scrollViaJavascript() {
+          var scaleFactor = 2.0;
+          if (window.internals) {
+             window.internals.settings.setPageScaleFactor(scaleFactor, 0, 0);
+          }
+
           // The page scale, as set by window.internals.settings.setPageScaleFactor should not be apparent
           // to javascript. So, we expect scrolling to (100,100) to be page coordinates, rather
           // than device pixels.
           shouldBe("window.scrollY", "100");
       }
 
-      function scaleWithEventSender() {
-          var scaleFactor = 2.0;
-          var scaleOffset = 0;
+      function scrollViaSetScaleFactor() {
           if (window.internals) {
-             window.internals.settings.setPageScaleFactor(scaleFactor, scaleOffset, scaleOffset);
+              // Test that the scroll offset changes even if scaleFactor remains
+              // the same.
+              window.internals.settings.setPageScaleFactor(1, 30, 30);
+              shouldBe("window.document.body.scrollTop", "30");
+              shouldBe("window.document.body.scrollLeft", "30");
           }
       }
 
       function test() {
-          scaleWithEventSender();
-          scroll();
+          scrollViaSetScaleFactor();
+          scrollViaJavascript();
       }
     </script>
     <script src="../js/resources/js-test-pre.js"></script>
index 4edfd16..31ded39 100644 (file)
@@ -1,3 +1,21 @@
+2012-05-09  Alexandre Elias  <aelias@google.com>
+
+        setPageScaleFactor should setScrollPosition if scale is unchanged
+        https://bugs.webkit.org/show_bug.cgi?id=84400
+
+        Reviewed by Adam Barth.
+
+        Previously, setPageScaleFactor forgot about its "origin" argument if
+        the page scale factor is unchanged.  This has proven undesirable in
+        practice because, for example, a single pinch gesture may zoom in and
+        back out to the original page scale factor, but at a different scroll
+        offset.
+
+        New test case added to scale-and-scroll-body-expected.txt
+
+        * page/Page.cpp:
+        (WebCore::Page::setPageScaleFactor):
+
 2012-05-09  Hugo Parente Lima  <hugo.lima@openbossa.org>
 
         Use suitable viewport values on XHTML-MP pages.
index 0c40f33..d30900f 100644 (file)
@@ -641,10 +641,16 @@ void Page::setMediaVolume(float volume)
 
 void Page::setPageScaleFactor(float scale, const IntPoint& origin)
 {
-    if (scale == m_pageScaleFactor)
-        return;
-
     Document* document = mainFrame()->document();
+    FrameView* view = document->view();
+
+    if (scale == m_pageScaleFactor) {
+        if (view && view->scrollPosition() != origin) {
+            document->updateLayoutIgnorePendingStylesheets();
+            view->setScrollPosition(origin);
+        }
+        return;
+    }
 
     m_pageScaleFactor = scale;
 
@@ -657,12 +663,10 @@ void Page::setPageScaleFactor(float scale, const IntPoint& origin)
     mainFrame()->deviceOrPageScaleFactorChanged();
 #endif
 
-    if (FrameView* view = document->view()) {
-        if (view->scrollPosition() != origin) {
-          if (document->renderer() && document->renderer()->needsLayout() && view->didFirstLayout())
-              view->layout();
-          view->setScrollPosition(origin);
-        }
+    if (view && view->scrollPosition() != origin) {
+        if (document->renderer() && document->renderer()->needsLayout() && view->didFirstLayout())
+            view->layout();
+        view->setScrollPosition(origin);
     }
 }