Closed
Bug 695935
Opened 13 years ago
Closed 13 years ago
Make document.mozRequestFullScreen() asynchronous
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: cpearce, Assigned: cpearce)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 1 obsolete file)
23.26 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
To match other anticipated implementations and the anticipated spec, we should make our requestFullScreen implementation asynchronous. We'll need a fullscreendenied event for that.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → chris
Version: 9 Branch → 10 Branch
Assignee | ||
Updated•13 years ago
|
Target Milestone: --- → mozilla10
Version: 10 Branch → Trunk
Assignee | ||
Updated•13 years ago
|
Assignee | ||
Comment 1•13 years ago
|
||
For the record, Chrome 17.0.926.0 Canary's full-screen API is synchronous, but Safari 5.1.1's is asynchronous. That is, video.webkitDisplayingFullscreen returns true immediately after calling video.webkitEnterFullScreen, whereas in Safari video.webkitDisplayingFullscreen does not return true until some time (>100ms) after calling video.webkitEnterFullScreen.
Assignee | ||
Comment 2•13 years ago
|
||
* Process the full-screen request state changes asynchronously. This is to allow us to write a spec which allows implementations to show an enter full-screen animation before full-screen state is reported, and/or a permission grant UI is required to enter full-screen. * Dispatch mozfullscreenerror events to document, not element, matching the W3 draft. * Note this is based on top of the patch in bug 691947, which is landed on inbound.
Attachment #571578 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 3•13 years ago
|
||
Hmm, a new case I need to handle which is introduced by this patch is if a change tab (or ALT+TAB too I guess) comes in after full-screen is requested, but before the request is actioned. We should cancel full-screen in this case, as otherwise (in the change-tab case) the window goes full-screen, but the document with the full-screen styles applied is in a background tab. I'll handle this in another patch in this bug. I can probably do this by checking whether the documents which receive mozfullscreenchange are descendent from the currently focused tab in browser.js.
Comment 4•13 years ago
|
||
Comment on attachment 571578 [details] [diff] [review] Patch v1 >+++ b/content/base/src/nsDocument.cpp > nsDocument::GetMozFullScreenEnabled(bool *aFullScreen) >+ if ((nsContentUtils::IsCallerChrome() && >+ nsContentUtils::IsFullScreenApiEnabled()) || That second line shouldbe indented by one more space >+nsDocument::IsFullScreenEnabled() { Curly brace on next line please. >+++ b/content/base/src/nsDocument.h >+ // Returns true is a request for DOM full-screen is currently enabled in s/is a request/if a request/ >+ // this document. This checks whether there are no windowed plugins in this >+ // doc tree, whether the document is visible, and whether the api is not >+ // disabled by pref. Replace all those "whether" with "that", please. I think that would be clearer. >+++ b/content/html/content/src/nsGenericHTMLElement.cpp >+class nsCallRequestFullScreen : public nsRunnable >+ mElement->OwnerDoc()->RequestFullScreen(mElement); Hmm. So if the element is adopted into a different document while the event is pending, this will request fullscreen on the new document. That seems pretty weird to me; I would prefer that we save the ownerDocument when posting the event and just bail out if it's changed since then. But either way is ok, I guess as long as it's specced. Per irc discussion, we need to store the IsCallerChrome state from when nsGenericHTMLElement::MozRequestFullScreen is called in the nsCallRequestFullScreen and propagate it to RequestFullScreen. This last should replace IsFullScreenEnabled() with a test more like the one in GetMozFullScreenEnabled (possibly factored out as a helper function). r=me with those changes, though I'd sort of like to take a look at the code addressing the IsCallerChrome bit once it's done.
Attachment #571578 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 5•13 years ago
|
||
* Don't grant full-screen requests when full-screen element has moved out of document. * Save IsCallerChrome result for async full-screen request.
Attachment #571578 -
Attachment is obsolete: true
Attachment #571898 -
Flags: review?(bzbarsky)
Comment 6•13 years ago
|
||
Comment on attachment 571898 [details] [diff] [review] Patch v2 r=me
Attachment #571898 -
Flags: review?(bzbarsky) → review+
Comment 7•13 years ago
|
||
Comment on attachment 571898 [details] [diff] [review] Patch v2 >--- a/content/base/src/nsDocument.cpp >+++ b/content/base/src/nsDocument.cpp >+ NS_IMETHOD Run() >+ nsDocument* doc = static_cast<nsDocument*>(mDoc.get()); :/ >+nsDocument::AsyncRequestFullScreen(Element* aElement) >+ nsCOMPtr<nsIRunnable> event(new nsCallRequestFullScreen(aElement)); Please make this nsCOMPtr<nsIRunnable> event = new nsCallRequestFullScreen(aElement);
Assignee | ||
Comment 8•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed6dfa32837e
Whiteboard: [inbound]
Comment 9•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ed6dfa32837e
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Updated•13 years ago
|
Keywords: dev-doc-needed
Comment 10•8 years ago
|
||
This is very old, I'm in the progress of updating our docs for the latest unprefixed implementation :-) So let's call this done.
Keywords: dev-doc-needed → dev-doc-complete
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•