Bug 213704 - [iOS] Implement support for UIWKDocumentRequestSpatialAndCurrentSelection
Summary: [iOS] Implement support for UIWKDocumentRequestSpatialAndCurrentSelection
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar, PlatformOnly
Depends on:
Blocks:
 
Reported: 2020-06-28 14:52 PDT by Daniel Bates
Modified: 2020-07-01 10:08 PDT (History)
2 users (show)

See Also:


Attachments
Patch (12.65 KB, patch)
2020-06-28 15:06 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (12.62 KB, patch)
2020-06-28 15:09 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (13.26 KB, patch)
2020-06-28 17:00 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (15.55 KB, patch)
2020-06-30 10:52 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (16.46 KB, patch)
2020-06-30 12:28 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
To Land (16.18 KB, patch)
2020-07-01 09:52 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2020-06-28 14:52:42 PDT
Implement support in WebKit for the UIWKDocumentRequestSpatialAndCurrentSelection flag I added to UIKit in <rdar://problem/64867540>.
Comment 1 Daniel Bates 2020-06-28 14:52:50 PDT
<rdar://problem/59738878>
Comment 2 Daniel Bates 2020-06-28 15:06:12 PDT
Created attachment 403011 [details]
Patch
Comment 3 Daniel Bates 2020-06-28 15:08:32 PDT
Comment on attachment 403011 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:4163
> +        if (bool shouldIncludeCurrentSelection = request.options.contains(DocumentEditingContextRequest::Options::SpatialAndCurrentSelection)) {

I'm leaning to removing this bool and assignment because its name only improves readability a very tiny bit....
Comment 4 Daniel Bates 2020-06-28 15:09:20 PDT
Created attachment 403012 [details]
Patch
Comment 5 Daniel Bates 2020-06-28 17:00:09 PDT
Created attachment 403016 [details]
Patch
Comment 6 Daniel Bates 2020-06-30 10:52:37 PDT
Created attachment 403207 [details]
Patch
Comment 7 Daniel Bates 2020-06-30 12:09:56 PDT
Comment on attachment 403207 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:477
> +    spatialBoxRect.size.height = 1;

I think I can get rid of this...
Comment 8 Daniel Bates 2020-06-30 12:23:28 PDT
Comment on attachment 403207 [details]
Patch

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

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:477
>> +    spatialBoxRect.size.height = 1;
> 
> I think I can get rid of this...

No, I can't so I will add a comment. The issue is that (and maybe this is a bug) hit testing the bottom of the line is considered the top of the next line (i.e. there's no gap in the programming model, though visually there could be via CSS line-height). So, a height of glyphWidth (because this is Ahem so things are square) or greater will cause the hit test to hit the next line. I don't want that because it then triggers another behavior: hit testing below the last line (also above the first line I think) is as if the entire line was hit (this is to cause a selection highlight of the entire line when painted). So, I just need to pick a height < glyphWidth. I like small things, so I chose 1.
Comment 9 Daniel Bates 2020-06-30 12:28:22 PDT
Created attachment 403222 [details]
Patch
Comment 10 Daniel Bates 2020-06-30 12:29:57 PDT
Comment on attachment 403222 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:34
> +#import "UserInterfaceSwizzler.h"

This is not needed.
Comment 11 Daniel Bates 2020-06-30 12:31:10 PDT
Comment on attachment 403222 [details]
Patch

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

> Source/WebKit/ChangeLog:11
> +        rect and current selection, if there is one.

and => and the
Comment 12 Daniel Bates 2020-06-30 17:15:16 PDT
Thanks for the review, Wenson!
Comment 13 Daniel Bates 2020-07-01 09:52:44 PDT
Created attachment 403302 [details]
To Land
Comment 14 Daniel Bates 2020-07-01 10:08:48 PDT
Committed r263813: <https://trac.webkit.org/changeset/263813>