Remove Plugin Finder Service (PFS) from Firefox

RESOLVED INCOMPLETE

Status

()

defect
RESOLVED INCOMPLETE
6 years ago
5 years ago

People

(Reporter: cers, Assigned: cers)

Tracking

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: p=13)

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

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

Comment 1

6 years ago
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

6 years ago
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.
What does PFS stand for? :-)

Comment 4

6 years ago
Plugin finder service, the thing that pops up and offers to install plugins.
(Assignee)

Comment 5

6 years ago
(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.
(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 ?
(Assignee)

Comment 7

6 years ago
This patch builds on top of bug 839206 and also makes the in-content UI mirror the door hanger behavior from there.
Attachment #723991 - Flags: review?(dolske)
(Assignee)

Comment 8

6 years ago
rebased on top of new patch in bug 839206
Attachment #723991 - Attachment is obsolete: true
Attachment #723991 - Flags: review?(dolske)
Attachment #729356 - Flags: review?(dolske)
(Assignee)

Comment 9

6 years ago
Attachment #731287 - Flags: review?(dolske)
(Assignee)

Updated

6 years ago
Attachment #731287 - Attachment is obsolete: true
Attachment #731287 - Flags: review?(dolske)
Duplicate of this bug: 264254
(Assignee)

Comment 11

6 years ago
We now only open the wizard if there is an installer it can handle. Manual installation links just open a tab
Attachment #729356 - Attachment is obsolete: true
Attachment #729356 - Flags: review?(dolske)
Attachment #749448 - Flags: review?(dolske)
http://i.qkme.me/3uhtc5.jpg
Status: NEW → ASSIGNED
Hardware: x86 → All
Version: unspecified → Trunk
Comment on attachment 749448 [details] [diff] [review]
Remove PFS and use install info from bug 839206 v3

Benjamin, are you able to review this?
Attachment #749448 - Flags: review?(benjamin)
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?
Attachment #749448 - Flags: review?(benjamin) → feedback-
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?
Attachment #749448 - Flags: review?(dolske) → review-
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.
Summary: Remove PFS from Firefox → Remove Plugin Finder Service (PFS) from Firefox
I assume we don't have all of those asian plugins listed in PFS?
I know of at least Alipay in PFS, see bug 713323. I'm not sure about others.
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.
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.
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.
Whiteboard: p=0
No longer blocks: fxdesktopbacklog
Flags: firefox-backlog+
Whiteboard: p=0 → p=13
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".
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → INCOMPLETE
Flags: firefox-backlog+ → firefox-backlog-
You need to log in before you can comment on or make changes to this bug.