388445 - Notify user when plugin fails to load due to blocklisting

Status: VERIFIED FIXED

(Firefox :: General, defect, P2)

Description

[Michael Wu [:mwu]]

The user should be notified when a plugin on a page wasn't loaded because it was blocklisted.

Comment 1

[Jesse Ruderman]

Should the UI text differentiate between plugins blocked for different reasons?  Perhaps there should be one message for a plugin that needs to be upgraded and a generic message for plugins that been blocked for other reasons.

Comment 2

[Michael Wu [:mwu]]

Attachment: WIP

Putting this patch here so it doesn't get lost..

Comment 3

[Michael Wu [:mwu]]

I probably won't be able to get to this. Rob, can you or someone else take this bug?

Comment 4

[Robert Strong (they/them - no direct email)]

We'll find someone... more importantly thanks for your help this summer!

Comment 5

[Dave Townsend [:mossop]]

We still have UI questions that need answering. A couple of points:

We display one notification bar for all plugins, its button attempts to find new plugins for all that aren't working. What should the bar say when:

1. Some plugins are not installed.
2. Some plugins are blocklisted.
3. Both of the above.

We don't have any way at the moment of linking to the blocklist information so users can find out just why we are barring them from viewing the content.

In the space of a not-installed or blocklisted plugin we display "Click here to download plugin." I'm not sure that's right for both cases. I'm not sure I can easily get it to display different messages depending on the case.

We don't display anything for disabled plugins (and those which point to missing files) unless the page content includes fallback text. This is correct, I think.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #5)
> We display one notification bar for all plugins, its button attempts to find
> new plugins for all that aren't working. What should the bar say when:
> 
> 1. Some plugins are not installed.
> 2. Some plugins are blocklisted.
> 3. Both of the above.

1. No changes, what we say now ("Some plugins required by this page are missing" or whatever)

2. New notification as per this bug ("Some plugins required by this page have been disabled"); ideally we'd have some way of knowing if there was an updated version available that wasn't blocklisted and a way to point to it.

3. I think we should probably - for this likely rare case - just fold the pointers-to-updated-plugins in with the missing, and only show the missing plugins notification. To the user it would appear pretty seamless. If we can't do that, though, we can stack notification messages; I'd put the "missing" plugin at a higher priority than the "blocker" one, I think.

> We don't have any way at the moment of linking to the blocklist information so
> users can find out just why we are barring them from viewing the content.

Is this in plan? It probably should be.

> In the space of a not-installed or blocklisted plugin we display "Click here to
> download plugin." I'm not sure that's right for both cases. I'm not sure I can
> easily get it to display different messages depending on the case.

Hm. That's bad, indeed, for a blocked plugin. An alternate message would be best, second-best would be just not displaying the content at all.

> We don't display anything for disabled plugins (and those which point to
> missing files) unless the page content includes fallback text. This is 
> correct, I think.

Can we borrow that solution for blocklisted plugins as well, and just add a notification message saying that some content on the page cannot be viewed as the plugins required are blocked/insecure?

Comment 7

[Dave Townsend [:mossop]]

(In reply to comment #6)
> > We don't have any way at the moment of linking to the blocklist information so
> > users can find out just why we are barring them from viewing the content.
> 
> Is this in plan? It probably should be.

Well I think about the simplest option at this point is to have a second button on the notification bar which opens the blocklist details page

> 
> > In the space of a not-installed or blocklisted plugin we display "Click here to
> > download plugin." I'm not sure that's right for both cases. I'm not sure I can
> > easily get it to display different messages depending on the case.
> 
> Hm. That's bad, indeed, for a blocked plugin. An alternate message would be
> best, second-best would be just not displaying the content at all.

No content is I think easiest, but I'll be checking out if there's an easy way to do an alternate, I think it involves adding a CSS pseudo state though which I don't like the sound of.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

What do we currently do (if anything?) for plugins that have been manually disabled? I wonder if we should be treating them any differently.

What I'm picturing is ..:

 - we issue a blocklist update to block SomePlugin 1.2
 - when that's picked up, we use the alert service to tell users that we've done this (perhaps this is the solution to bug 391714)
 - when they encounter content that requires that plugin, they see the disabled notification (perhaps with slightly modified text, and a link to the page that explains why)

Comment 9

[Dave Townsend [:mossop]]

To confirm then the plan will be that pages with blocklisted plugins will display no particular UI in the content area (same as for disabled plugins) but there will be the notification bar warning about the attempt to use a blocklisted plugin.

Comment 10

[Dave Townsend [:mossop]]

Some final strings and UI questions:

blockedpluginsMessage.title=Plugins that are required to display the media on this page have been blocklisted for your protection.
blockedpluginsMessage.infoButton.label=Tell Me More...
blockedpluginsMessage.infoButton.accesskey=T
blockedpluginsMessage.searchButton.label=Search for Updated Plugins...
blockedpluginsMessage.searchButton.accesskey=S

How do we handle opening the blocklist page, new tab? (foreground/background), new window?

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #10)
> Some final strings and UI questions:

I like your strings, Dave, but I'm gonna change 'em all just a little :)

blockedpluginsMessage.title=Some plugins required by this page have been blocked for your protection.
blockedpluginsMessage.infoButton.label=Details...
blockedpluginsMessage.infoButton.accesskey=D
blockedpluginsMessage.searchButton.label=Update Plugins...
blockedpluginsMessage.searchButton.accesskey=U

Trying to tighten/shorten, basically, and remove the direct call out to media. 

Also, while I know that we can't guarantee that there are updates, but the action that the user wants to take is to try and update, even if they end up with no joy.

U, I guess?

> How do we handle opening the blocklist page, new tab? (foreground/background),
> new window?

New tab, with focus (foreground) unless we're doing something PFS-like, which I'm assuming we're not.

Comment 12

[Reed Loden [:reed]]

(In reply to comment #11)
> blockedpluginsMessage.infoButton.label=Details...
> blockedpluginsMessage.searchButton.label=Update Plugins...

Need to use … instead of ... here, as per bug 373623.

Comment 13

[Dave Townsend [:mossop]]

Attachment: patch rev 1

This is the patch implementing the notification for blocked plugins.

Jst, there's just a very minor change to the plugins code to make blocklisted plugins fallback rather than use the missing plugin binding.

Comment 14

[Tony Chung [:tchung]]

dave, can you attach some example blocklisted plugins for testing? 

Comment 15

[Dave Townsend [:mossop]]

Attachment: Blocklist Quicktime

Comment 16

[Dave Townsend [:mossop]]

(In reply to comment #14)
> dave, can you attach some example blocklisted plugins for testing? 
> 

Tony, if you set the pref extensions.blocklist.url to https://bugzilla.mozilla.org/attachment.cgi?id=290797, then exit and delete the blocklist.xml from your profile folder this blocklist should get read and block the Quicktime plugin.

Comment 17

[Dave Townsend [:mossop]]

Attachment: patch rev 1

This is the same patch just fixed to apply cleanly.

Comment 18

[Dave Townsend [:mossop]]

Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.919; previous revision: 1.918
done
Checking in browser/locales/en-US/chrome/browser/browser.properties;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.properties,v  <--  browser.properties
new revision: 1.55; previous revision: 1.54
done
Checking in content/base/src/nsObjectLoadingContent.cpp;
/cvsroot/mozilla/content/base/src/nsObjectLoadingContent.cpp,v  <--  nsObjectLoadingContent.cpp
new revision: 1.67; previous revision: 1.66
done

Comment 19

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2007122704 Minefield/3.0b3pre

Comment 20

[Reed Loden [:reed]]

Comment on attachment 293844 [details] [diff] [review]
patch rev 1

>+  const iconURL = "chrome://mozapps/skin/plugins/pluginGeneric.png";

How does somebody override this if he/she wants to supply a different image? With CSS, it's easy, but this is hard coded.

Comment 21

[Dave Townsend [:mossop]]

(In reply to comment #20)
> How does somebody override this if he/she wants to supply a different image?
> With CSS, it's easy, but this is hard coded.

You might still be able to do it with CSS. I forget whether list-style-image or [src] takes precedent on the image.

In what circumstance are you thinking this is a problem?

Comment 22

[[e-mail]]

(In reply to comment #20)
>(From update of attachment 293844 [details] [diff] [review])
>>+  const iconURL = "chrome://mozapps/skin/plugins/pluginGeneric.png";
>How does somebody override this if he/she wants to supply a different image?
A theme can provide a PNG image in that exact location; otherwise, no way.

Comment 23

[Tony Chung [:tchung]]

Litmus test case: https://litmus.mozilla.org/show_test.cgi?id=5201