- Revision
- 230746
- Author
- wenson_hs...@apple.com
- Date
- 2018-04-17 19:50:25 -0700 (Tue, 17 Apr 2018)
Log Message
[Extra zoom mode] Double tap to zoom should account for text legibility in extra zoom mode
https://bugs.webkit.org/show_bug.cgi?id=184631
<rdar://problem/39303706>
Reviewed by Tim Horton.
Source/WebKit:
Implement the text legibility heuristic alluded to in r230506 by iterating through text runs in the document (up
to a maximum of 200) and building a histogram of font sizes that appear in the document, where each tally
represents a character.
The first and second text legibility zoom scales are then computed based on the zoom scales needed to
make 50% and 90% of the text legible, respectively. Here, a zoom scale that makes text legible is such that the
text would have an apparent font size of a hard-coded constant (currently, 12) after zooming. This means the
first and second text legibility scales may end up being close to one another, or even the same (in the case
where there is only a single font size in the entire document). In this case, we just snap the first scale to
the second, so that double tapping will only toggle between two zoom scales. In another case where the document
has no text (e.g. an image document), we just fall back to a zoom scale of 1.
Test: fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html
* WebProcess/WebPage/ViewGestureGeometryCollector.cpp:
(WebKit::ViewGestureGeometryCollector::computeTextLegibilityScales):
LayoutTests:
Add a layout test to check that double tap to zoom works in extra zoom mode, even when text spans the entire
width of the document.
* TestExpectations:
* fast/events/extrazoom/double-tap-to-zoom-on-full-width-text-expected.txt: Added.
* fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html: Added.
* resources/basic-gestures.js:
Add a helper method to double tap at a given location, and wait for zooming to finish.
(return.new.Promise):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (230745 => 230746)
--- trunk/LayoutTests/ChangeLog 2018-04-18 01:03:22 UTC (rev 230745)
+++ trunk/LayoutTests/ChangeLog 2018-04-18 02:50:25 UTC (rev 230746)
@@ -1,3 +1,23 @@
+2018-04-17 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [Extra zoom mode] Double tap to zoom should account for text legibility in extra zoom mode
+ https://bugs.webkit.org/show_bug.cgi?id=184631
+ <rdar://problem/39303706>
+
+ Reviewed by Tim Horton.
+
+ Add a layout test to check that double tap to zoom works in extra zoom mode, even when text spans the entire
+ width of the document.
+
+ * TestExpectations:
+ * fast/events/extrazoom/double-tap-to-zoom-on-full-width-text-expected.txt: Added.
+ * fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html: Added.
+ * resources/basic-gestures.js:
+
+ Add a helper method to double tap at a given location, and wait for zooming to finish.
+
+ (return.new.Promise):
+
2018-04-17 Tadeu Zagallo <tzaga...@apple.com>
Retain MessagePortChannel for transfer when disentangling ports
Modified: trunk/LayoutTests/TestExpectations (230745 => 230746)
--- trunk/LayoutTests/TestExpectations 2018-04-18 01:03:22 UTC (rev 230745)
+++ trunk/LayoutTests/TestExpectations 2018-04-18 02:50:25 UTC (rev 230746)
@@ -25,6 +25,7 @@
fast/viewport/ios [ Skip ]
fast/visual-viewport/ios/ [ Skip ]
fast/events/ios [ Skip ]
+fast/events/extrazoom [ Skip ]
fast/events/touch/ios [ Skip ]
fast/history/ios [ Skip ]
fast/scrolling/ios [ Skip ]
Added: trunk/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text-expected.txt (0 => 230746)
--- trunk/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text-expected.txt 2018-04-18 02:50:25 UTC (rev 230746)
@@ -0,0 +1,5 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Even though this text is laid out at the full document width, we should still be able to double tap to zoom in "extra zoom mode", such that the text becomes legible.
+Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
Added: trunk/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html (0 => 230746)
--- trunk/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html (rev 0)
+++ trunk/LayoutTests/fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html 2018-04-18 02:50:25 UTC (rev 230746)
@@ -0,0 +1,25 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<script src=""
+<script src=""
+<meta name="viewport" content="width=device-width">
+<body style="margin: 0; font-size: 10px;">
+ <div>
+ Even though this text is laid out at the full document width, we should still
+ be able to double tap to zoom in "extra zoom mode", such that the text becomes
+ legible.
+ </div>
+ <div>
+ Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor
+ incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis
+ nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
+ Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu
+ fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in
+ culpa qui officia deserunt mollit anim id est laborum.
+ </div>
+</body>
+<script>
+ jsTestIsAsync = true;
+ doubleTapToZoomAtPoint(75, 75).then(finishJSTest);
+</script>
+</html>
Modified: trunk/LayoutTests/resources/basic-gestures.js (230745 => 230746)
--- trunk/LayoutTests/resources/basic-gestures.js 2018-04-18 01:03:22 UTC (rev 230745)
+++ trunk/LayoutTests/resources/basic-gestures.js 2018-04-18 02:50:25 UTC (rev 230746)
@@ -10,6 +10,18 @@
});
}
+function doubleTapToZoomAtPoint(x, y)
+{
+ return new Promise(resolve => {
+ testRunner.runUIScript(`
+ (function() {
+ uiController.didEndZoomingCallback = () => uiController.uiScriptComplete();
+ uiController.singleTapAtPoint(${x}, ${y}, () => { });
+ uiController.singleTapAtPoint(${x}, ${y}, () => { });
+ })();`, resolve);
+ });
+}
+
function longPressAtPoint(x, y)
{
return new Promise(resolve => {
Modified: trunk/Source/WebKit/ChangeLog (230745 => 230746)
--- trunk/Source/WebKit/ChangeLog 2018-04-18 01:03:22 UTC (rev 230745)
+++ trunk/Source/WebKit/ChangeLog 2018-04-18 02:50:25 UTC (rev 230746)
@@ -1,3 +1,28 @@
+2018-04-17 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [Extra zoom mode] Double tap to zoom should account for text legibility in extra zoom mode
+ https://bugs.webkit.org/show_bug.cgi?id=184631
+ <rdar://problem/39303706>
+
+ Reviewed by Tim Horton.
+
+ Implement the text legibility heuristic alluded to in r230506 by iterating through text runs in the document (up
+ to a maximum of 200) and building a histogram of font sizes that appear in the document, where each tally
+ represents a character.
+
+ The first and second text legibility zoom scales are then computed based on the zoom scales needed to
+ make 50% and 90% of the text legible, respectively. Here, a zoom scale that makes text legible is such that the
+ text would have an apparent font size of a hard-coded constant (currently, 12) after zooming. This means the
+ first and second text legibility scales may end up being close to one another, or even the same (in the case
+ where there is only a single font size in the entire document). In this case, we just snap the first scale to
+ the second, so that double tapping will only toggle between two zoom scales. In another case where the document
+ has no text (e.g. an image document), we just fall back to a zoom scale of 1.
+
+ Test: fast/events/extrazoom/double-tap-to-zoom-on-full-width-text.html
+
+ * WebProcess/WebPage/ViewGestureGeometryCollector.cpp:
+ (WebKit::ViewGestureGeometryCollector::computeTextLegibilityScales):
+
2018-04-17 Megan Gardner <megan_gard...@apple.com>
Don't activate selection on become first responder
Modified: trunk/Source/WebKit/WebProcess/WebPage/ViewGestureGeometryCollector.cpp (230745 => 230746)
--- trunk/Source/WebKit/WebProcess/WebPage/ViewGestureGeometryCollector.cpp 2018-04-18 01:03:22 UTC (rev 230745)
+++ trunk/Source/WebKit/WebProcess/WebPage/ViewGestureGeometryCollector.cpp 2018-04-18 02:50:25 UTC (rev 230746)
@@ -26,17 +26,21 @@
#include "config.h"
#include "ViewGestureGeometryCollector.h"
+#include "Logging.h"
#include "ViewGestureGeometryCollectorMessages.h"
#include "WebCoreArgumentCoders.h"
#include "WebFrame.h"
#include "WebPage.h"
#include "WebProcess.h"
+#include <WebCore/FontCascade.h>
#include <WebCore/Frame.h>
#include <WebCore/FrameView.h>
#include <WebCore/HTMLImageElement.h>
+#include <WebCore/HTMLTextFormControlElement.h>
#include <WebCore/HitTestResult.h>
#include <WebCore/ImageDocument.h>
#include <WebCore/RenderView.h>
+#include <WebCore/TextIterator.h>
#if PLATFORM(IOS)
#include "SmartMagnificationControllerMessages.h"
@@ -128,10 +132,22 @@
#if PLATFORM(IOS)
+struct FontSizeAndCount {
+ unsigned fontSize;
+ unsigned count;
+};
+
std::optional<std::pair<double, double>> ViewGestureGeometryCollector::computeTextLegibilityScales(double& viewportMinimumScale, double& viewportMaximumScale)
{
- static const double defaultMaximumTextLegibilityScale = 1;
+ static const unsigned fontSizeBinningInterval = 2;
+ 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;
+
computeMinimumAndMaximumViewportScales(viewportMinimumScale, viewportMaximumScale);
if (m_cachedTextLegibilityScales)
return m_cachedTextLegibilityScales;
@@ -142,11 +158,65 @@
document->updateLayoutIgnorePendingStylesheets();
- // FIXME: Determine appropriate text legibility scales by examining text runs in the document. For now, hard code the second text legibility scale to be 1,
- // and set the first text legibility scale to be the halfway point between the initial scale and 1.
- double firstTextLegibilityScale = clampTo<double>((m_webPage.viewportConfiguration().initialScale() + defaultMaximumTextLegibilityScale) / 2, viewportMinimumScale, viewportMaximumScale);
- double secondTextLegibilityScale = clampTo<double>(defaultMaximumTextLegibilityScale, viewportMinimumScale, viewportMaximumScale);
+ auto documentRange = Range::create(*document, {{ document->documentElement(), Position::PositionIsBeforeAnchor }}, {{ document->documentElement(), Position::PositionIsAfterAnchor }});
+ HashSet<Node*> allTextNodes;
+ HashMap<unsigned, unsigned> fontSizeToCountMap;
+ unsigned numberOfIterations = 0;
+ unsigned totalSampledTextLength = 0;
+ for (TextIterator documentTextIterator { documentRange.ptr(), TextIteratorEntersTextControls }; !documentTextIterator.atEnd(); documentTextIterator.advance()) {
+ if (++numberOfIterations >= maximumNumberOfTextRunsToConsider)
+ break;
+
+ if (!is<Text>(documentTextIterator.node()))
+ continue;
+
+ auto& textNode = downcast<Text>(*documentTextIterator.node());
+ auto textLength = textNode.length();
+ if (!textLength || !textNode.renderer() || allTextNodes.contains(&textNode))
+ continue;
+
+ allTextNodes.add(&textNode);
+
+ unsigned fontSizeBin = fontSizeBinningInterval * round(textNode.renderer()->style().fontCascade().size() / fontSizeBinningInterval);
+ auto entry = fontSizeToCountMap.find(fontSizeBin);
+ fontSizeToCountMap.set(fontSizeBin, textLength + (entry == fontSizeToCountMap.end() ? 0 : entry->value));
+ totalSampledTextLength += textLength;
+ }
+
+ Vector<FontSizeAndCount> sortedFontSizesAndCounts;
+ sortedFontSizesAndCounts.reserveCapacity(fontSizeToCountMap.size());
+ for (auto& entry : fontSizeToCountMap)
+ sortedFontSizesAndCounts.append({ entry.key, entry.value });
+
+ std::sort(sortedFontSizesAndCounts.begin(), sortedFontSizesAndCounts.end(), [] (auto& first, auto& second) {
+ return first.fontSize < second.fontSize;
+ });
+
+ double firstTextLegibilityScale = 0;
+ double secondTextLegibilityScale = 0;
+ 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 (sortedFontSizesAndCounts.isEmpty()) {
+ firstTextLegibilityScale = fallbackTextLegibilityScale;
+ secondTextLegibilityScale = fallbackTextLegibilityScale;
+ } else if (secondTextLegibilityScale - firstTextLegibilityScale < minimumDifferenceBetweenTextLegibilityScales)
+ 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 }};
return m_cachedTextLegibilityScales;
}
