Bug 178252

Summary: Disambiguate name colision in element and video fullscreen delegate protocol selector names.
Product: WebKit Reporter: Jeremy Jones <jeremyj-wk>
Component: WebKit2Assignee: Jeremy Jones <jeremyj-wk>
Status: NEW ---    
Severity: Normal CC: fred.wang, jer.noble, simon.fraser, thorton
Priority: P2    
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.