Bug 125272

Summary: [Win] Exiting from Media Full Screen mode via 'escape' key does not work properly
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: MediaAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, jer.noble, jonlee
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Patch jer.noble: review+

Description Brent Fulgham 2013-12-04 17:58:16 PST
If you enter full screen video mode, and then exit using the "Escape" key, WebKit will not re-enter fullscreen mode. If you use the media control to enter/exit fullscreen mode, everything works properly.

This bug is caused by improper cleanup of the Document::m_fullScreenElementStack member:

When logic passes through the JavaScript layer, it pops the most recent full screen element off of the stack because it calls Document::webkitExitFullscreen(), which pops the element before calling the client's exitFullScreenForElement.

The 'escape' key handler in Windows calls Document::webkitWillExitFullScreenForElement(Element*), which bypasses the pop operation and leaves the element on the stack.  Subsequent calls to enter full screen mode fail because WebCore believes a full screen controller is already in place.

The proposed solution is to modify the WebKit/win/WebView implementation to use "webkitExitFullscreen" rather than "webkitWillExitFullScreenForElement" to more closely match that logic of the media control interface.
Comment 1 Brent Fulgham 2013-12-05 09:15:31 PST
Created attachment 218520 [details]
Patch
Comment 2 Jer Noble 2013-12-05 10:19:03 PST
Comment on attachment 218520 [details]
Patch

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

r=me, with nit.

> Source/WebKit/win/WebView.cpp:6958
> -    m_fullScreenElement->document().webkitWillExitFullScreenForElement(m_fullScreenElement.get());
> +    m_fullScreenElement->document().webkitExitFullscreen();

This should probably use webkitCancelFullScreen(), which will pop the entire full screen element stack.
Comment 3 Brent Fulgham 2013-12-05 12:12:06 PST
Committed r160184: <http://trac.webkit.org/changeset/160184>