[webkit-reviews] review granted: [Bug 200909] REGRESSION: Open in New Tab is missing from context menu : [Attachment 376736] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Dean Jackson
<dino at apple.com>'s request for review:
Bug 200909: REGRESSION: Open in New Tab is missing from context menu
https://bugs.webkit.org/show_bug.cgi?id=200909

Attachment 376736: Patch

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




--- Comment #3 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 376736
  --> https://bugs.webkit.org/attachment.cgi?id=376736
Patch

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:7724
> +    // even though they haven't moved to the new API.

The 'they' is ambiguous. Does it refer to MobileSafari, or to the methods?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ContextMenus.mm:50
> +    static auto window = adoptNS([[UIWindow alloc]
initWithFrame:NSMakeRect(0, 0, 800, 600)]);
> +    static auto driver = adoptNS([TestContextMenuDriver new]);
> +    static auto uiDelegate = adoptNS((NSObject<WKUIDelegate>
*)[delegateClass new]);
> +    static auto configuration = adoptNS([WKWebViewConfiguration new]);

These statics are weird. Is this just to extend lifetime? Can we just retain
something and release it when the test is complete?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ContextMenus.mm:143
> +    return previewActions;

Could this be written as:
return @[
  [UIPreviewAction actionWithTitle:@"Action" style:UIPreviewActionStyleDefault
handler:^(UIPreviewAction *, UIViewController *) { }];
];