Bug 82760 – WebViewImpl doesn't notify the page that the user has canceled fullscreen.

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Bill Budge]

Recent changes in WebCore::Document caused a regression in Chromium. The page isn't getting notified when the user presses ESC or clicks the exit fullscreen bubble.
http://trac.webkit.org/changeset/112596

[Bill Budge]

Created attachment 134862 [details]
Proposed Patch

[Darin Fisher (:fishd, Google)]

Comment on attachment 134862 [details]
Proposed Patch

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

> Source/WebKit/chromium/src/WebViewImpl.cpp:1337
> +        doc->webkitCancelFullScreen();

I was expecting to see webkitCancelFullScreen called from willExitFullScreen.
The implication of calling webkitCancelFullScreen here is that we will have
already called Document::webkitWillExitFullScreenForElement.  It seems like
that might confuse Document (if not now, then later).

[Bill Budge]

Created attachment 134908 [details]
Proposed patch

[Philippe Normand]

Comment on attachment 134908 [details]
Proposed patch

Attachment 134908 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12247610

[Bill Budge]

Created attachment 134973 [details]
Proposed Patch

[Bill Budge]

Resubmitting. There was some problem with the last patch on GTK.

[Darin Fisher (:fishd, Google)]

Comment on attachment 134973 [details]
Proposed Patch

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

> Source/WebKit/chromium/src/WebViewImpl.cpp:1327
> +            doc->webkitCancelFullScreen();

if exiting fullscreen is initiated by JS calling document.webkitCancelFullScreen(),
then we will eventually reach this code again.  it looks like the second call to
webkitCancelFullScreen() will call webkitExitFullscreen().  in this case, do we
have to worry about a second webkitfullscreenchange event being generated?  it
looks like Document::addDocumentToFullScreenChangeEventQueue() could be called
more than once.

[Bill Budge]

When we call webkitCancelFullScreen the second time, it exits at the first if-statement:


    if (!topDocument()->webkitFullscreenElement())
        return;

It seems a little fragile, but webkitExitFullscreen() isn't called a second time. If we want to avoid the second call, I can't see a way to distinguish whether the cancel is coming from the page or the embedder at this level. Ideas?

[Darin Fisher (:fishd, Google)]

Comment on attachment 134973 [details]
Proposed Patch

OK, thanks for verifying that!

[WebKit Review Bot]

Comment on attachment 134973 [details]
Proposed Patch

Clearing flags on attachment: 134973

Committed r112991: <http://trac.webkit.org/changeset/112991>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.