[webkit-reviews] review granted: [Bug 224837] [iOS] Text selection in image overlays should not be limited to rectilinear quads : [Attachment 426609] Depends on #224820
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Apr 20 23:44:56 PDT 2021
Tim Horton <thorton at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 224837: [iOS] Text selection in image overlays should not be limited to
rectilinear quads
https://bugs.webkit.org/show_bug.cgi?id=224837
Attachment 426609: Depends on #224820
https://bugs.webkit.org/attachment.cgi?id=426609&action=review
--- Comment #2 from Tim Horton <thorton at apple.com> ---
Comment on attachment 426609
--> https://bugs.webkit.org/attachment.cgi?id=426609
Depends on #224820
View in context: https://bugs.webkit.org/attachment.cgi?id=426609&action=review
> Source/WebCore/rendering/RenderLineBreak.cpp:235
> + auto behavior = HTMLElement::isImageOverlayText(element()) ?
SelectionRenderingBehavior::UseIndividualQuads :
SelectionRenderingBehavior::CoalesceBoundingRects;
It's a bit unfortunate that isImageOverlayText is infecting something like
RenderLineBreak. Can we maybe hide it in a SelectionGeometry helper or
something?
> Source/WebCore/rendering/RenderObject.cpp:740
> + auto behavior = HTMLElement::isImageOverlayText(node()) ?
SelectionRenderingBehavior::UseIndividualQuads :
SelectionRenderingBehavior::CoalesceBoundingRects;
Ditto
> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4808
> -static NSArray<WKTextSelectionRect *> *wkTextSelectionRects(const
Vector<WebCore::SelectionGeometry>& rects)
> +static NSArray<WKTextSelectionRect *> *wkTextSelectionRects(const
Vector<WebCore::SelectionGeometry>& rects, CGFloat scaleFactor)
While you're here... maybe drop the `wk` prefix?
> Source/WebKit/UIProcess/ios/WKTextSelectionRect.mm:114
> +- (WKTextSelectionRectCustomHandleInfo *)_customHandleInfo
Returning a random unrelated type here where this expects a
UITextSelectionRectCustomHandleInfo is pretty sketchy, even if it's "OK". Can
WKTextSelectionRectCustomHandleInfo be a subclass of
WKTextSelectionRectCustomHandleInfo like WKTextSelectionRect is a subclass of
UITextSelectionRect?
> Source/WebKit/UIProcess/ios/WKTextSelectionRect.mm:121
> + return [[[WKTextSelectionRectCustomHandleInfo alloc]
initWithFloatQuad:scaledQuad] autorelease];
See https://commits.webkit.org/r273062; I think this should be
adoptNS().autorelease() for future-ARC-happiness
> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:265
> + convertedQuad.setP1(view.contentsToRootView(convertedQuad.p1()));
There are two other instances of this code (InspectorTimelineAgent and
sendTapHighlightForNode), maybe it's time for ScrollView/FrameView to get a
FloatQuad version of contentsToRootView??
More information about the webkit-reviews
mailing list