1348089 - Implement new plugin option for Flash as a checkbox to toggle the allow/blocklist feature (plugins.flashBlock.enabled)

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, enhancement)

Description

[:Felipe Gomes (needinfo for replies!)]

The options displayed for the Flash plugin may need to be changed. These are the options that appear in Add-ons Manager > Plugins.

- The entries in the drop-down may change
- A checkbox may be needed
- A preferences page may be needed.

We don't need the final requirements yet but I'll update the user story when we do.

Comment 1

[Benjamin Smedberg]

For the record, we already have a preferences page, but it only has options on win32 where there's a checkbox to disable Flash protected mode. That option is not present on win64 or other platforms.

See https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/content/pluginPrefs.xul#16 and bug 1133000

Comment 2

[:Felipe Gomes (needinfo for replies!)]

Attachment: plugins-list.png

Main Plugins list page in the Add-ons Manager, where the Preferences button for Flash is now exposed unconditionally (whereas it was only exposed for win32 due to Flash protected mode.. see comment 1)

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Attachment: new-checkbox.png

The new Checkbox that will control the plugins.flashBlock.enabled pref, saying "Always block dangerous and deceptive Flash content". The final text and the address of the Learn More link are yet TBD.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Attachment: with-protected-mode.png

How the two checkboxes show up together in win32, where the "Enable Flash Protected Mode" checkbox also shows up.

Note that I added the new one before the existing one, because I think this new option is more important so it should come first.

Comment 5

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8852141 [details]
new-checkbox.png

Bram, can you review this (and the other screenshots here)? We can talk on Vidyo if you want. My two goals here are:

- make a final call on the string
- sign off on this UI at least as a first version. If we tweak the UI later we can do as a separate bug, unless this one is a really no-go.

Comment 6

[:Felipe Gomes (needinfo for replies!)]

Attachment: Bug 1348089 - Implement new plugin option for Flash as a checkbox to toggle the allow/blocklist feature (plugins.flashBlock.enabled).

Asking for review in advance of the final string TBD

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

Comment 7

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8852146 [details]
Bug 1348089 - Implement new plugin option for Flash as a checkbox to toggle the allow/blocklist feature (plugins.flashBlock.enabled).

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

Comment 8

[Benjamin Smedberg]

My proposed version in email was "Block unnecessary or risky Flash content." Other possibilities:

"Speed up Firefox by disabling unnecessary plugin usage."
"Make Firefox safer by disabling unnecessary Flash content."

My goals are to avoid making value judgements about particular domains or content, but to communicate a judgement about whether it represents enhanced risk (advertisers and other widely embedded domains) or whether it's annoying users with Flash prompts for no good reason.

Comment 9

[:Felipe Gomes (needinfo for replies!)]

Ok. And what if we end up using a whitelist too? Is there a version of the string that can transmit the idea that some items will be automatically allowed too?

I was going for something like:

"Let Mozilla automatically allow or block some Flash content to improve my experience", or "to keep me safe", etc..

But I couldn't find a way to further reduce this phrase.

Comment 10

[Benjamin Smedberg]

Yeah, I don't have anything better. "Automatically configure Flash for the best experience." is too vague? Need Bram's feedback/advice.

Comment 11

[David Durst [:ddurst]]

For posterity, I want to add that we've also discussed the potential need for a link to a place to "read more" -- both because of the nature of the changes, but also because as soon as there is talk of how things are determined, or what lists, etc, people are going to want to understand more about this than one line can tell them.

Not sure if that's something we need to incorporate now and plan on launching (via blog post?) in the future or not.

Comment 12

[:Felipe Gomes (needinfo for replies!)]

(In reply to David Durst [:ddurst] from comment #11)
> For posterity, I want to add that we've also discussed the potential need
> for a link to a place to "read more" -- both because of the nature of the
> changes, but also because as soon as there is talk of how things are
> determined, or what lists, etc, people are going to want to understand more
> about this than one line can tell them.
> 
> Not sure if that's something we need to incorporate now and plan on
> launching (via blog post?) in the future or not.

This is already in! See the new-checkbox.png screenshot. Although it's not very clear, the "Learn more" link is related to the checkbox, not to the plugin as a whole.

Comment 13

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8852146 [details]
Bug 1348089 - Implement new plugin option for Flash as a checkbox to toggle the allow/blocklist feature (plugins.flashBlock.enabled).

(canceling review for now as we might be adding a new label to provide a better description)

Comment 14

[David Durst [:ddurst]]

I just want to confirm that bug 1351509 is for a later phase, and we still are just seeking one string for the feature launch/landing in 55.

Comment 15

[Bram Pitoyo [:bram]]

(In reply to David Durst [:ddurst] from comment #14)
> I just want to confirm that bug 1351509 is for a later phase, and we still
> are just seeking one string for the feature launch/landing in 55.

Yes. All that’s left is to finalise the string for 55 (hence needinfo on Michelle). We can think of reorganisation after this bug has been fixed first. That bug shouldn’t impact this one from shipping.

Comment 16

[David Durst [:ddurst]]

We need to land this soon -- do we have a final string?

Comment 17

[mheubusch]

(In reply to David Durst [:ddurst] from comment #16)
> We need to land this soon -- do we have a final string?

Hi David: Please use the following string:

Block dangerous and intrusive Flash content

Comment 18

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8852146 [details]
Bug 1348089 - Implement new plugin option for Flash as a checkbox to toggle the allow/blocklist feature (plugins.flashBlock.enabled).

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

Comment 19

[:Felipe Gomes (needinfo for replies!)]

(This update just changed the string.. ReviewBoard decided to re-request review because I moved some patches around my tree, I think..)

Comment 20

[Robert Helmer [:rhelmer]]

Comment on attachment 8852146 [details]
Bug 1348089 - Implement new plugin option for Flash as a checkbox to toggle the allow/blocklist feature (plugins.flashBlock.enabled).

https://reviewboard.mozilla.org/r/124360/#review126996

Looks like we have some mochitests in

Comment 21

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8852146 [details]
Bug 1348089 - Implement new plugin option for Flash as a checkbox to toggle the allow/blocklist feature (plugins.flashBlock.enabled).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/124360/diff/3-4/

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bedd46fed76d
Implement new plugin option for Flash as a checkbox to toggle the allow/blocklist feature (plugins.flashBlock.enabled). r=rhelmer

Comment 23

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/bedd46fed76d

Comment 24

[Abe - QA (:Abe_LV)]

Verified as Fixed.
Here are test runs: https://public.etherpad-mozilla.org/p/1348089

Comment 25

[Andrei Vaida [:avaida]]

Updating tracking flags based on Comment 24.

Comment 26

[Bram Pitoyo [:bram]]

Comment on attachment 8852141 [details]
new-checkbox.png

Somehow, I’ve forgotten to formally put a + flag on the ui-review, even though the UI had been shipped and approved.

See bug 1351509 for layout revisions to this UI, that will make the activation dropdown menu even easier to access.