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.
Summary: Loupe sometimes flips to the bottom of the page when dragging the end of a se...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Megan Gardner
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-01-13 22:27 PST by Megan Gardner
Modified: 2022-01-18 21:14 PST (History)
2 users (show)

See Also:


Attachments
Patch (2.41 KB, patch)
2022-01-13 22:40 PST, Megan Gardner
no flags Details | Formatted Diff | Diff
Patch (7.01 KB, patch)
2022-01-14 13:11 PST, Megan Gardner
no flags Details | Formatted Diff | Diff
Patch (13.25 KB, patch)
2022-01-18 14:20 PST, Megan Gardner
no flags Details | Formatted Diff | Diff
Patch (13.15 KB, patch)
2022-01-18 14:48 PST, Megan Gardner
no flags Details | Formatted Diff | Diff
Patch for landing (13.14 KB, patch)
2022-01-18 20:12 PST, Megan Gardner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Megan Gardner 2022-01-13 22:27:33 PST
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.
Comment 1 Megan Gardner 2022-01-13 22:40:46 PST
Created attachment 449141 [details]
Patch
Comment 2 Megan Gardner 2022-01-13 22:43:22 PST
rdar://84440841
Comment 3 Tim Horton 2022-01-14 01:19:45 PST
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.
Comment 4 Wenson Hsieh 2022-01-14 08:33:04 PST
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).
Comment 5 Megan Gardner 2022-01-14 13:11:35 PST
Created attachment 449212 [details]
Patch
Comment 6 Wenson Hsieh 2022-01-14 15:31:01 PST
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)
Comment 7 Megan Gardner 2022-01-18 14:20:52 PST
Created attachment 449423 [details]
Patch
Comment 8 Megan Gardner 2022-01-18 14:48:51 PST
Created attachment 449428 [details]
Patch
Comment 9 Megan Gardner 2022-01-18 20:12:54 PST
Created attachment 449458 [details]
Patch for landing
Comment 10 EWS 2022-01-18 21:14:49 PST
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].