Attachment 341827 Details for Bug 186209 – Will CQ process this?

[patch] Will CQ process this?

bug-186209-20180601214630.patch (text/plain), 8.93 KB, created by Wenson Hsieh on 2018-06-01 21:46:31 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2018-06-01 21:46:31 PDT

Size: 8.93 KB

patch

obsolete

>Subversion Revision: 232434
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index dc54d31175c09f67b41b677358dd1176ce40bd6c..8348862a82c55ec9e4318c16c1c06c405f2fa1d1 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-06-01  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [Extra zoom mode] The user should always be able to double tap to zoom to a scale of at least 1
>+        https://bugs.webkit.org/show_bug.cgi?id=186209
>+        <rdar://problem/40529255>
>+
>+        Reviewed by Tim Horton.
>+
>+        Tweaks the way double-tap-to-zoom scales are determined in extra zoom mode. Rather than zooming to make the 50th
>+        and 90th percentiles of text in the document legible, only consider the 90th percentile of text size when
>+        determining zoom scale, and fix the other potential zoom scale at 1; additionally, if the zoom scales are close
>+        (within 0.3 of each other), snap the lower zoom scale to the higher value.
>+
>+        This results in the following changes in behavior:
>+        -   Enables double tap to zoom in cases where all the text in the page is already legible.
>+        -   On pages with mobile viewports, usually allows the user to toggle between initial scale and a scale of 1.
>+        -   If a significant portion of text is unusually small, the zoomed-in scale may exceed 1.
>+
>+        Test: fast/events/extrazoom/double-tap-to-zoom-with-large-text.html
>+
>+        * WebProcess/WebPage/ViewGestureGeometryCollector.cpp:
>+        (WebKit::ViewGestureGeometryCollector::collectGeometryForSmartMagnificationGesture):
>+        (WebKit::ViewGestureGeometryCollector::computeTextLegibilityScales):
>+
> 2018-06-01  Jeremy Jones  <jeremyj@apple.com>
> 
>         Keyboard focus should exit fullscreen.
>diff --git a/Source/WebKit/WebProcess/WebPage/ViewGestureGeometryCollector.cpp b/Source/WebKit/WebProcess/WebPage/ViewGestureGeometryCollector.cpp
>index 4e17a8f2bae278960e4e72ed1d20015003d62a4a..5944907d4463402ac23c1c73d22afef71f87b423 100644
>--- a/Source/WebKit/WebProcess/WebPage/ViewGestureGeometryCollector.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/ViewGestureGeometryCollector.cpp
>@@ -54,6 +54,10 @@ using namespace WebCore;
> 
> namespace WebKit {
> 
>+#if PLATFORM(IOS)
>+static const double minimumScaleDifferenceForZooming = 0.3;
>+#endif
>+
> ViewGestureGeometryCollector::ViewGestureGeometryCollector(WebPage& webPage)
>     : m_webPage(webPage)
> #if PLATFORM(MAC)
>@@ -90,8 +94,6 @@ void ViewGestureGeometryCollector::collectGeometryForSmartMagnificationGesture(F
> 
> #if PLATFORM(IOS)
>     if (m_webPage.platformPrefersTextLegibilityBasedZoomScaling()) {
>-        static const double minimumScaleDifferenceForZooming = 0.05;
>-
>         auto textLegibilityScales = computeTextLegibilityScales(viewportMinimumScale, viewportMaximumScale);
>         if (!textLegibilityScales) {
>             dispatchDidCollectGeometryForSmartMagnificationGesture({ }, { }, { }, false, 0, 0);
>@@ -143,10 +145,8 @@ std::optional<std::pair<double, double>> ViewGestureGeometryCollector::computeTe
>     static const double maximumNumberOfTextRunsToConsider = 200;
> 
>     static const double targetLegibilityFontSize = 12;
>-    static const double firstTextLegibilityScaleRatio = 0.5;
>-    static const double secondTextLegibilityScaleRatio = 0.1;
>-    static const double minimumDifferenceBetweenTextLegibilityScales = 0.2;
>-    static const double fallbackTextLegibilityScale = 1;
>+    static const double textLegibilityScaleRatio = 0.1;
>+    static const double defaultTextLegibilityZoomScale = 1;
> 
>     computeMinimumAndMaximumViewportScales(viewportMinimumScale, viewportMaximumScale);
>     if (m_cachedTextLegibilityScales)
>@@ -193,31 +193,24 @@ std::optional<std::pair<double, double>> ViewGestureGeometryCollector::computeTe
>         return first.fontSize < second.fontSize;
>     });
> 
>-    double firstTextLegibilityScale = 0;
>-    double secondTextLegibilityScale = 0;
>+    double defaultScale = clampTo<double>(defaultTextLegibilityZoomScale, viewportMinimumScale, viewportMaximumScale);
>+    double textLegibilityScale = defaultScale;
>     double currentSampledTextLength = 0;
>     for (auto& fontSizeAndCount : sortedFontSizesAndCounts) {
>         currentSampledTextLength += fontSizeAndCount.count;
>         double ratioOfTextUnderCurrentFontSize = currentSampledTextLength / totalSampledTextLength;
>-        LOG(ViewGestures, "About %.2f%% of text is smaller than font size %tu", ratioOfTextUnderCurrentFontSize * 100, fontSizeAndCount.fontSize);
>-        if (!firstTextLegibilityScale && ratioOfTextUnderCurrentFontSize >= firstTextLegibilityScaleRatio)
>-            firstTextLegibilityScale = targetLegibilityFontSize / fontSizeAndCount.fontSize;
>-        if (!secondTextLegibilityScale && ratioOfTextUnderCurrentFontSize >= secondTextLegibilityScaleRatio)
>-            secondTextLegibilityScale = targetLegibilityFontSize / fontSizeAndCount.fontSize;
>+        if (ratioOfTextUnderCurrentFontSize >= textLegibilityScaleRatio) {
>+            textLegibilityScale = clampTo<double>(targetLegibilityFontSize / fontSizeAndCount.fontSize, viewportMinimumScale, viewportMaximumScale);
>+            break;
>+        }
>     }
> 
>-    if (sortedFontSizesAndCounts.isEmpty()) {
>-        firstTextLegibilityScale = fallbackTextLegibilityScale;
>-        secondTextLegibilityScale = fallbackTextLegibilityScale;
>-    } else if (secondTextLegibilityScale - firstTextLegibilityScale < minimumDifferenceBetweenTextLegibilityScales)
>+    auto firstTextLegibilityScale = std::min<double>(textLegibilityScale, defaultScale);
>+    auto secondTextLegibilityScale = std::max<double>(textLegibilityScale, defaultScale);
>+    if (secondTextLegibilityScale - firstTextLegibilityScale < minimumScaleDifferenceForZooming)
>         firstTextLegibilityScale = secondTextLegibilityScale;
> 
>-    secondTextLegibilityScale = clampTo<double>(secondTextLegibilityScale, viewportMinimumScale, viewportMaximumScale);
>-    firstTextLegibilityScale = clampTo<double>(firstTextLegibilityScale, viewportMinimumScale, viewportMaximumScale);
>-
>-    LOG(ViewGestures, "The computed text legibility scales are: (%.2f, %.2f)", firstTextLegibilityScale, secondTextLegibilityScale);
>-
>-    m_cachedTextLegibilityScales = std::optional<std::pair<double, double>> {{ firstTextLegibilityScale, secondTextLegibilityScale }};
>+    m_cachedTextLegibilityScales.emplace(std::pair<double, double> { firstTextLegibilityScale, secondTextLegibilityScale });
>     return m_cachedTextLegibilityScales;
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index e5016d48ace75dadf084f9d0c507944ef7a05183..bcdf769fde486ffb014888188f1f9f9b8d8d20e8 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-06-01  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [Extra zoom mode] The user should always be able to double tap to zoom to a scale of at least 1
>+        https://bugs.webkit.org/show_bug.cgi?id=186209
>+        <rdar://problem/40529255>
>+
>+        Reviewed by Tim Horton.
>+
>+        Add a test to verify that double tapping zooms in on a page where all the text is large enough to be legible at
>+        initial scale.
>+
>+        * fast/events/extrazoom/double-tap-to-zoom-with-large-text-expected.txt: Added.
>+        * fast/events/extrazoom/double-tap-to-zoom-with-large-text.html: Added.
>+
> 2018-06-01  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         Editor can hold references to Documents after you navigate away
>diff --git a/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-with-large-text-expected.txt b/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-with-large-text-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..6d16bd5fb00f2fe1cf81cb6d685e193afc7615aa
>--- /dev/null
>+++ b/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-with-large-text-expected.txt
>@@ -0,0 +1,4 @@
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+Even though this text is large enough to be legible at initial scale, we should still allow double tapping to zoom for consistent behavior.
>diff --git a/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-with-large-text.html b/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-with-large-text.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..260df70dcf7c89b2bdc7691406c59cbcf0ddbf7e
>--- /dev/null
>+++ b/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-with-large-text.html
>@@ -0,0 +1,17 @@
>+<!DOCTYPE html> 
>+<html>
>+<script src="../../../resources/js-test.js"></script>
>+<script src="../../../resources/basic-gestures.js"></script>
>+<meta name="viewport" content="width=device-width">
>+<body style="margin: 0;">
>+    <div style="background-color: red; width: 150px; height: 150px;"></div>
>+    <h1 style="font-size: 2em;">
>+        Even though this text is large enough to be legible at initial scale, we should
>+        still allow double tapping to zoom for consistent behavior.
>+    </h1>
>+</body>
>+<script>
>+    jsTestIsAsync = true;
>+    doubleTapToZoomAtPoint(75, 75).then(finishJSTest);
>+</script>
>+</html>

Flags:

wenson_hsieh: commit-queue+

Actions: View | Formatted Diff | Diff

Attachments on bug 186209: 341797 | 341827