Closed Bug 688648 Opened 13 years ago Closed 13 years ago

Dispatch mozfullscreenerror event when requests for full-screen are denied

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla10

People

(Reporter: cpearce, Assigned: cpearce)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [inbound])

Attachments

(1 file, 3 obsolete files)

The original full-screen specification called for permission to go full-screen to be granted by the user before entering full-screen.

The problem with that though, is that the user may never grant permission, so script can't distinguish full-screen-not-granted yet from full-screen-decision-deferred-forever, and so script may never receive a full-screen denied event [1].

With our new ask-forgiveness model, this isn't the case, and we think it's reasonable to fire a "mozfullscreendenied" event when full-screen requests are denied.

The particular use case we're concerned about is an embedded player in an iframe can't tell whether the mozallowfullscreen attribute is present on the containing frame.

[1] http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2011-May/031584.html
Actually, we don't really need this, as currently Element.mozRequestFullScreen() is synchronous, so callers can just check document.mozFullScreen after calling Element.mozRequestFullScreen() to tell whether their request succeeded or not. We should consider implementing this if we change to an asynchronous request.

We now implement Document.mozFullScreenEnabled to handle the case mentioned in comment 0.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
Of course we want to enable implementations which aren't synchronous, and make our requestFullScreen() implementation compatible with them (i.e. asynchronous), in which case we'll want this event...
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Assignee: nobody → chris
Target Milestone: --- → mozilla10
Attached patch Patch v1 (obsolete) — Splinter Review
Dispatch mozfullscreendenied when full-screen requests are denied. This patch is based on the refactorings in the patch in bug 685779.
Attachment #570091 - Flags: review?(Olli.Pettay)
Depends on: 685779
Comment on attachment 570091 [details] [diff] [review]
Patch v1

 > nsresult nsGenericHTMLElement::MozRequestFullScreen()
> {
>-  // Only grant full-screen requests if this is called from inside a trusted
>-  // event handler (i.e. inside an event handler for a user initiated event).
>-  // This stops the full-screen from being abused similar to the popups of old,
>-  // and it also makes it harder for bad guys' script to go full-screen and
>-  // spoof the browser chrome/window and phish logins etc.
>-  if (!nsContentUtils::IsRequestFullScreenAllowed()) {
>+  // Only grant full-screen requests if the API is enabled, and if this
>+  // request comes from inside a trusted event handler (i.e. inside an
>+  // event handler for a user initiated event). This stops the full-screen
>+  // from being abused similar to the popups of old, and it also makes it
>+  // harder for bad guys' script to go full-screen and spoof the browser
>+  // chrome/window and phish logins etc.
>+
>+  nsIDocument* doc = OwnerDoc();
>+  if (!doc ||
As the method name indicates, OwnerDoc() returns always non-null.
(At least in content/ and dom/ only methods starting with Get* may return null,
 well, that is the rule. Not sure if it applies all the code.)
Attachment #570091 - Flags: review?(Olli.Pettay) → review+
The WhatWG draft spec calls this event fullscreenerror, and other vendors seem to agree with this naming, so we should probably change our event to being called mozfullscreenerror.
Summary: Dispatch mozfullscreendenied event when requests for full-screen are denied → Dispatch mozfullscreenerror event when requests for full-screen are denied
Attached patch Patch v2: Rebased (obsolete) — Splinter Review
Patch v1 rebased on top of recent landings. It's pretty much the same patch, but different enough that I'd feel guilty landing it without having it re-reviewed. ;)
Attachment #570091 - Attachment is obsolete: true
Attachment #570934 - Flags: review?(Olli.Pettay)
Comment on attachment 570934 [details] [diff] [review]
Patch v2: Rebased

># HG changeset patch
># Parent 0af265703b841b2c4031e020715022a0246f8f09
>Bug 688648 - Dispatch mozfullscreenerror event when requests for full-screen are denied. r=?
>
>diff --git a/content/base/public/nsContentUtils.h b/content/base/public/nsContentUtils.h
>--- a/content/base/public/nsContentUtils.h
>+++ b/content/base/public/nsContentUtils.h
>@@ -1730,16 +1730,24 @@ public:
>    * Returns true if the content is in a document and contains a plugin
>    * which we don't control event dispatch for, i.e. do any plugins in this
>    * doc tree receive key events outside of our control? This always returns
>    * false on MacOSX.
>    */
>   static bool HasPluginWithUncontrolledEventDispatch(nsIContent* aContent);
> 
>   /**
>+   * Asynchronously dispatches a "mozfullscreenerror" event to aElement
>+   * or to aDocument if aElement is null.
>+   */
>+  static void DispatchFullScreenEvent(nsIDocument* aDocument,
>+                                      Element* aElement,
>+                                      const nsString& aEventName);
>+

Actually, please use nsPLDOMEvent
It has PostDOMEvent for async firing.
Sorry, that I didn't mentioned that earlier.
Attachment #570934 - Flags: review?(bugs) → review-
Attached patch Patch v3 (obsolete) — Splinter Review
With nsPLDOMEvent.
Attachment #570934 - Attachment is obsolete: true
Attachment #571168 - Flags: review?(bugs)
Comment on attachment 571168 [details] [diff] [review]
Patch v3

Just drop nsContentUtils::DispatchFullScreenEvent
(which doesn't do anything fullscreen related) and create and post
nsPLDOMEvent in each place separately.
with that, r+
Attachment #571168 - Flags: review?(bugs) → review+
Attached patch Patch v4Splinter Review
Without nsContentUtils::DispatchFullScreenEvent.
Attachment #571168 - Attachment is obsolete: true
Attachment #571180 - Flags: review+
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/5a6ea5db053e
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.