1558897 - Consider removing "always allow" support and only supporting "ask to activate" for all NPAPI plugins (including the NPAPI test plugins)

Status: RESOLVED WONTFIX

(Core Graveyard :: Plug-ins, enhancement)

Description

[:Gijs (he/him)]

(Note: explicitly not changing anything wrt the GMP (gecko media plugin) ones!)

In https://bugzilla.mozilla.org/show_bug.cgi?id=1519434#c5 Chris said:

(In reply to Chris Peterson [:cpeterson] from comment #5)

(In reply to :Gijs (he/him) from comment #4)

0. I'm assuming we want to actually make this completely impossible, not just remove the UI and keep support for the internal "always run this plugin" state?

Good question! I hadn't thought of that.

We had planned to just remove the UI to prevent setting new always-allow site permissions and then let users' existing always-allow site permissions quietly expire after 90 days. But we should probably remove Gecko's always-run Flash code (somewhere in dom/plugins/base) now so Firefox Nightly users can actually experience the always-ask Flash UX now. If there are Flash issues, we want to find them during Nightly testing, not 90 days from now. :) We'll also need to remove any Flash code in the Site Permissions UI.

However, we do not want to remove always-run support for the dummy NPAPI test plugin (used for our automated NPAPI tests), Widevine, or OpenH264! Widevine and OpenH264 are GMPs (Gecko Media Plugins), not NPAPI plugins, so they might use a different code path. Just something to watch for.

It's not clear to me why we want to keep testing a dummy plugin for something that we don't support for anything "real". It would seem simpler to bring these plugins in line with flash. Chris, can you elaborate on why we wouldn't want to do that?

(In practice, this would mean we could remove a bunch more code and maybe also the flash_only pref.)

Comment 1

[Chris Peterson [:cpeterson]]

It's not clear to me why we want to keep testing a dummy plugin for something that we don't support for anything "real". It would seem simpler to bring these plugins in line with flash. Chris, can you elaborate on why we wouldn't want to do that?

We might be able to make the NPAPI dummy plugins pretend to handle the "application/x-shockwave-flash" MIME type so Firefox will recognize them without the plugin.load_flash_only pref. We would need to be careful that we don't accidentally load old, non-Flash NPAPI plugins that happen to sit in the user's plugin directory.

Jim is the module owner for NPAPI plugins, so he can decide how we might modernize these plugin tests.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a enhancement, but please change it back in case of error.

Comment 3

[Jim Mathies [:jimm]]

Can we just leave this code alone until we kill off plugins this spring? We've removed the UI, that seems sufficient.

Comment 4

[:Gijs (he/him)]

(In reply to Jim Mathies [:jimm] from comment #3)

Can we just leave this code alone until we kill off plugins this spring? We've removed the UI, that seems sufficient.

There's a bunch of directory scanning and reading/writing of pluginreg.dat, on the main thread in the parent process, as a prerequisite to starting content processes, that happens right now. For perf reasons, we want to get rid of that ASAP. The easiest way to do that seems to just stop supporting anything except flash so we can cache state in a pref and do the directory reading / pluginreg checks async, and drop support for all the other plugins. This seems much trickier to do when technically supporting multiple plugins...

That said, it seems like updating all the tests to deal with click-to-play is painful, so I'm currently intending on keeping that code for now... Do you see an easier way to avoid all the IO here? (see also the discussion in bug 1545123).

Comment 5

[Jim Mathies [:jimm]]

Forwarding this over to David. If the test work is a major undertaking I think it makes sense to just wait. We could also consider killing off tests as I don't think we really need test coverage for flash.

Comment 6

[David Parks [:handyman]]

(In reply to Jim Mathies [:jimm] from comment #3)

Can we just leave this code alone until we kill off plugins this spring? We've removed the UI, that seems sufficient.

I'm pretty sure that :gijs caught most of the value for this in bug 1545123. So I'd call this wontfix as per :jimms comment. Agreed?

Comment 7

[:Gijs (he/him)]

(In reply to David Parks (dparks) [:handyman] from comment #6)

(In reply to Jim Mathies [:jimm] from comment #3)

Can we just leave this code alone until we kill off plugins this spring? We've removed the UI, that seems sufficient.

I'm pretty sure that :gijs caught most of the value for this in bug 1545123. So I'd call this wontfix as per :jimms comment. Agreed?

Yep.