Bug 178252 - Disambiguate name colision in element and video fullscreen delegate protocol selector names.
Summary: Disambiguate name colision in element and video fullscreen delegate protocol ...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jeremy Jones
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-12 21:45 PDT by Jeremy Jones
Modified: 2018-05-23 06:55 PDT (History)
4 users (show)

See Also:


Attachments
Patch (11.28 KB, patch)
2017-10-12 21:52 PDT, Jeremy Jones
no flags Details | Formatted Diff | Diff
Patch (11.21 KB, patch)
2017-10-12 22:06 PDT, Jeremy Jones
no flags Details | Formatted Diff | Diff
Patch (11.27 KB, patch)
2017-10-16 18:20 PDT, Jeremy Jones
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.