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.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Megan Gardner]

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.

[Megan Gardner]

Created attachment 449141 [details]
Patch

[Megan Gardner]

rdar://84440841

[Tim Horton]

Comment on attachment 449141 [details]
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.

[Wenson Hsieh]

The failing selection test on iOS (editing/selection/ios/selection-handle-clamping-in-iframe.html) seems legit.

Perhaps we should be applying clamping logic before mapping to content coordinates? (which, in the case of the test, is in the coordinate space of a subframe).

[Megan Gardner]

Created attachment 449212 [details]
Patch

[Wenson Hsieh]

Comment on attachment 449212 [details]
Patch

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

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

All codepaths that call into this function happen as a result of `-changeSelectionWithTouchAt:withSelectionTouch:baseIsStart:withFlags:` in the UI process, so I *suspect* this should be okay.

> Source/WebCore/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.

Nit - "seleciton" is still misspelled here.

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

(Ditto)

> 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.

(Ditto)

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

(Ditto)

[Megan Gardner]

Created attachment 449423 [details]
Patch

[Megan Gardner]

Created attachment 449428 [details]
Patch

[Megan Gardner]

Created attachment 449458 [details]
Patch for landing

[EWS]

Committed r288180 (246161@main): <https://commits.webkit.org/246161@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449458 [details].