836420 - Remove Plugin Finder Service (PFS) from Firefox

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Plug-ins, defect)

Description

[Christian Sonne [:cers]]

We should remove PFS from Firefox, and replace it with a (small) list of a yet TBD number of supported plugins.

Comment 1

[Christian Sonne [:cers]]

The plan as it stands is to have a page for each supported plugin somewhere like https://mozilla.com/plugins/<plugin name> which will hold the relevant information about the particular plugin, and how to get it for the users system.

There should also be page that users can get to in case the plugin they encountered is not supported, perhaps explaining why it isn't and why plugins are bad for the web, etc etc..

I gather this will involve webdev and perhaps tech-evangelism?

Comment 2

[Benjamin Smedberg]

I do *not* think we should have any content for most plugins. We only need to help users install Flash (and should provide the most automated way to do that as possible). For every other plugin, there should be no in-product UI to guide them to any kind of installation. The SUMO site can provide guides to the other plugins that users might want to install.

My plan is to show fallback content for pages that have it. Where there is no fallback content, we should turn the object into a link which offers to download the content.

Comment 3

[Steven Michaud [:smichaud] (Retired)]

What does PFS stand for? :-)

Comment 4

[Benjamin Smedberg]

Plugin finder service, the thing that pops up and offers to install plugins.

Comment 5

[Christian Sonne [:cers]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> I do *not* think we should have any content for most plugins. We only need
> to help users install Flash (and should provide the most automated way to do
> that as possible). For every other plugin, there should be no in-product UI
> to guide them to any kind of installation. The SUMO site can provide guides
> to the other plugins that users might want to install.

I agree. By "supported" I meant the small list (which is likely to be flash and perhaps a handful others like java and silverlight).

Silverlight is used by popular services like netflix, and java is required to interact with pretty much every bank and government agency in Denmark at least, though it would otherwise be tempting not to include it.

Comment 6

[Carsten Book [:Tomcat]]

(In reply to Christian Sonne [:cers] from comment #1)
> The plan as it stands is to have a page for each supported plugin somewhere
> like https://mozilla.com/plugins/<plugin name> which will hold the relevant
> information about the particular plugin, and how to get it for the users
> system.
> 

Does this include version numbers like "update your flash to version x because of security problems etc.." and if so who is supposed to keep that information updated ?

Comment 7

[Christian Sonne [:cers]]

Attachment: Remove PFS and use install info from bug 839206

This patch builds on top of bug 839206 and also makes the in-content UI mirror the door hanger behavior from there.

Comment 8

[Christian Sonne [:cers]]

Attachment: Remove PFS and use install info from bug 839206 v2

rebased on top of new patch in bug 839206

Comment 9

[Christian Sonne [:cers]]

Attachment: Add additional supported plugins as per bug 848160

Comment 11

[Christian Sonne [:cers]]

Attachment: Remove PFS and use install info from bug 839206 v3

We now only open the wizard if there is an installer it can handle. Manual installation links just open a tab

Comment 12

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

http://i.qkme.me/3uhtc5.jpg

Comment 13

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

Comment on attachment 749448 [details] [diff] [review]
Remove PFS and use install info from bug 839206 v3

Benjamin, are you able to review this?

Comment 14

[Benjamin Smedberg]

Comment on attachment 749448 [details] [diff] [review]
Remove PFS and use install info from bug 839206 v3

I do not feel comfortable being the sole reviewer of this patch, since I am not primarily a frontend/UI coder. That said, it's obvious that we can't land this with http://people.mozilla.org/~csonne/flash.json *and* it's not clear that we should be hardcoding this list of plugins in the browser at all. I think we should have a manifest of plugins, cached and updated daily or so like the blocklist, from which we pull our install choices.

If dolske is unable to review this promptly, can he suggest an alternate reviewer or can we pick from the list of Firefox reviewers? Perhaps rstrong?

Comment 15

[Justin Dolske [:Dolske]]

Comment on attachment 749448 [details] [diff] [review]
Remove PFS and use install info from bug 839206 v3

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

::: browser/base/content/browser-plugins.js
@@ +167,5 @@
>        "flash": {
>          "displayName": "Flash",
> +        /* Link directly to Windows installer */
> +        "installWINNT": {
> +          "installInfoURL": "http://people.mozilla.org/~csonne/flash.json",

(No worries here, this is a temporary URL -- wouldn't land until bug 845638 is done.)

@@ +175,5 @@
> +          "manualInstallationURL": "https://get.adobe.com/flashplayer",
> +        },
> +        "installLinux": {
> +          "manualInstallationURL": "https://get.adobe.com/flashplayer",
> +        },

Per previous discussion: we should strongly consider having these all be mozilla.org URLs with a redirect, so we have some degree of control without needing to update Firefox. (OTOH it's a pretty low-probability need, so maybe we don't. Or do it as a separate followup.)

@@ +180,4 @@
>        },
>        "java": {
>          "displayName": "Java",
> +        "about": "https://support.mozilla.org/kb/use-java-plugin-to-view-interactive-content",

I suspect it would make things simpler to have just two possible actions for a plugin -- either we download&install it (Flash only), or we just load a tab with some URL. 

In the latter case, it seems like there's really no difference between "manualInstallationURL" and "about", other that which UI string gets used. That could just be a separate flag, or we could just make the UI generic enough to not need to show the difference.


eg, something like:

 flash : {
    installWINNT : {
      url: "http://somewhere/hashdata.json",
      type : "installer";
    },
    installLinux : {
      url: "http://blah/getflash",
      type: "userdownload"
    }
 },
 java : {
    installWINNT : {
      url: "http://sumo/java",
      type : "infopage"
    }
 }

@@ +223,5 @@
>      }
>      return false;
>    },
>  
> +  _getInstallInfo: function PH__getInstallInfo(aPlugin) {

It would be good to have some comments here (or up in supportedPlugins) about what the expected structure is / what the options are.

@@ +287,5 @@
>            let installStatus = doc.getAnonymousElementByAttribute(plugin, "class", "installStatus");
> +          let installLink = doc.getAnonymousElementByAttribute(plugin, "class", "installPluginLink");
> +          let pluginInfo = this._getInstallInfo(plugin);
> +          if (this.canInstallThisMimeType(pluginInfo.mimetype)) {
> +            installLink.innerHTML = gNavigatorBundle.getFormattedString("installPlugin.button.label",

Please use .textContent instead of .innerHTML.

.innerHTML carries the risk of security problems if unsanitized content ever gets into it, and it's not necessary here.

@@ +501,5 @@
>    // Callback for user clicking on a missing (unsupported) plugin.
>    installSinglePlugin: function (plugin) {
>      var missingPlugins = new Map();
>  
> +    var pluginInfo = this._getInstallInfo(plugin);

Per previous discussion -- It's really confusing to have both getPluginInfo() and getInstallInfo(), and then gave getInstallInfo() return something into "pluginInfo".

Just update the variable name in the rest of this short function.

@@ +506,5 @@
>      missingPlugins.set(pluginInfo.mimetype, pluginInfo);
>  
>      openDialog("chrome://mozapps/content/plugins/pluginInstallerWizard.xul",
>                 "PFSWindow", "chrome,centerscreen,resizable=yes",
>                 {plugins: missingPlugins, browser: gBrowser.selectedBrowser});

Is there any reason for this method -- installSinglePlugin() -- to continue to exist?

It's only used as a click callback from the in-content UI, which should probably just trigger display of the doorhanger (ie, show the panel, since it should already be there in icon-only mode).

@@ +584,2 @@
>        }
> +      else {

Same line, please. :)

::: toolkit/mozapps/plugins/content/pluginProblem.xml
@@ +43,5 @@
>                      <html:div class="msgReload">&reloadPlugin.pre;<html:a class="reloadLink" href="">&reloadPlugin.middle;</html:a>&reloadPlugin.post;</html:div>
>                  </html:div>
>  
>                  <html:div class="installStatus">
> +                    <html:div class="msg msgInstallPlugin"><html:a class="installPluginLink" href=""></html:a></html:div>

Presumably the &installPlugin; entity can be removed from plugins.dtd?

Comment 16

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

This might affect our friends in East Asia (China, Korea, etc) due to the vast number of (non-western) plugins prevalent over there.

When testing this patch, please ensure it doesn't break Asian usage of Firefox terribly.

Comment 17

[:Gavin Sharp [email: [e-mail]]]

I assume we don't have all of those asian plugins listed in PFS?

Comment 18

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I know of at least Alipay in PFS, see bug 713323. I'm not sure about others.

Comment 19

[Justin Dolske [:Dolske]]

Alipay (bug 713323) is not in PFS, that bug was for adding it to the Plugin Check page (ie, checking if it's up-to-date if a visiting user already has it installed). We're not adding new plugins to PFS, so there's nothing to worry about breaking.

Comment 20

[Daniel Veditz [:dveditz]]

Mozilla Online was talking about the need to add new plugins, and perhaps pointing the China edition at their own PFS server due to the unique needs of that market.

Comment 21

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I think the special China edition already points to their own PFS server, but not the zh_CN edition. I'm not sure which is more commonly used in the country, though.

Comment 22

[Benjamin Smedberg]

I think we're going to give up on any plugin-finding, and go back to just tracking this with bug 836415 "kill PFS with fire".