854952 - Make the fullscreen permission prompt prettier

Status: RESOLVED FIXED

(Firefox :: Theme, enhancement)

Description

[Wesley Johnston (:wesj)]

I sorta dread hitting the fullscreen button on things because it shows a prettty ugly dark gray rounded box near the top of the screen. It makes me cringe. I think we can do something prettier.

Comment 1

[Justin Dolske [:Dolske]]

Attachment: Screenshot of current prompt

Can you say more about what specifically you find bad about it?

It's better than when it first landed, but there are a couple of things I can easily see improving:

* Mentions "fullscreen" repeatedly
* Is a little odd to have it asking "allow" when it's already fullscreen (meaning, it's not so much "allow" as "make this warning go away")
* No button focused by default
* Overall a bunch of text and buttons and checkboxes

[Any HTML5 video will let you trigger the prompt. I recommend https://people.mozilla.com/~dolske/tmp/rickroll.ogg]

Comment 2

[Justin Dolske [:Dolske]]

Idea:

    site.com is now fullscreen.
   Press Esc at any time to exit.

   [Exit now] [Always Allow] [Ok]


No auto-deny by default. Either add it the 2nd time the prompt is shown (because the site is annoying you), or add a separate "So it looks like this site is annoying you" prompt if it's triggered repeatedly.

Or, as a smaller change:

    site.com is now fullscreen.
   Press Esc at any time to exit.

      [Deny]       [Allow]
    [x] Remember my decision

Comment 3

[Justin Dolske [:Dolske]]

Bug 716107 added this prompt, there's some light discussion about it there.

Comment 4

[Wesley Johnston (:wesj)]

I guess I find it intrusive, being center top of the screen, where the content I'm interested in likely is. I would guess it was designed to be that way so that I'd notice it(?), but it makes my first interaction with the site in full screen annoying. I can't do anything until I've interacted with it. i.e. Its referred to numerous times in bug 716107 as a warning, when really I think we just want an escape hatch for the odd case where a site is doing this without permission + instructions on how to get out?. I'm not really sure what its warning me about.

I would rather something that's obvious to me and easy to find for sites that try to enter fullscreen without permission, but inconspicuous enough that I don't feel compelled to interact with it on sites where I've requested it. I want less of a warning and more of an escape-hatch + info bar. (To be honest, I'm not positive we need this at all. Maybe we could just by default show a quick "Press ESC to exist" message, and only show the full prompt if a site seemed to be prompting a lot in a very short period of time?)

It also reminds me of my mobile video controls that I hate and have been trying to remove (see bug 704229). I don't think a larger rounded box like this in the middle of the screen feels "modern" enough.

In my head, I picture something along the bottom of the screen, full width, and similar to our video controls (but maybe with content only in a max-width section centered in the bar?). Maybe IE's permission prompts are an example of something similar, although I'm not a huge fan of their design. Or Android toasts? There's also no reason we need to use native UI buttons (theres no native ui visible anymore, I don't think users expect it in any other full screen apps they ever use).

Comment 5

[Wesley Johnston (:wesj)]

I forgot, as a warning it should be useful to prevent spoofing. I've never read it that hard that I think it would be effective, but I haven't had a site spontaneously go into fullscreen yet either. If I did, I think I would care more about just getting out than about finding out what url I was viewing....

Comment 6

[whispersofthedustandechoes]

Hello! I would love to help fix this bug! :)

Comment 7

[katsarjp]

Hello! I would love to help fix this bug! :)

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to whispersofthedustandechoes from comment #6)
> Hello! I would love to help fix this bug! :)

(In reply to katsarjp from comment #7)
> Hello! I would love to help fix this bug! :)

Wow, the two of you were really quick to this bug. Unfortunately I had already been talking with katsarjp about taking this bug, so I'll assign it to katsarjp.

whispersofthedustandechoes, if you would still like to work on it, you could propose your own patches too of course.

Comment 9

[whispersofthedustandechoes]

Sorry! I am katsarjp! I just realized I signed onto the wrong account! Sorry for the confusion. :/

Comment 10

[Josh Matthews [:jdm]]

Are you still working on this, katsarjp?

Comment 11

[Tiziana Sellitto [:tiziana]]

Attachment: Deleted 'fullscreenApproval.value' from fullscreen approval pane - changed button labels and fullscreen exit hint message - halved border radius

A preliminary patch

Comment 12

[Tiziana Sellitto [:tiziana]]

Attachment: preliminary prompt screenshot

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 779266 [details] [diff] [review]
Deleted 'fullscreenApproval.value' from fullscreen approval pane - changed button labels and fullscreen exit hint message - halved border radius

Review of attachment 779266 [details] [diff] [review]:
-----------------------------------------------------------------

Nice job on your first patch!

So this is the prompt today:
----------------------------------------------
|                                            |
|      mozilla.com is now fullscreen         |
|  Press ESC at any time to exit fullscreen  |
|            Allow fullscreen?               |
|             [Allow] [Deny]                 |
|    [ ] Remember decision for mozilla.com   |
|                                            |
----------------------------------------------

I like your changes so far (although we'll have to change the entity names to signal to localizers that they will have to update their translations).

I'd like to see a combination of Dolske's recommendation along with some alignment changes, like so:

(Windows, Linux)
----------------------------------------------
|                                            |
|      mozilla.com is now fullscreen.        |
|      Press ESC at any time to exit.        |
|                                            |
|                                            |
|                 [Allow Fullscreen] [Exit]  |
|     [x] Remember decision for mozilla.com  |
----------------------------------------------

(OS X)
----------------------------------------------
|                                            |
|      mozilla.com is now fullscreen.        |
|      Press ESC at any time to exit.        |
|                                            |
|                                            |
|                 [Exit] [Allow Fullscreen]  |
|     [x] Remember decision for mozilla.com  |
----------------------------------------------

It would also be nice to add some texture to the background. You can copy the /toolkit/themes/windows/global/media/imagedoc-darknoise.png and /toolkit/themes/osx/global/media/imagedoc-darknoise.png to /browser/themes/windows/fullscreen-darknoise.png and /browser/themes/osx/fullscreen-darknoise.png, respectively.

Comment 14

[Dão Gottwald [:dao]]

"Remember decision" really only makes sense for something like the Allow/Deny combo. In combination with "Exit" it seems weird, as "Exit" doesn't feel like a permission. I don't know how we arrived here, because comment 2 didn't propose it.

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

Thinking about this more, Tiziana, can you go further with implementing the suggestion in comment #2?

Comment 16

[Tiziana Sellitto [:tiziana]]

yes, I can. I could change the Button labels to "Allow/Deny" creating something like the second idea of comment 2 

-----------------------------------
|    site.com is now fullscreen.   |
|   Press Esc at any time to exit. |
|                                  |
|      [Deny]       [Allow]        |
|    [x] Remember my decision      |
------------------------------------

Should I use suggestions on comment 13 as for alignment, background texture and, button's order based on the platform?

Comment 17

[Jared Wein [:jaws] (please needinfo? me)]

Only keep the background texture and button order from comment #13.

Comment 18

[Tiziana Sellitto [:tiziana]]

Attachment: Bug 854952 - Deleted fullscreen approval question from fullscreen approval pane - changed fullscreen exit hint message fullscreenExitHint2 - halved border radius - added background texture - changed

I've not tested the patch on linux because I don't have a linux machine at the moment.

Comment 19

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 783808 [details] [diff] [review]
Bug 854952 - Deleted fullscreen approval question from fullscreen approval pane - changed fullscreen exit hint message fullscreenExitHint2 - halved border radius - added background texture - changed

Redirecting review since I don't think I'll have time to get to this.

Comment 20

[Justin Dolske [:Dolske]]

Comment on attachment 783808 [details] [diff] [review]
Bug 854952 - Deleted fullscreen approval question from fullscreen approval pane - changed fullscreen exit hint message fullscreenExitHint2 - halved border radius - added background texture - changed

Review of attachment 783808 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with the XP_UNIX change.

::: browser/base/content/browser.xul
@@ +1057,2 @@
>            <hbox>
> +#ifdef XP_MACOSX

XP_UNIX

Since that's what we're using in https://mxr.mozilla.org/mozilla-central/source/toolkit/components/prompts/content/tabprompts.xml#63

::: browser/themes/linux/browser.css
@@ +2037,2 @@
>    color: white;
> +  border-radius: 4px;

Tab-modal prompts are 2px, but this is a fine halfway step. :) We could consider a closer style unification in a followup.

Comment 21

[Tiziana Sellitto [:tiziana]]

Attachment: Deleted fullscreen approval question from fullscreen approval pane - changed fullscreen exit hint message fullscreenExitHint2 - halved border radius-added background texture - changed button order

Thanks for the review. I've changed the XP_UNIX and rebased the patch.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/a416967938b7

Comment 23

[Tim Taubert [:ttaubert] (inactive)]

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