| Summary: | [iOS] Implement support for UIWKDocumentRequestSpatialAndCurrentSelection | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||||||||||
| Component: | WebCore Misc. | Assignee: | Daniel Bates <dbates> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | webkit-bug-importer, wenson_hsieh | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar, PlatformOnly | ||||||||||||||
| Version: | WebKit Local Build | ||||||||||||||||
| Hardware: | iPhone / iPad | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Daniel Bates
2020-06-28 14:52:42 PDT
Created attachment 403011 [details]
Patch
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.... Created attachment 403012 [details]
Patch
Created attachment 403016 [details]
Patch
Created attachment 403207 [details]
Patch
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 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. Created attachment 403222 [details]
Patch
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 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 Thanks for the review, Wenson! Created attachment 403302 [details]
To Land
Committed r263813: <https://trac.webkit.org/changeset/263813> |