Bug 200909 – REGRESSION: Open in New Tab is missing from context menu

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dean Jackson]

REGRESSION: Open in New Tab is missing from context menu

[Dean Jackson]

Created attachment 376736 [details]
Patch

[Dean Jackson]

<rdar://problem/54476169>

[Simon Fraser (smfr)]

Comment on attachment 376736 [details]
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 *) { }];
];

[Dean Jackson]

Comment on attachment 376736 [details]
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?

Clarified.

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/ContextMenus.mm:50
>> +    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?

The issue is this is a helper function that is creating the state for the actual tests. So the variables have to exist somewhere, and it would be annoying to pass them in and out. The statics could live globally, but that isn't much different.

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/ContextMenus.mm:143
>> +    return previewActions;
> 
> Could this be written as:
> return @[
>   [UIPreviewAction actionWithTitle:@"Action" style:UIPreviewActionStyleDefault handler:^(UIPreviewAction *, UIViewController *) { }];
> ];

Yes.

[Dean Jackson]

Committed r248900: <https://trac.webkit.org/changeset/248900>