Title: [237743] trunk
Revision
237743
Author
wenson_hs...@apple.com
Date
2018-11-02 11:10:23 -0700 (Fri, 02 Nov 2018)

Log Message

[iOS] Changing view scale sometimes does not zoom the page to the new initial scale, when the page is at initial scale
https://bugs.webkit.org/show_bug.cgi?id=191180
<rdar://problem/45744786>

Reviewed by Simon Fraser.

Source/WebCore:

When computing the minimum scale in ViewportConfiguration::minimumScale, if our content width or height is
shorter than the view width or height, then we recompute the minimum scale such that the content dimensions will
fill the bounds of the view by setting the minimum scale to the view width or height divided by the content
width or height.

Suppose the minimum scale is equal to some value `s`; additionally, let `w_c` denote the content width and `w_v`
denote the view width (as integers). If `w_v / s` is not an integral value, the computed content width `w_c` may
be rounded, such that `w_v / w_c` is not precisely equal to `s`. In the case that `w_v / w_c` is ever so
slightly larger than `s`, we will end up overriding the minimum scale `s` with `w_v / w_c`.

As a result, specifying a viewport with a decimal `minimum-scale` will sometimes cause the computed minimum
scale of the viewport (and platform view) to be very slightly different from the minimum scale. The new layout
test below exercises this scenario, specifying a viewport with minimum and initial scales of 0.94 that results
in `ViewportConfiguration::minimumScale` returning 0.94158.

With the `WebPage::setViewportConfigurationViewLayoutSize` check added in r237127, this means setting
`_viewScale:` when the page is at initial scale sometimes doesn't zoom the page to the new initial scale when it
should, since the page scale factor and the initial scale are different enough such that
`areEssentiallyEqualAsFloat` returns false.

This patch addresses these issues by snapping to the minimum scale if the computed scale that fits content
dimensions to view dimensions results in a minimum scale that is close enough to the configuration's minimum
scale, such that the difference can be attributed to rounding error when computing content or view dimensions.

Test: fast/viewport/ios/viewport-minimum-and-initial-scale.html

* page/ViewportConfiguration.cpp:
(WebCore::ViewportConfiguration::minimumScale const):

LayoutTests:

Add a layout test, and make some adjustments to UIHelper.

* fast/viewport/ios/constant-width-viewport-after-changing-view-scale.html:
* fast/viewport/ios/device-width-viewport-after-changing-view-scale.html:
* fast/viewport/ios/viewport-minimum-and-initial-scale-expected.txt: Added.
* fast/viewport/ios/viewport-minimum-and-initial-scale.html: Added.

Add a new layout test that contains a viewport meta tag with minimum and initial scales set to 0.94, and checks
that the resulting minimum and initial scales are 0.94 instead of 0.94158.

* fast/viewport/watchos/viewport-disable-extra-zoom-adaptations.html:
* resources/ui-helper.js:

Make UIHelper.zoomScale return a number rather than a string, and adjust a few call sites.

(window.UIHelper.zoomScale):
(window.UIHelper.minimumZoomScale):
(window.UIHelper):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (237742 => 237743)


--- trunk/LayoutTests/ChangeLog	2018-11-02 17:45:37 UTC (rev 237742)
+++ trunk/LayoutTests/ChangeLog	2018-11-02 18:10:23 UTC (rev 237743)
@@ -1,3 +1,30 @@
+2018-11-02  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [iOS] Changing view scale sometimes does not zoom the page to the new initial scale, when the page is at initial scale
+        https://bugs.webkit.org/show_bug.cgi?id=191180
+        <rdar://problem/45744786>
+
+        Reviewed by Simon Fraser.
+
+        Add a layout test, and make some adjustments to UIHelper.
+
+        * fast/viewport/ios/constant-width-viewport-after-changing-view-scale.html:
+        * fast/viewport/ios/device-width-viewport-after-changing-view-scale.html:
+        * fast/viewport/ios/viewport-minimum-and-initial-scale-expected.txt: Added.
+        * fast/viewport/ios/viewport-minimum-and-initial-scale.html: Added.
+
+        Add a new layout test that contains a viewport meta tag with minimum and initial scales set to 0.94, and checks
+        that the resulting minimum and initial scales are 0.94 instead of 0.94158.
+
+        * fast/viewport/watchos/viewport-disable-extra-zoom-adaptations.html:
+        * resources/ui-helper.js:
+
+        Make UIHelper.zoomScale return a number rather than a string, and adjust a few call sites.
+
+        (window.UIHelper.zoomScale):
+        (window.UIHelper.minimumZoomScale):
+        (window.UIHelper):
+
 2018-11-02  Daniel Bates  <daba...@apple.com>
 
         [iOS] WebKit should dispatch DOM events when a modifier key is pressed

Modified: trunk/LayoutTests/fast/viewport/ios/constant-width-viewport-after-changing-view-scale.html (237742 => 237743)


--- trunk/LayoutTests/fast/viewport/ios/constant-width-viewport-after-changing-view-scale.html	2018-11-02 17:45:37 UTC (rev 237742)
+++ trunk/LayoutTests/fast/viewport/ios/constant-width-viewport-after-changing-view-scale.html	2018-11-02 18:10:23 UTC (rev 237743)
@@ -43,7 +43,7 @@
 
             appendOutput(`window size: [${innerWidth}, ${innerHeight}]`);
             appendOutput(`square size: [${square.clientWidth}, ${square.clientHeight}]`);
-            appendOutput(`zoom scale: ${parseFloat(await UIHelper.zoomScale()).toFixed(2)}`);
+            appendOutput(`zoom scale: ${(await UIHelper.zoomScale()).toFixed(2)}`);
             appendOutput("");
         }
 

Modified: trunk/LayoutTests/fast/viewport/ios/device-width-viewport-after-changing-view-scale.html (237742 => 237743)


--- trunk/LayoutTests/fast/viewport/ios/device-width-viewport-after-changing-view-scale.html	2018-11-02 17:45:37 UTC (rev 237742)
+++ trunk/LayoutTests/fast/viewport/ios/device-width-viewport-after-changing-view-scale.html	2018-11-02 18:10:23 UTC (rev 237743)
@@ -43,7 +43,7 @@
 
             appendOutput(`window size: [${innerWidth}, ${innerHeight}]`);
             appendOutput(`square size: [${square.clientWidth}, ${square.clientHeight}]`);
-            appendOutput(`zoom scale: ${parseFloat(await UIHelper.zoomScale()).toFixed(2)}`);
+            appendOutput(`zoom scale: ${(await UIHelper.zoomScale()).toFixed(2)}`);
             appendOutput("");
         }
 

Added: trunk/LayoutTests/fast/viewport/ios/viewport-minimum-and-initial-scale-expected.txt (0 => 237743)


--- trunk/LayoutTests/fast/viewport/ios/viewport-minimum-and-initial-scale-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/viewport/ios/viewport-minimum-and-initial-scale-expected.txt	2018-11-02 18:10:23 UTC (rev 237743)
@@ -0,0 +1,2 @@
+The current scale is 0.94000, and the minimum scale is 0.94000
+This test verifies that specifying initial and minimum scales of 0.94 sets the current and minimum zoom scales to exactly 0.94. This is best run under WebKitTestRunner.

Added: trunk/LayoutTests/fast/viewport/ios/viewport-minimum-and-initial-scale.html (0 => 237743)


--- trunk/LayoutTests/fast/viewport/ios/viewport-minimum-and-initial-scale.html	                        (rev 0)
+++ trunk/LayoutTests/fast/viewport/ios/viewport-minimum-and-initial-scale.html	2018-11-02 18:10:23 UTC (rev 237743)
@@ -0,0 +1,37 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="width=device-width, initial-scale=0.94, minimum-scale=0.94">
+    <style>
+    #description {
+        margin-top: 100px;
+    }
+
+    body {
+        margin: 0;
+        width: 100%;
+        height: 100%;
+    }
+    </style>
+    <script src=""
+    <script>
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+    }
+
+    addEventListener("load", async () => {
+        const zoomScale = (await UIHelper.zoomScale()).toFixed(5);
+        const minimumZoomScale = (await UIHelper.minimumZoomScale()).toFixed(5);
+        output.textContent = `The current scale is ${zoomScale}, and the minimum scale is ${minimumZoomScale}`;
+        testRunner.notifyDone();
+    });
+    </script>
+</head>
+
+<body>
+    <pre id="output"></pre>
+    <p id="description">This test verifies that specifying initial and minimum scales of 0.94 sets the current and
+    minimum zoom scales to exactly 0.94. This is best run under WebKitTestRunner.</p>
+</body>
+</html>

Modified: trunk/LayoutTests/fast/viewport/watchos/viewport-disable-extra-zoom-adaptations.html (237742 => 237743)


--- trunk/LayoutTests/fast/viewport/watchos/viewport-disable-extra-zoom-adaptations.html	2018-11-02 17:45:37 UTC (rev 237742)
+++ trunk/LayoutTests/fast/viewport/watchos/viewport-disable-extra-zoom-adaptations.html	2018-11-02 18:10:23 UTC (rev 237743)
@@ -52,7 +52,7 @@
             await logWindowDimensionsAfterSettingMetaContent({
                 "viewport" : "width=device-width"
             });
-            scaleAtDeviceWidthWithDefaultAdaptations = parseFloat(await UIHelper.zoomScale()).toFixed(3);
+            scaleAtDeviceWidthWithDefaultAdaptations = (await UIHelper.zoomScale()).toFixed(3);
             await logWindowDimensionsAfterSettingMetaContent({
                 "viewport" : "width=600"
             });
@@ -71,7 +71,7 @@
                 "viewport" : "width=device-width",
                 "disabled-adaptations" : `watch, three, four, watch`
             });
-            scaleAtDeviceWidthWithAdaptationDisabled = parseFloat(await UIHelper.zoomScale()).toFixed(3);
+            scaleAtDeviceWidthWithAdaptationDisabled = (await UIHelper.zoomScale()).toFixed(3);
             await logWindowDimensionsAfterSettingMetaContent({
                 "viewport" : "width=600",
                 "disabled-adaptations" : `five, watch`
@@ -87,7 +87,7 @@
                 "viewport" : "width=device-width, shrink-to-fit=0",
                 "disabled-adaptations" : "bogus, values"
             });
-            scaleAtDeviceWidthWithShrinkToFitDisabled = parseFloat(await UIHelper.zoomScale()).toFixed(3);
+            scaleAtDeviceWidthWithShrinkToFitDisabled = (await UIHelper.zoomScale()).toFixed(3);
             await logWindowDimensionsAfterSettingMetaContent({
                 "viewport" : "width=600, shrink-to-fit=-0.5",
                 "disabled-adaptations" : ",,,"

Modified: trunk/LayoutTests/resources/ui-helper.js (237742 => 237743)


--- trunk/LayoutTests/resources/ui-helper.js	2018-11-02 17:45:37 UTC (rev 237742)
+++ trunk/LayoutTests/resources/ui-helper.js	2018-11-02 18:10:23 UTC (rev 237743)
@@ -306,7 +306,7 @@
         return new Promise(resolve => {
             testRunner.runUIScript(`(() => {
                 uiController.uiScriptComplete(uiController.zoomScale);
-            })()`, resolve);
+            })()`, scaleAsString => resolve(parseFloat(scaleAsString)));
         });
     }
 
@@ -361,4 +361,16 @@
 
         return new Promise(resolve => testRunner.runUIScript(`uiController.resignFirstResponder()`, resolve));
     }
+
+    static minimumZoomScale()
+    {
+        if (!this.isWebKit2())
+            return Promise.resolve();
+
+        return new Promise(resolve => {
+            testRunner.runUIScript(`(() => {
+                uiController.uiScriptComplete(uiController.minimumZoomScale);
+            })()`, scaleAsString => resolve(parseFloat(scaleAsString)))
+        });
+    }
 }

Modified: trunk/Source/WebCore/ChangeLog (237742 => 237743)


--- trunk/Source/WebCore/ChangeLog	2018-11-02 17:45:37 UTC (rev 237742)
+++ trunk/Source/WebCore/ChangeLog	2018-11-02 18:10:23 UTC (rev 237743)
@@ -1,3 +1,40 @@
+2018-11-02  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [iOS] Changing view scale sometimes does not zoom the page to the new initial scale, when the page is at initial scale
+        https://bugs.webkit.org/show_bug.cgi?id=191180
+        <rdar://problem/45744786>
+
+        Reviewed by Simon Fraser.
+
+        When computing the minimum scale in ViewportConfiguration::minimumScale, if our content width or height is
+        shorter than the view width or height, then we recompute the minimum scale such that the content dimensions will
+        fill the bounds of the view by setting the minimum scale to the view width or height divided by the content
+        width or height.
+
+        Suppose the minimum scale is equal to some value `s`; additionally, let `w_c` denote the content width and `w_v`
+        denote the view width (as integers). If `w_v / s` is not an integral value, the computed content width `w_c` may
+        be rounded, such that `w_v / w_c` is not precisely equal to `s`. In the case that `w_v / w_c` is ever so
+        slightly larger than `s`, we will end up overriding the minimum scale `s` with `w_v / w_c`.
+
+        As a result, specifying a viewport with a decimal `minimum-scale` will sometimes cause the computed minimum
+        scale of the viewport (and platform view) to be very slightly different from the minimum scale. The new layout
+        test below exercises this scenario, specifying a viewport with minimum and initial scales of 0.94 that results
+        in `ViewportConfiguration::minimumScale` returning 0.94158.
+
+        With the `WebPage::setViewportConfigurationViewLayoutSize` check added in r237127, this means setting
+        `_viewScale:` when the page is at initial scale sometimes doesn't zoom the page to the new initial scale when it
+        should, since the page scale factor and the initial scale are different enough such that
+        `areEssentiallyEqualAsFloat` returns false.
+
+        This patch addresses these issues by snapping to the minimum scale if the computed scale that fits content
+        dimensions to view dimensions results in a minimum scale that is close enough to the configuration's minimum
+        scale, such that the difference can be attributed to rounding error when computing content or view dimensions.
+
+        Test: fast/viewport/ios/viewport-minimum-and-initial-scale.html
+
+        * page/ViewportConfiguration.cpp:
+        (WebCore::ViewportConfiguration::minimumScale const):
+
 2018-11-02  Philippe Normand  <pnorm...@igalia.com>
 
         [GTK][WPE] Unreviewed, another --no-video --no-web-audio build fix following r237677

Modified: trunk/Source/WebCore/page/ViewportConfiguration.cpp (237742 => 237743)


--- trunk/Source/WebCore/page/ViewportConfiguration.cpp	2018-11-02 17:45:37 UTC (rev 237742)
+++ trunk/Source/WebCore/page/ViewportConfiguration.cpp	2018-11-02 18:10:23 UTC (rev 237743)
@@ -268,13 +268,30 @@
     if (m_forceAlwaysUserScalable)
         minimumScale = std::min(minimumScale, forceAlwaysUserScalableMinimumScale);
 
+    auto scaleForFittingContentIsApproximatelyEqualToMinimumScale = [] (double viewLength, double contentLength, double minimumScale) {
+        if (contentLength <= 1 || viewLength <= 1)
+            return false;
+
+        if (minimumScale < (viewLength - 0.5) / (contentLength + 0.5))
+            return false;
+
+        if (minimumScale > (viewLength + 0.5) / (contentLength - 0.5))
+            return false;
+
+        return true;
+    };
+
     double contentWidth = m_contentSize.width();
-    if (contentWidth > 0 && contentWidth * minimumScale < m_viewLayoutSize.width() && !shouldIgnoreVerticalScalingConstraints())
-        minimumScale = m_viewLayoutSize.width() / contentWidth;
+    if (contentWidth > 0 && contentWidth * minimumScale < m_viewLayoutSize.width() && !shouldIgnoreVerticalScalingConstraints()) {
+        if (!scaleForFittingContentIsApproximatelyEqualToMinimumScale(m_viewLayoutSize.width(), contentWidth, minimumScale))
+            minimumScale = m_viewLayoutSize.width() / contentWidth;
+    }
 
     double contentHeight = m_contentSize.height();
-    if (contentHeight > 0 && contentHeight * minimumScale < m_viewLayoutSize.height() && !shouldIgnoreHorizontalScalingConstraints())
-        minimumScale = m_viewLayoutSize.height() / contentHeight;
+    if (contentHeight > 0 && contentHeight * minimumScale < m_viewLayoutSize.height() && !shouldIgnoreHorizontalScalingConstraints()) {
+        if (!scaleForFittingContentIsApproximatelyEqualToMinimumScale(m_viewLayoutSize.height(), contentHeight, minimumScale))
+            minimumScale = m_viewLayoutSize.height() / contentHeight;
+    }
 
     minimumScale = std::min(std::max(minimumScale, m_configuration.minimumScale), m_configuration.maximumScale);
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to