Bug 163674

Summary: Add WKWebView fullscreen delegate SPI
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, jbedard, ossy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch andersca: review+

Description Jer Noble 2016-10-19 11:15:07 PDT
Add WKWebView fullscreen delegate SPI
Comment 1 Jer Noble 2016-10-19 11:17:47 PDT
rdar://problem/28722363
Comment 2 Jer Noble 2016-10-19 11:18:16 PDT
Created attachment 292085 [details]
Patch
Comment 3 Jer Noble 2016-10-19 21:02:03 PDT
Created attachment 292148 [details]
Patch
Comment 4 Jer Noble 2016-10-20 10:26:30 PDT
Created attachment 292222 [details]
Patch
Comment 5 Anders Carlsson 2016-10-24 13:12:14 PDT
Comment on attachment 292222 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:3942
> +    return [static_cast<WebKit::FullscreenClient&>(_page->fullscreenClient()).delegate().leakRef() autorelease];

Can just use .autorelease() here instead of .leakRef() autorelease].

> Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:249
> +@property (nonatomic, setter=_setFullscreenDelegate:) id<_WKFullscreenDelegate> _fullscreenDelegate WK_API_AVAILABLE(macos(10.13));
> +@property (nonatomic, readonly) BOOL _isInFullscreen WK_API_AVAILABLE(macos(10.13));

Should be WK_MAC_TBA.
Comment 6 Jer Noble 2016-10-25 08:55:25 PDT
Committed r207820: <http://trac.webkit.org/changeset/207820>
Comment 7 Csaba Osztrogonác 2016-10-25 09:26:37 PDT
(In reply to comment #6)
> Committed r207820: <http://trac.webkit.org/changeset/207820>

It broke the debug build, see build.webkit.org for details.
Comment 8 Csaba Osztrogonác 2016-10-25 09:29:21 PDT
/Volumes/Data/slave/elcapitan-debug/build/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:3942:102: error: expected ';' after return statement
    return static_cast<WebKit::FullscreenClient&>(_page->fullscreenClient()).delegate().autorelease());
                                                                                                     ^
                                                                                                     ;

Not only the debug, but all builds .... Please build locally before landing random syntax errors.
Comment 9 Csaba Osztrogonác 2016-10-25 09:30:46 PDT
fix already landed in https://trac.webkit.org/changeset/207822 , thanks Jonathan.
Comment 10 Simon Fraser (smfr) 2016-10-27 19:26:01 PDT
Comment on attachment 292222 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:249
>> +@property (nonatomic, setter=_setFullscreenDelegate:) id<_WKFullscreenDelegate> _fullscreenDelegate WK_API_AVAILABLE(macos(10.13));
>> +@property (nonatomic, readonly) BOOL _isInFullscreen WK_API_AVAILABLE(macos(10.13));
> 
> Should be WK_MAC_TBA.

This broke building against older SDKs that don't know about "macos".