Bug 200907

Summary: Do not adjust viewport if editing selection is already visible
Product: WebKit Reporter: Megan Gardner <megan_gardner>
Component: New BugsAssignee: Megan Gardner <megan_gardner>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, rniwa, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews100 for mac-highsierra
none
Archive of layout-test-results from ews115 for mac-highsierra
none
Archive of layout-test-results from ews214 for win-future
none
Patch
none
Patch
none
Patch
none
Patch for landing none

Description Megan Gardner 2019-08-19 19:26:32 PDT
Do not adjust viewport if editing selection is already visible
Comment 1 Megan Gardner 2019-08-19 19:33:50 PDT
Created attachment 376733 [details]
Patch
Comment 2 EWS Watchlist 2019-08-19 20:39:32 PDT
Comment on attachment 376733 [details]
Patch

Attachment 376733 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12944127

New failing tests:
fast/events/autoscroll-input-when-very-zoomed.html
Comment 3 EWS Watchlist 2019-08-19 20:39:34 PDT
Created attachment 376739 [details]
Archive of layout-test-results from ews100 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 4 EWS Watchlist 2019-08-19 21:25:32 PDT
Comment on attachment 376733 [details]
Patch

Attachment 376733 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12944206

New failing tests:
fast/events/autoscroll-input-when-very-zoomed.html
Comment 5 EWS Watchlist 2019-08-19 21:25:33 PDT
Created attachment 376741 [details]
Archive of layout-test-results from ews115 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 6 EWS Watchlist 2019-08-19 21:33:25 PDT
Comment on attachment 376733 [details]
Patch

Attachment 376733 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12944300

New failing tests:
fast/events/autoscroll-input-when-very-zoomed.html
Comment 7 EWS Watchlist 2019-08-19 21:33:28 PDT
Created attachment 376742 [details]
Archive of layout-test-results from ews214 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews214  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 8 Megan Gardner 2019-08-20 15:41:07 PDT
Created attachment 376822 [details]
Patch
Comment 9 Megan Gardner 2019-08-21 13:00:49 PDT
Created attachment 376908 [details]
Patch
Comment 10 Megan Gardner 2019-08-21 14:48:26 PDT
Created attachment 376924 [details]
Patch
Comment 11 Simon Fraser (smfr) 2019-08-21 16:05:55 PDT
Comment on attachment 376924 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=376924&action=review

> Source/WebCore/ChangeLog:11
> +        Currently due to scrolling being mostly handled by integers, we are getting

"scrolling being mostly handled by integers" -> scrollPositions being integral

> Source/WebCore/ChangeLog:14
> +        start dealing with scrolling with floats/doubles, but until such time,

to make scrollPositions be LayoutUnits or floating point

> Source/WebCore/rendering/RenderLayer.cpp:2704
> +            // This will likely be unnecessary once we convert scrolling to work with floats/doubles instead of ints

I would change this comment to "Avoid scrolling to the rounded value of revealRect.location() if we don't actually need to scroll"

> LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed.html:25
> +

Remove the blank line

> LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed.html:28
> +        await UIHelper.delayFor(200);

What's the reason for the delay?

> LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed.html:38
> +        // Everything isn't quite set yet if we don't delay

That's a bit vague. What are we waiting for?

> LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed.html:42
> +
> +

Two blank lines.

> LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed.html:52
> +                output += 'FAIL: page has scrolled on the secont input';

secont

> LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed.html:58
> +        await UIHelper.immediateZoomToScale(1.0);
> +        await UIHelper.immediateScrollTo(0, 0);

These shouldn't be necessary.

> LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed.html:81
> +This test focuses a form, them zoomes and scrolls the page.

zoomes

> LayoutTests/fast/scrolling/ios/autoscroll-input-when-very-zoomed.html:85
> +<div id="result"></div>

This seems unused
Comment 12 Megan Gardner 2019-08-21 16:29:14 PDT
Created attachment 376947 [details]
Patch for landing
Comment 13 WebKit Commit Bot 2019-08-21 17:06:44 PDT
Comment on attachment 376947 [details]
Patch for landing

Clearing flags on attachment: 376947

Committed r248977: <https://trac.webkit.org/changeset/248977>
Comment 14 WebKit Commit Bot 2019-08-21 17:06:46 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Radar WebKit Bug Importer 2019-08-21 17:08:35 PDT
<rdar://problem/54579095>