[webkit-reviews] review granted: [Bug 200907] Do not adjust viewport if editing selection is already visible : [Attachment 376924] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Megan Gardner
<megan_gardner at apple.com>'s request for review:
Bug 200907: Do not adjust viewport if editing selection is already visible
https://bugs.webkit.org/show_bug.cgi?id=200907

Attachment 376924: Patch

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




--- Comment #11 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 376924
  --> https://bugs.webkit.org/attachment.cgi?id=376924
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