[webkit-reviews] review granted: [Bug 211151] [Text manipulation] Add a userInfo dictionary to _WKTextManipulationToken : [Attachment 397907] Build fix for non-internal SDKs (2)
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Apr 28 19:08:12 PDT 2020
Darin Adler <darin at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 211151: [Text manipulation] Add a userInfo dictionary to
_WKTextManipulationToken
https://bugs.webkit.org/show_bug.cgi?id=211151
Attachment 397907: Build fix for non-internal SDKs (2)
https://bugs.webkit.org/attachment.cgi?id=397907&action=review
--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 397907
--> https://bugs.webkit.org/attachment.cgi?id=397907
Build fix for non-internal SDKs (2)
View in context: https://bugs.webkit.org/attachment.cgi?id=397907&action=review
> Source/WebCore/editing/TextManipulationController.cpp:261
> +static Optional<TextManipulationController::ManipulationTokenInfo>
tokenInfo(RefPtr<Node>&& node)
Seems a little strange for this to take RefPtr<Node>&& instead of Node*.
> Source/WebCore/editing/TextManipulationController.cpp:268
> + if (auto element = makeRefPtr(is<Element>(node.get()) ?
downcast<Element>(node.get()) : node->parentElement())) {
Since we null-checked node, this should be *node rather than node.get(), and
then need to add a &.
But also, seems like we need this helper function that returns node if element
or parent element. Because it comes up a lot.
> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1617
> + if (!!info->tagName)
!x.isNull() is a better idiom than !!x I think. Geoff Garen was talking about
this recently.
> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1662
> + [wkToken
setUserInfo:createUserInfo(token.info).autorelease()];
This can be get(). Don’t need autorelease to pass as an argument; need it to
return as a result.
More information about the webkit-reviews
mailing list