[webkit-reviews] review granted: [Bug 236980] [iOS] Adjust some behaviors around the "Markup Image" action in the callout bar : [Attachment 452742] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Feb 21 15:51:22 PST 2022


Aditya Keerthi <akeerthi at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 236980: [iOS] Adjust some behaviors around the "Markup Image" action in the
callout bar
https://bugs.webkit.org/show_bug.cgi?id=236980

Attachment 452742: Patch

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




--- Comment #2 from Aditya Keerthi <akeerthi at apple.com> ---
Comment on attachment 452742
  --> https://bugs.webkit.org/attachment.cgi?id=452742
Patch

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm:100
> +using namespace WebCore;

I think we prefer to avoid adding `using namespace` in new code, and it looks
like you would just need to add prefixes to `contextMenuItemTitleMarkupImage()`
below.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm:317
> +    [webView objectByEvaluatingJavaScript:@"image = document.images[0];
getSelection().setBaseAndExtent(document.body, 0, image, 1);"];

Nit: I find this easier to read without the `image` variable.

(if you prefer to keep it, could use `let` for consistency with line 297)


More information about the webkit-reviews mailing list