- Revision
- 248977
- Author
- megan_gard...@apple.com
- Date
- 2019-08-21 17:06:38 -0700 (Wed, 21 Aug 2019)
Log Message
Do not adjust viewport if editing selection is already visible
https://bugs.webkit.org/show_bug.cgi?id=200907
<rdar://problem/53903417>
Reviewed by Simon Fraser.
Source/WebCore:
Test: fast/scrolling/ios/autoscroll-input-when-very-zoomed.html
Currently due to scrolling being mostly handled by integers, we are getting
issues with rounding errors when trying to adjust the viewport while
editing text when we are significantly zoomed in. The real fix would be to
start dealing with scrolling with floats/doubles, but until such time,
we should early out of adjusting selections that we are certain are currently
visible.
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollRectToVisible):
LayoutTests:
* fast/scrolling/ios/autoscroll-input-when-very-zoomed-expected.txt: Added.
* fast/scrolling/ios/autoscroll-input-when-very-zoomed.html: Added.
* resources/ui-helper.js:
(window.UIHelper.immediateZoomToScale):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (248976 => 248977)
--- trunk/LayoutTests/ChangeLog 2019-08-21 23:56:24 UTC (rev 248976)
+++ trunk/LayoutTests/ChangeLog 2019-08-22 00:06:38 UTC (rev 248977)
@@ -1,3 +1,16 @@
+2019-08-21 Megan Gardner <megan_gard...@apple.com>
+
+ Do not adjust viewport if editing selection is already visible
+ https://bugs.webkit.org/show_bug.cgi?id=200907
+ <rdar://problem/53903417>
+
+ Reviewed by Simon Fraser.
+
+ * fast/scrolling/ios/autoscroll-input-when-very-zoomed-expected.txt: Added.
+ * fast/scrolling/ios/autoscroll-input-when-very-zoomed.html: Added.
+ * resources/ui-helper.js:
+ (window.UIHelper.immediateZoomToScale):
+
2019-08-21 Tim Horton <timothy_hor...@apple.com>
[Mail] Tapping top of message scrolls back to copied text instead of top of the message
Added: trunk/LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed-expected.txt (0 => 248977)
--- trunk/LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed-expected.txt 2019-08-22 00:06:38 UTC (rev 248977)
@@ -0,0 +1,3 @@
+This test focuses a form, them zooms and scrolls the page. Then text is entered in the form, and we check to make sure the page has scrolled so that the input is visible again, but not more that once, as it should be visible after the first scroll.
+PASS: page has scrolled when the selection is not visible, and not scrolled when the selection is already visible.
+
Added: trunk/LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed.html (0 => 248977)
--- trunk/LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed.html 2019-08-22 00:06:38 UTC (rev 248977)
@@ -0,0 +1,89 @@
+<!DOCTYPE html><!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<script src=""
+<script src=""
+<script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+
+ const pageScale = 7.5;
+
+ async function runTest()
+ {
+ if (!testRunner.runUIScript)
+ return;
+
+ var output = '';
+
+ await UIHelper.setHardwareKeyboardAttached(false);
+ await UIHelper.activateElementAndWaitForInputSession(document.getElementById('editable'));
+
+ await UIHelper.immediateZoomToScale(3.284);
+ await UIHelper.immediateScrollTo(390, 390);
+
+ await UIHelper.delayFor(200);
+
+ var originalXOffset = window.pageXOffset;
+ var originalYOffset = window.pageYOffset;
+
+ await UIHelper.enterText(" ");
+
+ var secondXOffset = window.pageXOffset;
+ var secondYOffset = window.pageYOffset;
+
+ // Scrolling is not immedate, wait until the viewport has time to adjust
+ await UIHelper.delayFor(25);
+ await UIHelper.enterText("a");
+
+ var finalXOffset = window.pageXOffset;
+ var finalYOffset = window.pageYOffset;
+
+ if ((originalXOffset != secondXOffset) && (secondXOffset == finalXOffset) && (originalYOffset != secondYOffset) && (secondYOffset == finalYOffset))
+ output += 'PASS: page has scrolled when the selection is not visible, and not scrolled when the selection is already visible.';
+ else {
+ if ((originalXOffset == secondXOffset) || (originalYOffset == secondYOffset))
+ output += 'FAIL: page has failed to scrolled on the first input<br>';
+ if ((secondXOffset != finalXOffset) || (secondYOffset != finalYOffset))
+ output += 'FAIL: page has scrolled on the secont input';
+ }
+ output += '<br>';
+
+ document.getElementById('testArea').innerHTML = output;
+ await UIHelper.immediateZoomToScale(1.0);
+ await UIHelper.immediateScrollTo(0, 0);
+ testRunner.notifyDone();
+ }
+
+ window.addEventListener('load', runTest, false);
+</script>
+<style>
+ #testArea {
+ height: 2000px;
+ width: 300px;
+ background-color: silver;
+ font-family: monospace;
+ font-size: 18px;
+ }
+ #editable {
+ font-family: monospace;
+ font-size: 18px;
+ margin-top: 100px;
+ margin-left: 100px;
+ }
+</style>
+</head>
+<body>
+This test focuses a form, them zooms and scrolls the page.
+Then text is entered in the form, and we check to make sure the page has scrolled
+so that the input is visible again, but not more that once, as it should be visible
+after the first scroll.
+<div id="testArea"><input id="editable" type="text" value="Test text"></input></div>
+</body>
+</html>
+
+
+
+
Modified: trunk/LayoutTests/resources/ui-helper.js (248976 => 248977)
--- trunk/LayoutTests/resources/ui-helper.js 2019-08-21 23:56:24 UTC (rev 248976)
+++ trunk/LayoutTests/resources/ui-helper.js 2019-08-22 00:06:38 UTC (rev 248977)
@@ -708,6 +708,12 @@
return new Promise(resolve => testRunner.runUIScript(uiScript, resolve));
}
+ static immediateZoomToScale(scale)
+ {
+ const uiScript = `uiController.immediateZoomToScale(${scale})`;
+ return new Promise(resolve => testRunner.runUIScript(uiScript, resolve));
+ }
+
static typeCharacter(characterString)
{
if (!this.isWebKit2() || !this.isIOSFamily()) {
Modified: trunk/Source/WebCore/ChangeLog (248976 => 248977)
--- trunk/Source/WebCore/ChangeLog 2019-08-21 23:56:24 UTC (rev 248976)
+++ trunk/Source/WebCore/ChangeLog 2019-08-22 00:06:38 UTC (rev 248977)
@@ -1,3 +1,23 @@
+2019-08-21 Megan Gardner <megan_gard...@apple.com>
+
+ Do not adjust viewport if editing selection is already visible
+ https://bugs.webkit.org/show_bug.cgi?id=200907
+ <rdar://problem/53903417>
+
+ Reviewed by Simon Fraser.
+
+ Test: fast/scrolling/ios/autoscroll-input-when-very-zoomed.html
+
+ Currently due to scrolling being mostly handled by integers, we are getting
+ issues with rounding errors when trying to adjust the viewport while
+ editing text when we are significantly zoomed in. The real fix would be to
+ start dealing with scrolling with floats/doubles, but until such time,
+ we should early out of adjusting selections that we are certain are currently
+ visible.
+
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::scrollRectToVisible):
+
2019-08-21 Tim Horton <timothy_hor...@apple.com>
[Mail] Tapping top of message scrolls back to copied text instead of top of the message
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (248976 => 248977)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2019-08-21 23:56:24 UTC (rev 248976)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2019-08-22 00:06:38 UTC (rev 248977)
@@ -2701,8 +2701,11 @@
targetRect.move(0, frameView.headerHeight());
LayoutRect revealRect = getRectToExpose(viewRect, targetRect, insideFixed, options.alignX, options.alignY);
- ScrollOffset clampedScrollPosition = roundedIntPoint(revealRect.location()).constrainedBetween(minScrollPosition, maxScrollPosition);
- frameView.setScrollPosition(clampedScrollPosition);
+ // Avoid scrolling to the rounded value of revealRect.location() if we don't actually need to scroll
+ if (revealRect != viewRect) {
+ ScrollOffset clampedScrollPosition = roundedIntPoint(revealRect.location()).constrainedBetween(minScrollPosition, maxScrollPosition);
+ frameView.setScrollPosition(clampedScrollPosition);
+ }
// This is the outermost view of a web page, so after scrolling this view we
// scroll its container by calling Page::scrollRectIntoView.
