Make the fullscreen permission prompt prettier

RESOLVED FIXED in Firefox 25

Status

()

Firefox
Theme
--
enhancement
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: wesj, Assigned: tiziana)

Tracking

Trunk
Firefox 25
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=jaws][lang=css][lang=js][lang=xul])

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

4 years ago
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.

Updated

4 years ago
Severity: normal → enhancement
OS: Linux → All
Hardware: x86_64 → All
Summary: Fullscreen permission prompt is kinda ugly → Make the fullscreen permission prompt prettier
Version: unspecified → Trunk
Created attachment 731006 [details]
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]
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
Bug 716107 added this prompt, there's some light discussion about it there.
(Reporter)

Comment 4

4 years ago
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).
(Reporter)

Comment 5

4 years ago
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....
Whiteboard: [good first bug][mentor=jaws][lang=css][lang=js][lang=xul]
Hello! I would love to help fix this bug! :)

Comment 7

4 years ago
Hello! I would love to help fix this bug! :)
(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.
Assignee: nobody → katsarjp
Status: NEW → ASSIGNED
Sorry! I am katsarjp! I just realized I signed onto the wrong account! Sorry for the confusion. :/
Are you still working on this, katsarjp?
Flags: needinfo?(katsarjp)
Assignee: katsarjp → tiziana.sel
Flags: needinfo?(katsarjp)
(Assignee)

Comment 11

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

A preliminary patch
Attachment #779266 - Flags: feedback?(jaws)
(Assignee)

Comment 12

4 years ago
Created attachment 779268 [details]
preliminary prompt screenshot
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.
Attachment #779266 - Flags: feedback?(jaws) → feedback+
"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.
Thinking about this more, Tiziana, can you go further with implementing the suggestion in comment #2?
(Assignee)

Comment 16

4 years ago
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?
Only keep the background texture and button order from comment #13.
(Assignee)

Comment 18

4 years ago
Created 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

I've not tested the patch on linux because I don't have a linux machine at the moment.
Attachment #779266 - Attachment is obsolete: true
Attachment #783808 - Flags: review?(jaws)
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.
Attachment #783808 - Flags: review?(jaws) → review?(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.
Attachment #783808 - Flags: review?(dolske) → review+
(Assignee)

Comment 21

4 years ago
Created attachment 785119 [details] [diff] [review]
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.
Attachment #779268 - Attachment is obsolete: true
Attachment #783808 - Attachment is obsolete: true
Attachment #785119 - Flags: checkin?(dolske)
https://hg.mozilla.org/integration/fx-team/rev/a416967938b7
Whiteboard: [good first bug][mentor=jaws][lang=css][lang=js][lang=xul] → [good first bug][mentor=jaws][lang=css][lang=js][lang=xul][fixed-in-fx-team]
Attachment #785119 - Flags: checkin?(dolske) → checkin+
https://hg.mozilla.org/mozilla-central/rev/a416967938b7
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][mentor=jaws][lang=css][lang=js][lang=xul][fixed-in-fx-team] → [good first bug][mentor=jaws][lang=css][lang=js][lang=xul]
Target Milestone: --- → Firefox 25
You need to log in before you can comment on or make changes to this bug.