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

Jeremy Jones

Reported 2017-10-12 21:45:31 PDT

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

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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Jeremy Jones

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

Created attachment 323633 [details] Patch

Jeremy Jones

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

Created attachment 323634 [details] Patch

Simon Fraser (smfr)

Comment 3 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?

Tim Horton

Comment 4 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.

Jeremy Jones

Comment 5 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.

Jeremy Jones

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

Created attachment 323965 [details] Patch

Frédéric Wang (:fredw)

Comment 7 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.

Note You need to log in before you can comment on or make changes to this bug.