Bug 178252

Summary:

Disambiguate name colision in element and video fullscreen delegate protocol selector names.

Product:

WebKit

Reporter:

Jeremy Jones <jeremyj-wk>

Component:

WebKit2

Assignee:

Jeremy Jones <jeremyj-wk>

Status:

NEW ---

Severity:

Normal

CC:

fred.wang, jer.noble, simon.fraser, thorton

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Description Jeremy Jones 2017-10-12 21:45:31 PDT

Disambiguate name colision in element and video fullscreen delegate protocol selector names.

Comment 1 Jeremy Jones 2017-10-12 21:52:27 PDT

Created attachment 323633 [details]
Patch

Comment 2 Jeremy Jones 2017-10-12 22:06:14 PDT

Created attachment 323634 [details]
Patch

Comment 3 Simon Fraser (smfr) 2017-10-13 11:18:50 PDT

Comment on attachment 323634 [details]
Patch

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

> Source/WebKit/ChangeLog:9
> +        but take different param types. This means that a class cannot implement both of these delegates protocols becuase of the ambiguous

becuase

> Source/WebKit/UIProcess/API/Cocoa/WKUIDelegatePrivate.h:98
> +- (void)_webViewDidEnterVideoFullscreen:(WKWebView *)webView WK_API_AVAILABLE(macosx(10.14), ios(12.0));
> +- (void)_webViewDidExitVideoFullscreen:(WKWebView *)webView WK_API_AVAILABLE(macosx(10.14), ios(12.0));

Are we already using WK_API_AVAILABLE(macosx(10.14), ios(12.0));? I thought we did something else until later.

> Source/WebKit/UIProcess/API/Cocoa/_WKFullscreenDelegate.h:43
> +- (void)_webViewWillEnterWebFullscreen:(NSView *)webView WK_API_AVAILABLE(macosx(10.14), ios(12.0));
> +- (void)_webViewDidEnterWebFullscreen:(NSView *)webView WK_API_AVAILABLE(macosx(10.14), ios(12.0));
> +- (void)_webViewWillExitWebFullscreen:(NSView *)webView WK_API_AVAILABLE(macosx(10.14), ios(12.0));
> +- (void)_webViewDidExitWebFullscreen:(NSView *)webView WK_API_AVAILABLE(macosx(10.14), ios(12.0));

Why doe these take an NSView, rather than a WKWebView?

Comment 4 Tim Horton 2017-10-13 11:54:17 PDT

Comment on attachment 323634 [details]
Patch

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

>> Source/WebKit/UIProcess/API/Cocoa/WKUIDelegatePrivate.h:98
>> +- (void)_webViewDidExitVideoFullscreen:(WKWebView *)webView WK_API_AVAILABLE(macosx(10.14), ios(12.0));
> 
> Are we already using WK_API_AVAILABLE(macosx(10.14), ios(12.0));? I thought we did something else until later.

Please use the TBA macros!

>> Source/WebKit/UIProcess/API/Cocoa/_WKFullscreenDelegate.h:43
>> +- (void)_webViewDidExitWebFullscreen:(NSView *)webView WK_API_AVAILABLE(macosx(10.14), ios(12.0));
> 
> Why doe these take an NSView, rather than a WKWebView?

WKView/WKWebView.

Comment 5 Jeremy Jones 2017-10-16 18:19:15 PDT

(In reply to Tim Horton from comment #4)
> Comment on attachment 323634 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=323634&action=review
> 
> >> Source/WebKit/UIProcess/API/Cocoa/WKUIDelegatePrivate.h:98
> >> +- (void)_webViewDidExitVideoFullscreen:(WKWebView *)webView WK_API_AVAILABLE(macosx(10.14), ios(12.0));
> > 
> > Are we already using WK_API_AVAILABLE(macosx(10.14), ios(12.0));? I thought we did something else until later.
> 
> Please use the TBA macros!

Done

> 
> >> Source/WebKit/UIProcess/API/Cocoa/_WKFullscreenDelegate.h:43
> >> +- (void)_webViewDidExitWebFullscreen:(NSView *)webView WK_API_AVAILABLE(macosx(10.14), ios(12.0));
> > 
> > Why doe these take an NSView, rather than a WKWebView?
> 
> WKView/WKWebView.

Comment 6 Jeremy Jones 2017-10-16 18:20:48 PDT

Created attachment 323965 [details]
Patch

Comment 7 Frédéric Wang (:fredw) 2018-05-23 06:55:37 PDT

Comment on attachment 323965 [details]
Patch

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

@Jeremy: What's the status of this?

> Source/WebKit/ChangeLog:9
> +        but take different param types. This means that a class cannot implement both of these delegates protocols becuase of the ambiguous

You forgot to fix s/becuase/because/

> Source/WebKit/UIProcess/API/Cocoa/_WKFullscreenDelegate.h:43
> +- (void)_webViewDidExitWebFullscreen:(NSView *)webView WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));

It looks like your comment on why NSView is used was not published.