Bug 188826

Summary: Change Selection modification to not snap the grabber when selecting above or below the selection anchor
Product: WebKit Reporter: Megan Gardner <megan_gardner>
Component: New BugsAssignee: Megan Gardner <megan_gardner>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bdakin, commit-queue, rniwa, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Megan Gardner
Reported 2018-08-21 16:29:06 PDT
Change Selection modification to not snap the grabber when selecting above or below the selection anchor
Attachments
Patch (6.85 KB, patch)
2018-08-21 16:37 PDT, Megan Gardner 		no flags
DetailsFormatted DiffDiff
Patch for landing (6.34 KB, patch)
2018-08-21 17:37 PDT, Megan Gardner 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Megan Gardner
Comment 1 2018-08-21 16:37:06 PDT
Created attachment 347724 [details] Patch
Radar WebKit Bug Importer
Comment 2 2018-08-21 16:38:20 PDT
<rdar://problem/43583165>
Tim Horton
Comment 3 2018-08-21 17:15:25 PDT
Comment on attachment 347724 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347724&action=review > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1224 > + IntPoint adjustedPoint(pointInRootViewCoordinates.x(), pointInRootViewCoordinates.y()); Why not just `IntPoint adjustedPoint = pointInRootViewCoordinates`? > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1228 > + int startY = caret.y() + caret.height() / 2; We have center(), maybe use that?
Wenson Hsieh
Comment 4 2018-08-21 17:19:54 PDT
Comment on attachment 347724 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347724&action=review > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1242 > + HitTestResult hitTest = frame.eventHandler().hitTestResultAtPoint(adjustedPoint, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent); adjustedPoint is in root view coordinates, no?
Tim Horton
Comment 5 2018-08-21 17:22:02 PDT
Comment on attachment 347724 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347724&action=review >> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1242 >> + HitTestResult hitTest = frame.eventHandler().hitTestResultAtPoint(adjustedPoint, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent); > > adjustedPoint is in root view coordinates, no? Oops.
Megan Gardner
Comment 6 2018-08-21 17:37:34 PDT
Created attachment 347741 [details] Patch for landing
WebKit Commit Bot
Comment 7 2018-08-21 18:41:56 PDT
Comment on attachment 347741 [details] Patch for landing Clearing flags on attachment: 347741 Committed r235153: <https://trac.webkit.org/changeset/235153>
WebKit Commit Bot
Comment 8 2018-08-21 18:41:58 PDT
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 9 2018-08-23 09:55:50 PDT
Ryan is fixing layout tests in bug 188888. In the future, please wait for EWS before landing patches.
Wenson Hsieh
Comment 10 2018-09-08 15:22:07 PDT
Comment on attachment 347741 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=347741&action=review > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1230 > + adjustedPoint.setY(startY); It looks like the caret bounds are in content coordinates, but adjusted point is in root view coordinates? > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1235 > + adjustedPoint.setY(endY); (Here too)
Note You need to log in before you can comment on or make changes to this bug.