Dispatch mozfullscreenerror event when requests for full-screen are denied

RESOLVED FIXED in mozilla10

Status

()

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

People

(Reporter: cpearce, Assigned: cpearce)

Tracking

({dev-doc-complete})

Trunk
mozilla10
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

6 years ago
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
(Assignee)

Comment 1

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
(Assignee)

Comment 2

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

Updated

6 years ago
Blocks: 695935
(Assignee)

Updated

6 years ago
Assignee: nobody → chris
Target Milestone: --- → mozilla10
(Assignee)

Comment 3

6 years ago
Created attachment 570091 [details] [diff] [review]
Patch v1

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)
(Assignee)

Updated

6 years ago
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+
(Assignee)

Comment 5

6 years ago
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.
ok
(Assignee)

Updated

6 years ago
Summary: Dispatch mozfullscreendenied event when requests for full-screen are denied → Dispatch mozfullscreenerror event when requests for full-screen are denied
(Assignee)

Comment 7

6 years ago
Created attachment 570934 [details] [diff] [review]
Patch v2: Rebased

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-
(Assignee)

Comment 9

6 years ago
Created attachment 571168 [details] [diff] [review]
Patch v3

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+
(Assignee)

Comment 11

6 years ago
Created attachment 571180 [details] [diff] [review]
Patch v4

Without nsContentUtils::DispatchFullScreenEvent.
Attachment #571168 - Attachment is obsolete: true
Attachment #571180 - Flags: review+
(Assignee)

Comment 12

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a6ea5db053e
(Assignee)

Updated

6 years ago
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/5a6ea5db053e
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Keywords: dev-doc-needed
Documented:

https://developer.mozilla.org/en/DOM/element.mozRequestFullScreen#Notes
https://developer.mozilla.org/en/DOM/Using_full-screen_mode#When_a_full-screen_request_fails
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.