781384 - Allow web app script to exit fullscreen

Status: RESOLVED FIXED

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

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

Web apps can enter fullscreen when not in user generated event handlers [1]. We should have the same provision for exiting fullscreen! Currently webapps can enter, but not exit fullscreen from script when running in non-user generated event handlers!

[1] http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsGenericElement.cpp#2813

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch: allow mozCancelFullScreen from web apps in web app origin

Comment 2

[Justin Lebar (not reading bugmail)]

I'm removing the window::IsInAppOrigin calls in bug 777135, which should land in a day or two.  The shiny new way of doing this is

  document->NodePrincipal()->AppStatus() >= NO_APP

.  r=me with that change.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

As I understand it, we'll need bug 781620 to land before the code change in comment #2 starts behaving as required here.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch, rebased on top of permission changes.

Justin, I presume this is what the patch needs to be? I'm assuming the change in Bug 781620 (or wherever) will land at some stage, so that nsIPrincipal::GetAppStatus() returns < nsIPrincipal::APP_STATUS_INSTALLED for origins not inside the app origin.

I want to land this patch even though it enables script in apps' subdocuments which are outside of the app origin to cancel fullscreen, as without this change the Gaia system app can't cancel fullscreen and so we're having fullscreen permission dialogs hanging around on B2G.

Comment 5

[Justin Lebar (not reading bugmail)]

> Justin, I presume this is what the patch needs to be?

Famous last words, but: That should work.  :)

> I want to land this patch even though it enables script in apps' subdocuments which are outside of 
> the app origin to cancel fullscreen, as without this change the Gaia system app can't cancel 
> fullscreen and so we're having fullscreen permission dialogs hanging around on B2G.

Can you elaborate on what's going wrong here?  It seems like we should make it so the system app can cancel fullscreen while scripts in iframes outside the app origin can't.  Or is that the wrong model?

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Justin Lebar [:jlebar] from comment #5)
> > I want to land this patch even though it enables script in apps' subdocuments which are outside of 
> > the app origin to cancel fullscreen, as without this change the Gaia system app can't cancel 
> > fullscreen and so we're having fullscreen permission dialogs hanging around on B2G.
> 
> Can you elaborate on what's going wrong here? 

Sure. Without this patch, this line of code in the system app has no effect:
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/permission_manager.js#L37

This is because Gecko's current code only honours Document.mozCancelFullScreen() calls in user generated event handlers and the "noCallback" here (which is called when the user clicks the "no don't allow this origin to go fullscreen" button on the permission dialog) doesn't run in a user generated event handler.

I don't think this is the only problem causing fullscreen permission dialogs to hang around, but it's at least one issue. I'm going to debug this today...

> It seems like we should make
> it so the system app can cancel fullscreen while scripts in iframes outside
> the app origin can't.  Or is that the wrong model?

That's the plan, and my hope is that once we land the patch I have here, and once we make the change you're discussing with Fabrice in bug 781620, then that model should be implemented. Right?

Comment 7

[Justin Lebar (not reading bugmail)]

> That's the plan, and my hope is that once we land the patch I have here, and once we make the change 
> you're discussing with Fabrice in bug 781620, then that model should be implemented. Right?

Oh, I see.  So we're only allowing scripts in iframes outside the app origin to exit full-screen until we fix bug 781620.  Sounds great.

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

Hmm, not quite, I wasn't very clear.

To be explicit, the behaviour we'll have under the various cases with various patches landed is:

Without this patch or bug 781620 landed: Any fullscreen document (including documents which are same-origin and which are not same-origin to the enclosing app) can exit fullscreen only when calling Document.mozCancelFullScreen() in a user generated event handler.

Once this patch lands: Any fullscreen document whose origin is or is not same-origin to the enclosing app can exit fullscreen.

Once we change nsIPrincipal::GetAppStatus() to return APP_STATUS_NOT_INSTALLED for documents loaded by apps which have origins which aren't the app's origin (as you're discussing with Fabrice in Bug 781620): Any document which is fullscreen and whose origin is the app origin, OR any fullscreen document which isn't same-origin to the enclosing app provided the call to Document.mozCancelFullScreen() is running in a user generated event handler.

Hopefully that is clearer and makes sense.

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Chris Pearce (:cpearce) from comment #8)
> Once we change nsIPrincipal::GetAppStatus() to return
> APP_STATUS_NOT_INSTALLED for documents loaded by apps which have origins
> which aren't the app's origin (as you're discussing with Fabrice in Bug
> 781620): Any document which is fullscreen and whose origin is the app
> origin, OR any fullscreen document which isn't same-origin to the enclosing
> app provided the call to Document.mozCancelFullScreen() is running in a user
> generated event handler.
> 
> Hopefully that is clearer and makes sense.

Heh, that second-to-last paragraph should read:

Any document which is fullscreen and whose origin is the app origin, OR any fullscreen document which isn't same-origin to the enclosing app provided the call to Document.mozCancelFullScreen() is running in a user generated event handler *can exit fullscreen*.

*Now* that should be clearer. ;)

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f674f3e9d064

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f674f3e9d064

Comment 12

[Justin Lebar (not reading bugmail)]

> Hopefully that is clearer and makes sense.

Yes, and that sounds good.  Thanks.