[webkit-reviews] review granted: [Bug 231234] Allow text selection to flip. : [Attachment 440221] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 5 09:34:40 PDT 2021


Wenson Hsieh <wenson_hsieh at apple.com> has granted Megan Gardner
<megan_gardner at apple.com>'s request for review:
Bug 231234: Allow text selection to flip.
https://bugs.webkit.org/show_bug.cgi?id=231234

Attachment 440221: Patch

https://bugs.webkit.org/attachment.cgi?id=440221&action=review




--- Comment #2 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 440221
  --> https://bugs.webkit.org/attachment.cgi?id=440221
Patch

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

Nice! r=mews

> Source/WTF/ChangeLog:9
> +	   Add an internal flag to guard text selectin flipping while

Nit - selectin => selection

> Source/WebKit/ChangeLog:11
> +	   This is currently guarded behind and off-by-default flag so that we
can

Nit - "behind and" => "behind an"

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1506
> +static std::optional<SimpleRange> rangeForPointInRootViewCoordinates(Frame&
frame, const IntPoint& pointInRootViewCoordinates, bool baseIsStart, bool
selectionFlippingEnabled, bool &selectionFlipped)

Nit - instead of an out-reference (`bool& selectionFlipped`), I think we
generally prefer to coalesce that into the return value in modern code (i.e.
make this helper function either return `std::pair<SimpleRange, bool>`, or
`std::pair<SimpleRange, SelectionWasFlipped>` where `SelectionWasFlipped` is a
strongly typed enum)


More information about the webkit-reviews mailing list