Make document.mozCancelFullScreen restore previous full-screen state

RESOLVED FIXED in mozilla11

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: cpearce, Assigned: cpearce)

Tracking

({dev-doc-complete})

Trunk
mozilla11
dev-doc-complete
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
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

6 years ago
Assignee: nobody → chris
(Assignee)

Comment 1

6 years ago
Created attachment 578145 [details] [diff] [review]
Patch 1 v1: Rename nsDocument::SetFullScreenState(Element*,bool) to nsDocument::SetFullScreenElement(Element*)

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

6 years ago
Created attachment 578148 [details] [diff] [review]
Patch 2 v1: Implement full-screen element stacking

* 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 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+
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 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

6 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.
> DispatchFullScreenChange() uses nsPLDOMEvent::PostDOMEvent()

Ah, ok.  Perfect.
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

6 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

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2ab9e661401
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3678c3f162e
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/c2ab9e661401
https://hg.mozilla.org/mozilla-central/rev/c3678c3f162e
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Keywords: dev-doc-needed
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.