Closed
Bug 932854
Opened 11 years ago
Closed 11 years ago
Consider showing a notification bar for hidden plugins
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(firefox26+ verified, firefox27+ verified, firefox28 verified, b2g-v1.2 fixed)
VERIFIED
FIXED
mozilla28
People
(Reporter: benjamin, Assigned: benjamin)
References
(Depends on 1 open bug)
Details
Attachments
(10 files, 4 obsolete files)
36.22 KB,
image/png
|
Details | |
42.67 KB,
image/png
|
Details | |
42.53 KB,
image/png
|
Details | |
11.65 KB,
patch
|
jaws
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
962 bytes,
patch
|
gfritzsche
:
review+
|
Details | Diff | Splinter Review |
1.23 KB,
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
3.79 KB,
patch
|
jaws
:
review+
|
Details | Diff | Splinter Review |
3.24 KB,
patch
|
jaws
:
review+
|
Details | Diff | Splinter Review |
139.35 KB,
image/png
|
Details | |
66.20 KB,
image/png
|
Details |
I think we need to consider showing a notification bar for hidden plugins. The current icon is not discoverable enough for users on some important banking sites. Here are the reasons I think we should prefer a notification bar instead of an auto-popup doorhanger, whether it stays on the page permanently or temporarily:
* when we had the auto-popup doorhanger, users had two opposite complaints: that it would pop up but interacting with the page would immediately dismiss it; and that they wanted to stop it from popping up but there wasn't any way to do that
* Users who are not paying attention when the page is loading may not see a temporary popup.
* I'm willing to bet that somebody could clickjack a popup.
I have a patch which shows a notification bar when plugins are hidden. The close button on the notification bar will close it permanently for that site.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #824708 -
Flags: review?(jaws)
Assignee | ||
Comment 2•11 years ago
|
||
Comment on attachment 824708 [details] [diff] [review]
Show a notification bar when a plugin is hidden. Explicitly closing the notification bar will hide it permanently for that site. r?jaws ui-r?madhav/lco
It seems I can only mark ui-review for a single person. lco your review is also desired!
Attachment #824708 -
Flags: ui-review?(madhava)
Comment 3•11 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #2)
> It seems I can only mark ui-review for a single person. lco your review is
> also desired!
http://logbot.glob.com.au/?c=mozilla%23developers&s=23+Sep+2013&e=23+Sep+2013&h=jaws#c766579
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 824708 [details] [diff] [review]
Show a notification bar when a plugin is hidden. Explicitly closing the notification bar will hide it permanently for that site. r?jaws ui-r?madhav/lco
Cancelling review: some bugs and lco is tweaking the design more than a little.
Attachment #824708 -
Flags: ui-review?(madhava)
Attachment #824708 -
Flags: review?(jaws)
Are you planning to keep the current navbar icon?
If you do, please consider having a pref to disable the notification bar.
Assignee | ||
Comment 6•11 years ago
|
||
When a site uses a hidden plugin, show a version of the in-content plugin UI at the top of the page for discoverability. It's cute that we can reuse the in-content binding for this, but it's also gotten poor reviews from the uifolk, so I'm just going to leave it here as a checkpoint and go back to a more normal button approach.
Assignee | ||
Comment 7•11 years ago
|
||
This (part A) produces a notification bar which uses the info/grey styling in all cases, and the only difference between a vulnerable and normal plugin is the icon. It is not clear yet whether the vulnerable plugin should be restyled to have the stripy background or the no-entry icon of the in-content UI
I will be preparing some screenshots and a try build, although the current UI is a boring as it sounds.
Attachment #824708 -
Attachment is obsolete: true
Attachment #826970 -
Attachment is obsolete: true
Attachment #827663 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 8•11 years ago
|
||
Try builds will be at https://tbpl.mozilla.org/?tree=Try&rev=4ad590b08e56
Assignee | ||
Comment 9•11 years ago
|
||
Hidden non-vulnerable screenshot.
Assignee | ||
Comment 10•11 years ago
|
||
Assignee | ||
Comment 11•11 years ago
|
||
Assignee | ||
Comment 12•11 years ago
|
||
Oh also! This doesn't implement the link that was in the screenshots. I can do that, although I'll have to modify the notification binding a bit (Enn helped me figure out what the API should be). I'd like to know whether to implement the link.
Updated•11 years ago
|
Attachment #827663 -
Flags: review?(jaws)
Comment 13•11 years ago
|
||
Comment on attachment 827663 [details] [diff] [review]
when a site uses a hidden plugin, show a notification bar to aid discoverability.
Review of attachment 827663 [details] [diff] [review]:
-----------------------------------------------------------------
I have two issues with the patch for this bug:
1) The screenshots here don't make much sense to me.
The text in the notification bar for the multiple plugin case says "Allow example.com to run plugins? [Block plugins] [Options]". The problem that we have today is that users who need to run the plugins aren't finding out how to do so.
So why are we giving such huge affordance towards blocking the plugins instead of running the plugins? Secondly, it seems that we are posing a question but visibly only providing one potential answer and hiding the other choices behind the "Options" button.
2) The purpose of this bug as I understand it is to introduce a notification bar for hidden plugins because we are finding that users are having a hard time "fixing" the website they are on when we block a hidden plugin. One may interpret this as that users are having a hard time learning/finding the plugin icon in the location bar.
If the user chooses to "Block Now" and then later regrets their decision, they won't have a path towards fixing their mistake because the notification bar won't reappear. Since we don't expect them to have figured out the plugin icon in the location bar, they won't have any recourse until the permission expires.
Attachment #827663 -
Flags: review?(jaws) → review-
Comment 14•11 years ago
|
||
(In reply to Jared Wein [:jaws] from comment #13)
> Comment on attachment 827663 [details] [diff] [review]
Lots of good stuff.
Jared please continue to bring your [not so]common-sense to the fore and challenge the dogma-driven hatred for plugins.
Ben will not be allowed to Fahrenheit-451 Applets. Well done that man!
Cheers Richard Maher
Comment 15•11 years ago
|
||
How about the following?
Allow example.com to run plugins? [Allow now] [Dismiss] [Options]
Where [Options] opens the doorhanger and allows the user to choose "Block Now", and [Dismiss] simply dismisses the notification bar but reloading the page will reshow the notification bar.
This seems clearer to me because the plugins are blocked by default, so choosing "Block Now" in this state seems redundant. The actions that a user can take here are basically 1) stop blocking the plugin; 2) don't bother me with this notification; 3) don't bother me with this notification now and in the future.
Comment 16•11 years ago
|
||
Bsmedberg, thanks for the screenshots, and Jaws, for the suggestions. Before we go in too deep here, I wanted to step back and explain the goals for the design of the infobar and our current restrictions.
To be clear, we're not aiming for the perfect design; we're aiming for something that is acceptable in FF26 and does not harm our users (whether we want to push back to FF27 is a different matter that's out of scope for discussion in this bug). Here's a concept that shorlander worked on which is closer to our ideal design: http://cl.ly/image/3R0w221e3z1e/o
In adding the infobar, we are trying to accomplish two goals:
1. Make the user's options apparent and clear to him/her without being too obtrusive
2. Direct the user to what we think is the "best", safe choice.
Goal #1:
Because just having the icon in the URL bar wasn't discoverable, it wasn't apparent to the user whether he had choices at all when it comes to hidden plugins. After some discussion on the design approach, we decided to go with an infobar. Like all of the other suggestions, there are some downsides to this approach, but we chose it because it's easily visible to the user without being obtrusive to their task.
I also agree that the current strings in Ben's screenshots can be improved upon because they still don't communicate the user's choices clearly. "Block Plugin" is confusing because the plugin IS already being blocked; the text we initially wanted was "Continue Blocking". We also all agree that "Options" is a terrible name, and doesn't truly describe the user's choice. However, one of the constraints we are working with is shipping these UI changes in FF26. This severely limits our string options because we can't propose any new strings.
I like Jaws' suggestion of replacing "Options" with "Allow" since it doesn't require us to add any new strings. We can't do much about "Continue Blocking", but I'm ok keeping it in for now, and iterating in the next release.
---
Goal #2:
For vulnerable plugins, the "best", safe choice we can give the user is to block the plugin automatically and emphasize that default. I know there are many cases where a knowledgeable user (or even just one willing to take the risk) should be allowed to choose to have the plugin run. But I want to increase the likelihood that the user knows what he's choosing, not just accidentally clicking a button. This is why we introduced the "Block Plugins" button; I want to increase the cognitive friction for users who want to enable a potentially dangerous plugin. I also want the user to understand that allowing a plugin is not his only choice.
Again, Jaws' suggestion of "Allow" to replace "Options" is fine here, because clicking that button doesn't automatically enable the plugin, but rather, opens the doorhanger with the user's choices. Therefore, this is a higher friction, but still discoverable path.
For regular plugins, the "best" choice is a different matter, because we have less confidence in how unsafe the plugin really is. Thus, we err on the side of assuming that it's safe to run the plugin, if the user knows what it is. If possible, I would actually prefer providing the "Allow Now" and "Allow and Remember" choices in the infobar, rather than than the "Block Plugin" button that's currently in the screenshot.
---
Again, I don't think the screenshots that Ben has proposed amount to the best solution. I know there are many good ideas, and I don't want to lose them. But I also want to convey the design choices we had to make given the restrictions we had so that we can move forward with the implementation.
Comment 17•11 years ago
|
||
The "Allow Now" and "Allow and Remember" strings are available in Beta. I'm not sure why we're trying work around using them in the infobar?
At the risk of sounding like a curmudgeon, it seems we are settling for a lower potential solution when a better solution is available.
Comment 18•11 years ago
|
||
(In reply to Jared Wein [:jaws] from comment #17)
> The "Allow Now" and "Allow and Remember" strings are available in Beta. I'm
> not sure why we're trying work around using them in the infobar?
>
> At the risk of sounding like a curmudgeon, it seems we are settling for a
> lower potential solution when a better solution is available.
I don't think you're being curmudgeonly :) My suggestion as well would be:
For the vulnerable plugins case -- use "Block Plugin" and "Allow"
For the regular plugins case -- use "Allow and Remember" and "Allow Now"
Comment 19•11 years ago
|
||
Comment on attachment 827663 [details] [diff] [review]
when a site uses a hidden plugin, show a notification bar to aid discoverability.
I believe this was sorted out by bsmedberg/lco/jaws/madhava elsewhere and a new patch is coming?
Attachment #827663 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 20•11 years ago
|
||
Attachment #827663 -
Attachment is obsolete: true
Attachment #829764 -
Flags: review?(jaws)
Comment 21•11 years ago
|
||
Comment on attachment 829764 [details] [diff] [review]
when a site uses a hidden plugin, show a notification bar to aid discoverability. This patch, suitable for trunk and branch, uses existing strings which are not ideal.
Review of attachment 829764 [details] [diff] [review]:
-----------------------------------------------------------------
r=me but I would very much prefer to not introduce another !important here if it is not really necessary.
::: browser/themes/shared/plugin-doorhanger.inc.css
@@ +60,5 @@
> + color: white;
> +}
> +
> +notification.pluginVulnerable .messageImage {
> + list-style-image: url("chrome://browser/skin/notification-pluginBlocked.png") !important;
Why is the !important needed here? The tagname + 2 class selectors should put the specificity higher than the class + attribute selectors that are used for the |.messageImage[value="plugin-hidden"]| selector above.
Attachment #829764 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 22•11 years ago
|
||
Attachment #830834 -
Flags: review?(georg.fritzsche)
Comment 23•11 years ago
|
||
Comment on attachment 830834 [details] [diff] [review]
Fix tests to avoid infobar in chrome tests
Review of attachment 830834 [details] [diff] [review]:
-----------------------------------------------------------------
This looks good to me.
Attachment #830834 -
Flags: review?(georg.fritzsche) → review+
Assignee | ||
Comment 24•11 years ago
|
||
I hate tests with side effects. I also hate the mochitest chrome harness.
Comment 25•11 years ago
|
||
Backed out for Windows mochitest-other crashes.
https://hg.mozilla.org/integration/fx-team/rev/5fa603642231
https://tbpl.mozilla.org/php/getParsedLog.php?id=30490234&tree=Fx-Team
Assignee | ||
Comment 26•11 years ago
|
||
Attachment #831825 -
Flags: review?(jmathies)
Assignee | ||
Comment 27•11 years ago
|
||
I have a bunch of followups to file around tests that are doing variously stupid things, to pay down some technical debt.
Updated•11 years ago
|
Attachment #831825 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 28•11 years ago
|
||
Attachment #832213 -
Flags: review?(jaws)
Assignee | ||
Comment 29•11 years ago
|
||
Attachment #830975 -
Attachment is obsolete: true
Attachment #832220 -
Flags: review?(jaws)
Comment 30•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/be954182277a
https://hg.mozilla.org/mozilla-central/rev/aafde14ba277
https://hg.mozilla.org/mozilla-central/rev/c09869a04882
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Updated•11 years ago
|
Attachment #832213 -
Flags: review?(jaws) → review+
Updated•11 years ago
|
Attachment #832220 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 31•11 years ago
|
||
reopening for the last two patches.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 32•11 years ago
|
||
Comment on attachment 829764 [details] [diff] [review]
when a site uses a hidden plugin, show a notification bar to aid discoverability. This patch, suitable for trunk and branch, uses existing strings which are not ideal.
[Approval Request Comment]
Bug caused by (feature/regressing bug #): CtP-by-default made this more urgent, but it also affects existing users who have out-of-date Java or Flash.
User impact if declined: We cannot turn on the java block by default
Testing completed (on m-c, etc.): landed in nightly
Risk to taking this patch (and alternatives if risky): risk is localized to the CtP code, so targeted testing should show any issues. Heavyweight themes would also need some updates to style the icons properly, but wouldn't cause breaking changes.
String or IDL/UUID changes made by this patch: none
Attachment #829764 -
Flags: approval-mozilla-beta?
Attachment #829764 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 33•11 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/6ce923a22a9e (telemetry)
https://hg.mozilla.org/integration/fx-team/rev/1458d327affd (trunk-only strings)
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Comment 35•11 years ago
|
||
Updated•11 years ago
|
Attachment #829764 -
Flags: approval-mozilla-beta?
Attachment #829764 -
Flags: approval-mozilla-beta+
Attachment #829764 -
Flags: approval-mozilla-aurora?
Attachment #829764 -
Flags: approval-mozilla-aurora+
Updated•11 years ago
|
status-firefox26:
--- → affected
status-firefox27:
--- → affected
status-firefox28:
--- → fixed
tracking-firefox26:
--- → +
tracking-firefox27:
--- → +
Assignee | ||
Comment 37•11 years ago
|
||
Checkin-needed to uplift the following 4 csets to the branches:
https://hg.mozilla.org/mozilla-central/rev/be954182277a
https://hg.mozilla.org/mozilla-central/rev/aafde14ba277
https://hg.mozilla.org/mozilla-central/rev/c09869a04882
https://hg.mozilla.org/integration/fx-team/rev/6ce923a22a9e
The first will require a browser/base/content/tests/general rename, the rest should apply straight. If there are any issues I'll deal with it myself in the morning.
Keywords: checkin-needed
Comment 38•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/b1a341fb814f
https://hg.mozilla.org/releases/mozilla-aurora/rev/598daa4e76e1
https://hg.mozilla.org/releases/mozilla-aurora/rev/f3a99bcfe01b
https://hg.mozilla.org/releases/mozilla-aurora/rev/97325de07f00
https://hg.mozilla.org/releases/mozilla-beta/rev/02d3ec08c22d
https://hg.mozilla.org/releases/mozilla-beta/rev/76fc8e53c514
https://hg.mozilla.org/releases/mozilla-beta/rev/61fc6330e610
https://hg.mozilla.org/releases/mozilla-beta/rev/adb1127cd412
Comment 39•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/02d3ec08c22d
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/76fc8e53c514
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/61fc6330e610
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/adb1127cd412
Flags: in-testsuite+
Updated•11 years ago
|
status-b2g-v1.2:
--- → fixed
Comment 40•11 years ago
|
||
Verified as fixed with Firefox 26 beta 7 (build ID: 20131122094025) on: Win 7 64-bit, XP 32-bit, Mac OS X 10.7 and Ubuntu 12.04 32-bit.
The notification bar attached in the screenshot is visible. Tested with:
http://benjamin.smedbergs.us/tests/ctptests/flash-hidden.html
http://benjamin.smedbergs.us/tests/ctptests/javaflash-hidden.html
http://benjamin.smedbergs.us/tests/ctptests/java-hidden.html
http://www.drive.google.com
https://www.alipay.com
I assume that after clicking the "Block Plugin" button from the notification bar and then restarting the browser, it's expected for the notification bar to not appear anymore.
Updated•11 years ago
|
Comment 41•11 years ago
|
||
(In reply to Manuela Muntean [:Manuela] [QA] from comment #40)
> I assume that after clicking the "Block Plugin" button from the notification
> bar and then restarting the browser, it's expected for the notification bar
> to not appear anymore.
Yes, that's expected. If needed for testing you can reset this as in bug 941449, comment 13 & 14.
Comment 42•11 years ago
|
||
It would be useful if this could be a configurable option. If you operate in a disabled-by-default mode, the bar seems to "nag" you to turn on plugins. For 90% of the sites I visit, I don't want to enable any plugins. On the rare (10%) exceptions, I'd rather manually enable the plugin by clicking the disabled element (using https://addons.mozilla.org/en-US/firefox/addon/click-to-play-per-element/?src=api as devs have disabled per-element click-to-play) or by clicking the icon on the left of the address bar.
Could we consider making this an about:config option or a preference?
Assignee | ||
Comment 43•11 years ago
|
||
The "Continue Blocking" button will hide the infobar on that site permanently. See also bug 942461.
Comment 44•11 years ago
|
||
Thanks :bsmedberg, but my issue is that a vast number of websites use Flash (mostly for ads), requiring me to "do something" on the notification bar for every website I visit (for the first time) - this seems excessive when all I want to do is say "never enable any plugin unless I click on a placeholder (usually a video or game, and one of the many accompanying ad panels".
I'll keep an eye on 942461.
Comment 45•11 years ago
|
||
A pref would be nice, but I think that in our current situation, with UX unfinished and Flash ubiquitous, this notification bar is actually doing more hurt than good. I filed 943382 to argue that case.
Comment 46•11 years ago
|
||
Verified fixed FF 28.0a1 2013-11-25 win 7
Comment 47•11 years ago
|
||
FWIW, the X button is barely seen on Windows for outdated plugins.
Comment 48•11 years ago
|
||
Verified as fixed with on Aurora 27.0a2 (build ID: 20131128004001) on: Win 7 64-bit, Mac OS X 10.8.5 and Ubuntu 12.04 32-bit.
Comment 49•11 years ago
|
||
Why does it affect the spotlight-module on youtube? And when I allow the plugin, the videoplayer is broken.
Flags: needinfo?(benjamin)
Assignee | ||
Comment 50•11 years ago
|
||
sjw, I don't know what you're talking about, but commenting on this closed bug is probably not the best way. If you think there's a bug, could you please file it with specific steps to reproduce/results/etc?
Flags: needinfo?(benjamin)
Comment 51•11 years ago
|
||
This feature doesn't work at all for me nowadays on http://connect.garmin.com/transfer/upload#. The plugin is hidden but even when I allow it in the add-on manager, or set to ask each time, its never getting started nor get I a notification. This has been regressed with Firefox 28 beta. I will investigate today and file a follow-up bug.
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•