Closed Bug 895939 Opened 6 years ago Closed 6 years ago

Click to play doorhanger notification is ugly

Categories

(Firefox :: Theme, defect)

24 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 25
Tracking Status
firefox24 + verified
firefox25 --- verified

People

(Reporter: wesj, Assigned: benjamin)

References

Details

Attachments

(3 files)

The new click to play doorhangers seem a bit unstyled on linux? Or maybe its my distro (XFCE)? (I also kinda hate that I now have to click twice to unblock things, but I assume that was intentional?)

Apologies for the screenshot being a bit faded, but it gives a general idea.
Blocks: 880735
Is the transparency specific to this doorhanger, or do you have transparency on all your popups? Otherwise, this is a dup of bug 888510. Can you mark if appropriate?
Flags: needinfo?(wjohnston)
Based on Wes' comment #0 I don't think the translucent panel is something that is consistent, I think it was just timing of when he took the screenshot.

Repurposing this bug to make sure that the UI is cleaned up for release in 24, as we can't ship it in its current form.

From bug 888510 comment #17,
> At the least, we will need to add some more padding between the buttons and the
> edges of the panel, change the background color in the box surrounding the
> buttons (likely to match http://screencast.com/t/tUvmSFvy) and we also need to
> align the close button properly.

Adding tracking-24 request because we plan on enabling CtP for users of Java in 24 so many users will be seeing this UI without an opt-in.
Assignee: nobody → benjamin
Flags: needinfo?(wjohnston)
OS: Linux → All
Hardware: x86_64 → All
Summary: Click to play notification is ugly → Click to play doorhanger notification is ugly
Version: unspecified → 24 Branch
Ideally it would look like http://people.mozilla.com/~shorlander/files/click-to-play-prototype/clickToPlay-Mockup-03.html.

From IRC,
> 5:14 PM <bsmedberg> as I noted in the bug, I couldn't make that because of
> focus styling on the buttons, and I don't know how to style the default button
> 5:14 PM <bsmedberg> and dao said I should use native buttons, so I kept them

This is why I have recommended the simpler changes in comment #2.
Can we also drop the italics for the host name? I don't think we do this anywhere else in the UI.
Forgot to reply in here, but the fading is just caused by me hitting print-screen and defocusing the popup.
The italic hostname was specified by lco. We both agree that we want to highlight the host in some way. If there is a way that is more consistent with the rest of the app, I'm happy to try it, but I would oppose just making it normal dialog text.
What's special about this doorhanger compared to all others that requires the highlight?
Which other doorhangers are we talking about? The identity doorhanger highlights SSL sites in large bold lettering. What other UI shows the domain in this kind of context?
The identity panel isn't a doorhanger. Doorhanger is another name for site-specific notifications using PopupNotifications.jsm. See https://bugzilla.mozilla.org/show_bug.cgi?id=doorhanger

We have quite a few of them: http://mxr.mozilla.org/mozilla-central/search?string=PopupNotifications.show

The list of anchor nodes can be used as a more readable proxy for the list of our doorhanger notifications, in case you're curious: http://hg.mozilla.org/mozilla-central/annotate/46d73e889cb4/browser/base/content/browser.xul#l498
Attached image windows drop down
This is pretty ugly on Windows as well. First time I viewed this I thought it was unfinished UI. Also, not sure why clicking on an individual flash applet forces the display of this for the entire site. But that's probably in another bug.
I see at least two different styles already. The storage popup uses "This site (foo.com) ...". The geolocation popup doesn't separate the domain at all. Mixed-content doesn't include the domain at all.

Given that lco specified the current appearance, I am not planning on changing it in this patch/bug. If you think it's important, please file another bug and reach a decision with her. I do think that making the domain obvious is an important part of the dialog, since it makes it easier to scan what site the user is acting on.

I have a patch I'm testing now which copies basic styles from the identity popup: jaws has reviewed the screenshots here and I'm waiting on a mac build to verify that the close button is in the correct position there.

Single plugin: http://www.screencast.com/users/bsmedberg/folders/Default/media/13837c5a-9f50-4c49-b66b-2055b1fcae34
multi-plugin: http://www.screencast.com/users/bsmedberg/folders/Default/media/0d8b7137-2252-48c5-9da5-366e42034b5d
Attachment #781897 - Flags: review?(jaws)
Comment on attachment 781897 [details] [diff] [review]
Click-to-activate plugin notification is ugly, r?jaws

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

::: browser/themes/shared/plugin-doorhanger.inc.css
@@ +28,5 @@
>    -moz-margin-start: 6px;
>  }
>  
>  .click-to-play-plugins-notification-button-container {
> +  background: linear-gradient(to bottom, rgba(0,0,0,0.04) 60%, transparent);

The 'to bottom,' part isn't needed because it's the default direction for gradients.
Attachment #781897 - Flags: review?(jaws) → review+
https://hg.mozilla.org/mozilla-central/rev/c456780d7d1f
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Comment on attachment 781897 [details] [diff] [review]
Click-to-activate plugin notification is ugly, r?jaws

[Approval Request Comment]
Bug caused by (feature/regressing bug #): design changes in bug 880735
User impact if declined: The doorhanger is unacceptably cramped and not visually appealing
Testing completed (on m-c, etc.): Manual verification on all platforms, landed on m-c
Risk to taking this patch (and alternatives if risky): Very low risk: margin/styling changes only
String or IDL/UUID changes made by this patch: None
Attachment #781897 - Flags: approval-mozilla-aurora?
Comment on attachment 781897 [details] [diff] [review]
Click-to-activate plugin notification is ugly, r?jaws

low risk, cosmetic change looks good to land
Attachment #781897 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
No longer blocks: 899829
(In reply to Benjamin Smedberg  [:bsmedberg] PTO 8-Aug until 18-Aug, workweek high latency 19-Aug through 23-Aug from comment #11)
> multi-plugin:
> http://www.screencast.com/users/bsmedberg/folders/Default/media/0d8b7137-
> 2252-48c5-9da5-366e42034b5d
Why is there a darker part around the first plugin in the list?
(In reply to Paul Silaghi [QA] from comment #19)
> (In reply to Benjamin Smedberg  [:bsmedberg] PTO 8-Aug until 18-Aug,
> workweek high latency 19-Aug through 23-Aug from comment #11)
> > multi-plugin:
> > http://www.screencast.com/users/bsmedberg/folders/Default/media/0d8b7137-
> > 2252-48c5-9da5-366e42034b5d
> Why is there a darker part around the first plugin in the list?

That is because the rows are supposed to be "zebra colored", meaning that if there was a third row it would also be dark. This should help people when reading from left-to-right to know what buttons apply to what row.
Thanks Jared.
The doorhanger looks ok on FF 24b1, 25.0a2 2013-08-07 Win 7, Mac OS X 10.8, Ubuntu 13.04
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.