1438857 - Remove the Flash plugin infobar

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, enhancement, P1)

Description

[:Felipe Gomes (needinfo for replies!)]

The Flash infobar is currently unused and will likely never be used again, so we should remove all the code supporting it.

A suppress list was once implemented to hide this infobar for specific domains (bug 1369160), but after a while we entirely disabled the infobar (bug 1377036)

Comment 1

[Chris H-C :chutten]

We should ensure the PLUGINS_INFOBAR_* probes are removed as part of this work

Comment 2

[:prathiksha]

Attachment: Bug 1438857 - Remove the Flash plugin infobar.

Review commit: https://reviewboard.mozilla.org/r/235972/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235972/

Comment 3

[:prathiksha]

Just something to note here, I will remove the "plugins.show_infobar" pref entirely in Bug 1453579 as it overlaps with some infobar blocklist code.

Comment 4

[Code Review Bot [:reviewbot]]

Comment on attachment 8967285 [details]
Bug 1438857 - Remove the Flash plugin infobar.

https://reviewboard.mozilla.org/r/235972/#review241712


Code analysis found 3 defects in this patch:
 - 3 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: browser/modules/PluginContent.jsm:896
(Diff revision 1)
>      document = document || this.content.document;
>  
>      // We're only interested in the top-level document, since that's
>      // the one that provides the Principal that we send back to the
>      // parent.
>      let principal = document.defaultView.top.document.nodePrincipal;

Error: 'principal' is assigned a value but never used. [eslint: no-unused-vars]

::: browser/modules/PluginContent.jsm:897
(Diff revision 1)
>  
>      // We're only interested in the top-level document, since that's
>      // the one that provides the Principal that we send back to the
>      // parent.
>      let principal = document.defaultView.top.document.nodePrincipal;
>      let location = document.location.href;

Error: 'location' is assigned a value but never used. [eslint: no-unused-vars]

::: browser/modules/PluginContent.jsm:900
(Diff revision 1)
>      // parent.
>      let principal = document.defaultView.top.document.nodePrincipal;
>      let location = document.location.href;
>  
>      // Make a copy of the actions from the last popup notification.
>      let haveInsecure = false;

Error: 'haveinsecure' is assigned a value but never used. [eslint: no-unused-vars]

Comment 5

[:prathiksha]

Comment on attachment 8967285 [details]
Bug 1438857 - Remove the Flash plugin infobar.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/235972/diff/1-2/

Comment 6

[Chris H-C :chutten]

Comment on attachment 8967285 [details]
Bug 1438857 - Remove the Flash plugin infobar.

https://reviewboard.mozilla.org/r/235972/#review241880

Thank you for your note about plugins.show_infobar

My r+ is for the Telemetry removal.

Comment 7

[Chris Peterson [:cpeterson]]

Let's wait until the Nightly 62 dev cycle begins (May 7) to land Prathiksha's fix so this change gets more "bake time".

Comment 8

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8967285 [details]
Bug 1438857 - Remove the Flash plugin infobar.

https://reviewboard.mozilla.org/r/235972/#review248066

Hello Prathiksha! (Sorry for the delay.. due to the soft freeze on nightly I postponed this a bit and it fell off my radar)

This looks great.. There are a few nits pointed out below (as well as the eslint msgs to be fixed), and two larger things:

(1) the hideNotificationBar function from browser-plugins.js, and related code that triggers it from PluginContent.jsm, can also be removed.

(2) the main issue remaining is that the updateHiddenPluginUI function was (arguably incorrectly) responsible for also displaying the blocked (insecure) plugin state (through the haveInsecure parameter that it sent to the parent).

To solve (2), the best thing would be to handle the insecure plugin case together in the PluginContent:ShowClickToPlayNotification message.  Probably this information is already available there in the msg.data.plugins array, so you might not even need to add a new parameter like haveInsecure.

Alternatively, if it gets too complicated, you could undo the removal of the updateHiddenPluginUI function and just leave the part that is necessary for the haveInsecure flag, and then file a follow-up bug to do it the cleaner way

::: browser/app/profile/firefox.js:713
(Diff revision 2)
>  // Should plugins that are hidden show the infobar UI?
>  pref("plugins.show_infobar", false);
>  

nit: this can be removed

::: toolkit/components/telemetry/TelemetryEnvironment.jsm:254
(Diff revision 2)
>    ["network.proxy.http", {what: RECORD_PREF_STATE}],
>    ["network.proxy.ssl", {what: RECORD_PREF_STATE}],
>    ["pdfjs.disabled", {what: RECORD_PREF_VALUE}],
>    ["places.history.enabled", {what: RECORD_PREF_VALUE}],
> -  ["plugins.remember_infobar_dismissal", {what: RECORD_PREF_VALUE}],
>    ["plugins.show_infobar", {what: RECORD_PREF_VALUE}],

nit: this can be removed

Comment 9

[:prathiksha]

Attachment: Bug 1438857 - Remove the Flash plugin infobar. r?felipe

Remove Flash plugin infobar code.

Comment 10

[:prathiksha]

> ::: browser/app/profile/firefox.js:713
> (Diff revision 2)
> >  // Should plugins that are hidden show the infobar UI?
> >  pref("plugins.show_infobar", false);
> >  
> 
> nit: this can be removed
> 
> ::: toolkit/components/telemetry/TelemetryEnvironment.jsm:254
> (Diff revision 2)
> >    ["network.proxy.http", {what: RECORD_PREF_STATE}],
> >    ["network.proxy.ssl", {what: RECORD_PREF_STATE}],
> >    ["pdfjs.disabled", {what: RECORD_PREF_VALUE}],
> >    ["places.history.enabled", {what: RECORD_PREF_VALUE}],
> > -  ["plugins.remember_infobar_dismissal", {what: RECORD_PREF_VALUE}],
> >    ["plugins.show_infobar", {what: RECORD_PREF_VALUE}],
> 
> nit: this can be removed

I address not removing the "plugins.show_infobar" pref in my original patch in Comment 3. The pref is still used here https://searchfox.org/mozilla-central/rev/f0c15db995198a1013e1c5f5b5bea54ef83f1049/toolkit/components/url-classifier/SafeBrowsing.jsm#241 :)

Comment 11

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8998530 [details]
Bug 1438857 - Remove the Flash plugin infobar. r?felipe

:Felipe Gomes (needinfo me!) has approved the revision.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ce626742482
Remove the Flash plugin infobar. r=Felipe

Comment 13

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/7ce626742482