<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:mitz@webkit.org" title="mitz@webkit.org <mitz@webkit.org>"> <span class="fn">mitz@webkit.org</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - Make preview inline navigation work API"
href="https://bugs.webkit.org/show_bug.cgi?id=155383">bug 155383</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #273784 Flags</td>
<td>review?
</td>
<td>review-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Make preview inline navigation work API"
href="https://bugs.webkit.org/show_bug.cgi?id=155383#c4">Comment # 4</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Make preview inline navigation work API"
href="https://bugs.webkit.org/show_bug.cgi?id=155383">bug 155383</a>
from <span class="vcard"><a class="email" href="mailto:mitz@webkit.org" title="mitz@webkit.org <mitz@webkit.org>"> <span class="fn">mitz@webkit.org</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=273784&action=diff" name="attach_273784" title="Patch">attachment 273784</a> <a href="attachment.cgi?id=273784&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=273784&action=review">https://bugs.webkit.org/attachment.cgi?id=273784&action=review</a>
r- because this breaks the Mac build. Some more comments below.
<span class="quote">> Source/WebKit2/ChangeLog:16
> + _WKPreviewActionItem is now WKPreviewActionItem, but the _WKPreviewAction
> + class is still a private implementation detail, so it still has an underscore
> + even though the filenames do not.</span >
If it’s an *internal* implementation detail, i.e, the class isn’t available even to SPI clients and isn’t exposed in a framework header (which I think is the case or at least should be the case), then per our convention it should *not* be underscore-prefixed.
<span class="quote">> Source/WebKit2/ChangeLog:19
> + nowWKPreviewActionItemIdentifiers.h/mm and all the the identifiers have been </span >
nowWK
<span class="quote">> Source/WebKit2/UIProcess/API/Cocoa/WKElementInfoInternal.h:39
> +#endif // WK_API_ENABLED</span >
I think we normally don’t // WK_API_ENABLED.
<span class="quote">> Source/WebKit2/UIProcess/API/Cocoa/WKPreviewActionItemInternal.h:32
> +@interface _WKPreviewAction : UIPreviewAction <NSCopying, WKPreviewActionItem></span >
So the underscore should go.
Not new, but you’re claiming NSCopying conformance here but I don’t see that class overrides -copyWithZone:, so at the very least it fails to copy the identifier (and that’s assuming UIPreviewAction is copyable, in which case, there’s no need to declare that its subclass is NSCopying-conformant).
<span class="quote">> Source/WebKit2/UIProcess/API/Cocoa/WKUIDelegate.h:150
> +- (BOOL)webView:(WKWebView *)webView shouldPreviewElement:(WKPreviewElementInfo *)elementInfo WK_AVAILABLE(NA, WK_IOS_TBA);
> +
> +/*! @abstract Allows your app to provide a custom view controller to show when the given element is peeked.
> + @param webView The web view invoking the delegate method.
> + @param elementInfo The elementInfo for the element the user is peeking.
> + @param defaultActions An array of the actions that WebKit would use as previewActionItems for this element by
> + default. These actions would be used if allowsLinkPreview is YES but these delegate methods have not been
> + implemented, or if this delegate method returns nil.
> + @discussion Returning a view controller will result in that view controller being displayed as a peek preview.
> + To use the defaultActions, your app is responsible for returning whichever of those actions it wants in your
> + view controller's implementation of -previewActionItems.
> +
> + Returning nil will result in WebKit's default preview behavior. webView:commitPreviewingViewController: will only be invoked
> + if a non-nil view controller was returned.
> + */
> +- (nullable UIViewController *)webView:(WKWebView *)webView previewingViewControllerForElement:(WKPreviewElementInfo *)elementInfo defaultActions:(NSArray <id <WKPreviewActionItem>> *)previewActions WK_AVAILABLE(NA, WK_IOS_TBA);
> +
> +/*! @abstract Allows your app to pop to the view controller it created.
> + @param webView The web view invoking the delegate method.
> + @param previewingViewController The view controller that is being popped.
> + */
> +- (void)webView:(WKWebView *)webView commitPreviewingViewController:(UIViewController *)previewingViewController WK_AVAILABLE(NA, WK_IOS_TBA);</span >
These should all be in an #if TARGET_OS_IPHONE block.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>