[webkit-reviews] review granted: [Bug 235220] Loupe sometimes flips to the bottom of the page when dragging the end of a selection to the top of a page with seleciton flipping. : [Attachment 449141] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Megan Gardner
<megan_gardner at apple.com>'s request for review:
Bug 235220: Loupe sometimes flips to the bottom of the page when dragging the
end of a selection to the top of a page with seleciton flipping.
https://bugs.webkit.org/show_bug.cgi?id=235220

Attachment 449141: Patch

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




--- Comment #3 from Tim Horton <thorton at apple.com> ---
Comment on attachment 449141
  --> https://bugs.webkit.org/attachment.cgi?id=449141
Patch

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

> Source/WebKit/ChangeLog:3
> +	   Loupe sometimes flips to the bottom of the page when dragging the
end of a selection to the top of a page with seleciton flipping.

`seleciton`

> Source/WebKit/ChangeLog:12
> +	   in the loupe being pushed to the bottom of the page (because the
seleciton was on the previous page

`seleciton`

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1507
> +    auto pointInDocument =
frame.view()->rootViewToContents(pointInRootViewCoordinates).constrainedBetween
(frame.view()->unobscuredContentRect().minXMinYCorner(),
frame.view()->unobscuredContentRect().maxXMaxYCorner());

Maybe stash `frame.view()->unobscuredContentRect()` in a local?

Also, is this function ever called with points that aren't from user input?
(and in which case constraining might be wrong?)

Also, can you double-check that autoscrolling still works? I think it will be
OK, but best to check.

Also, is it testable? Seems like it should be.