Summary: | WebViewImpl doesn't notify the page that the user has canceled fullscreen. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Bill Budge <bbudge> | ||||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | fishd, gustavo, pnormand, webkit.review.bot, xan.lopez | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Bill Budge
2012-03-30 11:32:26 PDT
Created attachment 134862 [details]
Proposed Patch
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). Created attachment 134908 [details]
Proposed patch
Comment on attachment 134908 [details] Proposed patch Attachment 134908 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12247610 Created attachment 134973 [details]
Proposed Patch
Resubmitting. There was some problem with the last patch on GTK. 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. 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? Comment on attachment 134973 [details]
Proposed Patch
OK, thanks for verifying that!
Comment on attachment 134973 [details] Proposed Patch Clearing flags on attachment: 134973 Committed r112991: <http://trac.webkit.org/changeset/112991> All reviewed patches have been landed. Closing bug. |