Last Comment Bug 688648 - Dispatch mozfullscreenerror event when requests for full-screen are denied
: Dispatch mozfullscreenerror event when requests for full-screen are denied
Status: RESOLVED FIXED
[inbound]
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla10
Assigned To: Chris Pearce (:cpearce)
:
Mentors:
Depends on: 685779
Blocks: 545812 695935
  Show dependency treegraph
 
Reported: 2011-09-22 16:33 PDT by Chris Pearce (:cpearce)
Modified: 2012-02-07 09:51 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (21.11 KB, patch)
2011-10-27 14:29 PDT, Chris Pearce (:cpearce)
bugs: review+
Details | Diff | Review
Patch v2: Rebased (22.31 KB, patch)
2011-11-01 00:01 PDT, Chris Pearce (:cpearce)
bugs: review-
Details | Diff | Review
Patch v3 (22.71 KB, patch)
2011-11-01 15:38 PDT, Chris Pearce (:cpearce)
bugs: review+
Details | Diff | Review
Patch v4 (23.23 KB, patch)
2011-11-01 16:26 PDT, Chris Pearce (:cpearce)
cpearce: review+
Details | Diff | Review

Description Chris Pearce (:cpearce) 2011-09-22 16:33:05 PDT
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
Comment 1 Chris Pearce (:cpearce) 2011-10-19 18:52:43 PDT
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.
Comment 2 Chris Pearce (:cpearce) 2011-10-19 18:59:42 PDT
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...
Comment 3 Chris Pearce (:cpearce) 2011-10-27 14:29:07 PDT
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.
Comment 4 Olli Pettay [:smaug] 2011-10-29 12:19:14 PDT
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.)
Comment 5 Chris Pearce (:cpearce) 2011-10-30 19:15:18 PDT
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.
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-10-30 19:25:55 PDT
ok
Comment 7 Chris Pearce (:cpearce) 2011-11-01 00:01:50 PDT
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. ;)
Comment 8 Olli Pettay [:smaug] 2011-11-01 12:59:03 PDT
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.
Comment 9 Chris Pearce (:cpearce) 2011-11-01 15:38:55 PDT
Created attachment 571168 [details] [diff] [review]
Patch v3

With nsPLDOMEvent.
Comment 10 Olli Pettay [:smaug] 2011-11-01 15:44:25 PDT
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+
Comment 11 Chris Pearce (:cpearce) 2011-11-01 16:26:17 PDT
Created attachment 571180 [details] [diff] [review]
Patch v4

Without nsContentUtils::DispatchFullScreenEvent.
Comment 13 Ed Morley [:emorley] 2011-11-02 06:32:09 PDT
https://hg.mozilla.org/mozilla-central/rev/5a6ea5db053e

Note You need to log in before you can comment on or make changes to this bug.