Bug 139465 – Restore interface state when stopping optimized fullscreen.

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jeremy Jones]

Restore interface state when stopping optimized fullscreen.

[Jeremy Jones]

rdar://problem/18878722

[Jeremy Jones]

Created attachment 242976 [details]
Patch

[WebKit Commit Bot]

Attachment 242976 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:910:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebKit2/UIProcess/Cocoa/UIDelegate.h:90:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
Total errors found: 2 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sam Weinig]

WebKit doesn't really have any concept of tab, so it is weird to be adding it here. Can you explain why this is necessary?

Comment on attachment 242976 [details]
Patch

Tabs aren’t a WebKit concept so at least the terminology in most of this patch is wrong. The new method seems similar to -webViewFocus: in the legacy WebUIDelegate, but the boolean parameter suggests that it’s different. But it seems strange to have an “animate” parameter to this delegate method. The delegate can implement a UI where animation doesn’t make sense, or everything is always animated. So perhaps the right delegate method for this should be specifically about the optimized full screen video scenario and just tell the delegate what happened (some playback mode was exited in some way).

[Jeremy Jones]

(In reply to comment #4)
> WebKit doesn't really have any concept of tab, so it is weird to be adding
> it here. Can you explain why this is necessary?

I guess "tab" is too concrete. It is about making sure the page that optimized fullscreen animates back into is visible.

[Jeremy Jones]

(In reply to comment #5)
> Comment on attachment 242976 [details]
> Patch
> 
> Tabs aren’t a WebKit concept so at least the terminology in most of this
> patch is wrong. The new method seems similar to -webViewFocus: in the legacy
> WebUIDelegate, but the boolean parameter suggests that it’s different. But
> it seems strange to have an “animate” parameter to this delegate method. The
> delegate can implement a UI where animation doesn’t make sense, or
> everything is always animated. So perhaps the right delegate method for this
> should be specifically about the optimized full screen video scenario and
> just tell the delegate what happened (some playback mode was exited in some
> way).

How about -fullscreenMayReturnToInline which describes the event that is happening. Which is basically saying that fullscreen may return to inline at any time, so get the interface ready for that.

[Jeremy Jones]

Created attachment 243076 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 243076 [details]
Patch

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

> Source/WebKit2/ChangeLog:8
> +        Implement fullscreenMayReturnToInline() to request the client application show the corresponding tab.

I think the "may" is a a bit vague and confusing for clients of the API, but we can fix that later.

[Jon Lee]

The commit queue is reflecting no patches left to merge in-- it looks stuck. Can we get this checked in directly?

[Jeremy Jones]

(In reply to comment #10)
> The commit queue is reflecting no patches left to merge in-- it looks stuck.
> Can we get this checked in directly?

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm
	M	Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.h
	M	Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm
	M	Source/WebKit2/ChangeLog
	M	Source/WebKit2/UIProcess/API/APIUIClient.h
	M	Source/WebKit2/UIProcess/API/Cocoa/WKUIDelegatePrivate.h
	M	Source/WebKit2/UIProcess/Cocoa/UIDelegate.h
	M	Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm
	M	Source/WebKit2/UIProcess/WebPageProxy.cpp
	M	Source/WebKit2/UIProcess/WebPageProxy.h
	M	Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.h
	M	Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.mm
Committed r177154