Changeset 118121 in webkit

Timestamp:

May 22, 2012 10:03:16 PM (12 years ago)

Author:

commit-queue@webkit.org

Message:

[chromium] Apply viewport tag initial-scale only once
​https://bugs.webkit.org/show_bug.cgi?id=82949

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

First, check that isPageScaleFactorSet return false before setting
initial-scale. We need to call dispatchViewportPropertiesDidChange()
when the viewport width changes, since that's an input to the viewport
tag calculation. When this happens, we shouldn't pop back to initial
scale.

Second, check that isNewNavigation is true when deciding to clear
isPageScaleFactorIsSet in didCommitLoad. We only want to clear it on
the very first commit, otherwise we'll pop back to initial scale if
the user zooms in before the load is complete.

New test WebFrameTest::FixedLayoutInitializeAtMinimumPageScale.

• src/ChromeClientImpl.cpp:

(WebKit::ChromeClientImpl::dispatchViewportPropertiesDidChange):

• src/WebViewImpl.cpp:

(WebKit::WebViewImpl::didCommitLoad):

Location:

trunk/Source/WebKit/chromium

Files:

• ChangeLog (modified) (1 diff)
• src/ChromeClientImpl.cpp (modified) (2 diffs)
• src/WebViewImpl.cpp (modified) (1 diff)
• tests/WebFrameTest.cpp (modified) (3 diffs)
• tests/data/fixed_layout.html (added)

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2012-05-22  Alexandre Elias  <aelias@google.com>
+
+        [chromium] Apply viewport tag initial-scale only once
+        https://bugs.webkit.org/show_bug.cgi?id=82949
+
+        Reviewed by Adam Barth.
+
+        First, check that isPageScaleFactorSet return false before setting
+        initial-scale. We need to call dispatchViewportPropertiesDidChange()
+        when the viewport width changes, since that's an input to the viewport
+        tag calculation. When this happens, we shouldn't pop back to initial
+        scale.
+
+        Second, check that isNewNavigation is true when deciding to clear
+        isPageScaleFactorIsSet in didCommitLoad. We only want to clear it on
+        the very first commit, otherwise we'll pop back to initial scale if
+        the user zooms in before the load is complete.
+
+        New test WebFrameTest::FixedLayoutInitializeAtMinimumPageScale.
+
+        * src/ChromeClientImpl.cpp:
+        (WebKit::ChromeClientImpl::dispatchViewportPropertiesDidChange):
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::didCommitLoad):
+
 2012-05-21  Shawn Singh  <shawnsingh@chromium.org>
 

trunk/Source/WebKit/chromium/src/ChromeClientImpl.cpp

@@ -666 +666 @@
     m_webView->setFixedLayoutSize(IntSize(layoutWidth, layoutHeight));
 
-    // FIXME: Investigate the impact this has on layout/rendering if any.
-    // This exposes the correct device scale to javascript and media queries.
+    bool needInitializePageScale = !m_webView->isPageScaleFactorSet();
     if (useDefaultDeviceScaleFactor && settings->defaultDeviceScaleFactor())
         m_webView->setDeviceScaleFactor(settings->defaultDeviceScaleFactor());
@@ -673 +672 @@
         m_webView->setDeviceScaleFactor(computed.devicePixelRatio);
     m_webView->setPageScaleFactorLimits(computed.minimumScale, computed.maximumScale);
-    m_webView->setPageScaleFactorPreservingScrollOffset(computed.initialScale * computed.devicePixelRatio);
+    if (needInitializePageScale)
+        m_webView->setPageScaleFactorPreservingScrollOffset(computed.initialScale * computed.devicePixelRatio);
 #endif
 }

trunk/Source/WebKit/chromium/src/WebViewImpl.cpp

@@ -3103 +3103 @@
 #endif
     m_observedNewNavigation = false;
-    if (!isNavigationWithinPage)
+    if (*isNewNavigation && !isNavigationWithinPage)
         m_pageScaleFactorIsSet = false;
 

trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp

@@ -35 +35 @@
 #include "Frame.h"
 #include "FrameTestHelpers.h"
+#include "FrameView.h"
 #include "ResourceError.h"
 #include "WebDocument.h"
@@ -40 +41 @@
 #include "WebFormElement.h"
 #include "WebFrameClient.h"
+#include "WebFrameImpl.h"
 #include "WebRange.h"
 #include "WebScriptSource.h"
@@ -243 +245 @@
     // Force the layout to happen before leaving the test.
     webView->mainFrame()->contentAsText(1024).utf8();
+}
+
+TEST_F(WebFrameTest, FixedLayoutInitializeAtMinimumPageScale)
+{
+    registerMockedHttpURLLoad("fixed_layout.html");
+
+    FixedLayoutTestWebViewClient client;
+    client.m_screenInfo.horizontalDPI = 160;
+    int viewportWidth = 640;
+    int viewportHeight = 480;
+    client.m_windowRect = WebRect(0, 0, viewportWidth, viewportHeight);
+
+    // Make sure we initialize to minimum scale, even if the window size
+    // only becomes available after the load begins.
+    WebViewImpl* webViewImpl = static_cast<WebViewImpl*>(FrameTestHelpers::createWebViewAndLoad(m_baseURL + "fixed_layout.html", true, 0, &client));
+    webViewImpl->enableFixedLayoutMode(true);
+    webViewImpl->settings()->setViewportEnabled(true);
+    webViewImpl->resize(WebSize(viewportWidth, viewportHeight));
+
+    int defaultFixedLayoutWidth = 980;
+    float minimumPageScaleFactor = viewportWidth / (float) defaultFixedLayoutWidth;
+    EXPECT_EQ(minimumPageScaleFactor, webViewImpl->pageScaleFactor());
+
+    // Assume the user has pinch zoomed to page scale factor 2.
+    float userPinchPageScaleFactor = 2;
+    webViewImpl->setPageScaleFactorPreservingScrollOffset(userPinchPageScaleFactor);
+    webViewImpl->mainFrameImpl()->frameView()->layout();
+
+    // Make sure we don't reset to initial scale if the page continues to load.
+    bool isNewNavigation;
+    webViewImpl->didCommitLoad(&isNewNavigation, false);
+    webViewImpl->didChangeContentsSize();
+    EXPECT_EQ(userPinchPageScaleFactor, webViewImpl->pageScaleFactor());
+
+    // Make sure we don't reset to initial scale if the viewport size changes.
+    webViewImpl->resize(WebSize(viewportWidth, viewportHeight + 100));
+    EXPECT_EQ(userPinchPageScaleFactor, webViewImpl->pageScaleFactor());
 }
 #endif