Closed
Bug 704039
Opened 13 years ago
Closed 13 years ago
Make document.mozCancelFullScreen restore previous full-screen state
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: cpearce, Assigned: cpearce)
References
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
The W3C editor's draft of the full-screen spec [1] now includes a stack of full-screen elements, so that Document.mozCancelFullScreen() "rolls-back" to restore the previously full-screen element to full-screen state. We should implement this, targeting Firefox 11.
[1] http://dvcs.w3.org/hg/fullscreen/raw-file/tip/Overview.html
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → chris
Assignee | ||
Comment 1•13 years ago
|
||
Whenever we call nsDocument::SetFullScreenState(Element*,bool) with a null element, the bool argument is false, and whenever the Element is non-null, the bool is true, so we can rename that SetFullScreenElement() and remove the bool argument.
Attachment #578145 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 2•13 years ago
|
||
* Change full-screen model to match the W3 spec of having a stack of full-screen elements, so that every time you request full-screen the existing full-screen element (if any) is pushed onto a stack, and when Document.mozCancelFullScreen() is called we rollback to having the previous full-screen element as the full-screen element. See http://dvcs.w3.org/hg/fullscreen/raw-file/tip/Overview.html for spec.
* Each document maintains its own stack of full-screen elements, and we still push the containing elements onto the stacks in ancestor documents too. The popping algorithm will ensure that "containing full-screen elements" won't get set to full-screen state upon calling mozCancelFullScreen(), only elements which directly requested full-screen should return to full-screen state.
* Calling Document.mozCancelFullScreen() automatically clears the stacks of every descendent document, and sets the full-screen element to the previous non-container full-screen element in this or ancestor documents.
* Add a static function, nsIDocument::ExitFullScreen() to "fully exit" full-screen. This clears all full-screen stacks of all full-screen documents.
* Deny requests for full-screen in a document which has full-screen subdocuments, and when requesting full-screen on an element not descendent from the current full-screen element in the requesting document (as required by the spec).
Attachment #578148 -
Flags: review?(bzbarsky)
Comment 3•13 years ago
|
||
Comment on attachment 578145 [details] [diff] [review]
Patch 1 v1: Rename nsDocument::SetFullScreenState(Element*,bool) to nsDocument::SetFullScreenElement(Element*)
r=me
Attachment #578145 -
Flags: review?(bzbarsky) → review+
Comment 4•13 years ago
|
||
A question about patch 2. DispatchFullScreenChange seems like it can trigger arbitrary script including more fullscreen requests or cancellations. Does the code actually handle that sort of thing properly? It's not clear to me that it does...
Comment 5•13 years ago
|
||
Comment on attachment 578148 [details] [diff] [review]
Patch 2 v1: Implement full-screen element stacking
>+ static bool IsFullScreenAncestor(Element* aElement);
Why not just make this a member method on Element?
>+++ b/content/base/public/nsIDocument.h
You need an IID change here. Well, maybe not strictly need, but I think it would be a good idea.
More after the answer to my question about reentry.
Assignee | ||
Comment 6•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #4)
> A question about patch 2. DispatchFullScreenChange seems like it can
> trigger arbitrary script [...]
DispatchFullScreenChange() uses nsPLDOMEvent::PostDOMEvent(), which is supposed to dispatch asynchronously, so all state changes in the doctree should be complete before any of the event handlers run. Unless I'm missing something?
Callers of DispatchFullScreenChange() assume that no event handlers will run while they're making state changes to the doctree.
Comment 7•13 years ago
|
||
> DispatchFullScreenChange() uses nsPLDOMEvent::PostDOMEvent()
Ah, ok. Perfect.
Comment 8•13 years ago
|
||
Comment on attachment 578148 [details] [diff] [review]
Patch 2 v1: Implement full-screen element stacking
Is there a reason that ClearFullScreenStack() uses GetFullScreenElement() and not FullScreenStackTop()?
r=me with that explained or fixed and the other comments addressed.
Attachment #578148 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 9•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #8)
> Is there a reason that ClearFullScreenStack() uses GetFullScreenElement()
> and not FullScreenStackTop()?
Nope. I'll change make ClearFullScreenStack() to use FullScreenStackTop().
Thanks for review!
Assignee | ||
Comment 10•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2ab9e661401
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3678c3f162e
Target Milestone: --- → mozilla11
Comment 11•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c2ab9e661401
https://hg.mozilla.org/mozilla-central/rev/c3678c3f162e
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•13 years ago
|
Keywords: dev-doc-needed
Comment 12•13 years ago
|
||
Doc updated:
https://developer.mozilla.org/en/DOM/document.mozCancelFullScreen
And mentioned on Firefox 11 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•