Run a Telemetry Experiment on Nightly to test Flash set to Click-To-Activate by default

RESOLVED FIXED

Status

()

Core
Plug-ins
P3
normal
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: Felipe, Assigned: Felipe)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {flashplayer, site-compat})

Firefox Tracking Flags

(Not tracked)

Details

User Story

- Nightly users
- minBuildID must include bug 1350381
- Only users who have Flash installed
- Only users with Flash as "Always Activate" (default)

- Allow lists: empty
- First-party block list: empty
- 3rd-party block list: the disconnect.me list

- Favor html5 fallback over plugin content: on the presence of a <video>

- Population size: 25% of nightly, split 50/50 on test/control
- Non-blocked versions of Flash
- From Apr 14 to Apr 23

- Data collection:
no extra data collection is necessary outside of what's already collected by Telemetry

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

3 months ago
We now have all the pieces in place to run an experiment on Nightly to set Flash as Click-to-Activate by default. That includes a dynamic allow/blocklist of sites that are allowed to run Flash (in first-party or third-party frames) and a system to prefer html5 fallback over Flash.

We will want to fine tune all these things, and will get wider data about how this is behaving out of a shield study on release (bug 1335232), but to get basic info about how this works, we should run this on Nightly first.

I'll keep the User Story updated with the experiment parameters.
(Assignee)

Comment 1

3 months ago
Created attachment 8855446 [details] [diff] [review]
flash-as-ctp

Benjamin, would you like to review it, or do you suggest someone else?
Attachment #8855446 - Flags: review?(benjamin)
(Assignee)

Comment 2

3 months ago
Comment on attachment 8855446 [details] [diff] [review]
flash-as-ctp

I need to change something as the plugins.flashBlock.enabled pref needs to be changed on "test-waiting" time in order for the lists to download.
Attachment #8855446 - Flags: review?(benjamin)
(Assignee)

Comment 3

2 months ago
Created attachment 8857090 [details] [diff] [review]
Experiment to make Flash Click-to-Play
Attachment #8855446 - Attachment is obsolete: true
Attachment #8857090 - Flags: review?(rhelmer)
Attachment #8857090 - Flags: review?(dothayer)
Comment on attachment 8857090 [details] [diff] [review]
Experiment to make Flash Click-to-Play

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

::: experiments/flash-as-ctp-nightly55/code/bootstrap.js
@@ +203,5 @@
> +  let classifier = Cc['@mozilla.org/url-classifier/dbservice;1']
> +                     .getService(Ci.nsIURIClassifier);
> +
> +  let { NetUtil } = Cu.import("resource://gre/modules/NetUtil.jsm", {});
> +  let table = Services.prefs.getCharPref('urlclassifier.flashSubDocTable');

For completeness's sake, should we be checking if this pref has a user value in checkHasAnyUserPref?

Also nit: for consistency this should maybe be declared as a constant above with all the other pref names.

@@ +290,5 @@
> +function install(a,b) {
> +}
> +
> +function uninstall(a,b) {
> +  stopExperiment();

I don't think it matters, since I think the only side effect would be rechecking the shavar lists, but this is run on upgrade as well, right?

Is there any other case where this can be hit without the stopExperiment call in shutdown being run?
Attachment #8857090 - Flags: review?(dothayer) → review+
(Assignee)

Updated

2 months ago
Attachment #8857090 - Attachment is obsolete: true
Attachment #8857090 - Flags: review?(rhelmer)
(Assignee)

Comment 5

2 months ago
Created attachment 8857590 [details] [diff] [review]
Experiment to make Flash Click-to-Play, r=dothayer

I've made some minor changes but I don't think it needs re-review. Just improved the tagging for people who change prefs in the middle of the experiment. Instead of being lumped in a single "disqualified" cohort, they will be tagged as "disqualified-prefchanged".. Just to make it easier to understand how many people undo the changes.
Attachment #8857590 - Flags: review+
(Assignee)

Comment 6

2 months ago
(In reply to Doug Thayer [:dthayer] from comment #4)
> > +  let classifier = Cc['@mozilla.org/url-classifier/dbservice;1']
> > +                     .getService(Ci.nsIURIClassifier);
> > +
> > +  let { NetUtil } = Cu.import("resource://gre/modules/NetUtil.jsm", {});
> > +  let table = Services.prefs.getCharPref('urlclassifier.flashSubDocTable');
> 
> For completeness's sake, should we be checking if this pref has a user value
> in checkHasAnyUserPref?
> 
> Also nit: for consistency this should maybe be declared as a constant above
> with all the other pref names.

Done

> 
> @@ +290,5 @@
> > +function install(a,b) {
> > +}
> > +
> > +function uninstall(a,b) {
> > +  stopExperiment();
> 
> I don't think it matters, since I think the only side effect would be
> rechecking the shavar lists, but this is run on upgrade as well, right?
> 
> Is there any other case where this can be hit without the stopExperiment
> call in shutdown being run?

Yeah, when an experiment expires while the user is not running Firefox, the next time that they open Firefox, only the uninstall() Function will be called  (i.e., startup() and shutdown() won't). So that's why it's important to have it there.
(Assignee)

Comment 7

2 months ago
Created attachment 8857592 [details]
experiment.xpi, unsigned

Wei, can you sign this addon for me?
Attachment #8857592 - Flags: feedback?(wezhou)
(Assignee)

Comment 8

2 months ago
Hello Stefan, could you QA this for us?


Using a fresh profile and a recent Nightly build:

#### Set up ####

** Go to about:config

- Ensure that toolkit.telemetry.enabled = true
- Change devtools.chrome.enabled to true

** Open the Web Developer -> Browser Console tool

- Copy & paste & <Enter> the following: (it can be all at once, doesn't need to be line by line)

Services.prefs.setBoolPref("xpinstall.signatures.required", false);
Services.prefs.setBoolPref("experiments.logging.enabled", true);
Services.prefs.setBoolPref("extensions.logging.enabled", true);
Services.prefs.setIntPref("experiments.logging.level", 0);
Services.prefs.setCharPref("experiments.force-sample-value", "0.1");
Services.prefs.setCharPref("experiments.manifest.uri", "https://people.mozilla.org/~fgomes/flash-as-ctp-55/firefox-manifest.json");

- Afterwards, copy & paste & <Enter> the following: (**)

Cu.import("resource:///modules/experiments/Experiments.jsm");
Experiments.instance().updateManifest();


#### Test cases ####

* After all of that runs, go to about:support, scroll to the very bottom and check if "Flash as Click-To-Activate" is listed in "Experimental Features", and Active = true.  See what the Branch says:

* If "control":
 -- nothing should have changed. Navigate to a site known to have Flash, and see if it's loaded without asking the user anything. Check that Flash is still "Always Activate" in about:addons

* If "test-waiting":
 -- Restart Firefox, wait some seconds (might take up to 1min after restart), and open/refresh about:support to see if branch changes to "test"

 -- When it has changed to "test":
   -- Check that Flash is behaving in Ask to Activate mode, by going to some random Flash website and see if it asks for activation
   -- Check https://itisatrap.org/firefox/flashblock.html : The "Actual" column should match the "Expected for Ask-to-Activate" column.
   -- Go to about:config and change the pref "plugins.flashBlock.enabled" to false, and check that the branch has changed to "disqualified-prefchanged"

* On a clean profile again, before running the (**) command above, change the pref "plugins.flashBlock.enabled" to true. This will guarantee that there's a non-standard pref before the experiment starts. In this case, the experiment should directly go to a branch named "disqualified".


* If you're able to uninstall Flash, please do so for this test. Make sure Firefox is restarted after Flash is uninstalled, then run the experiment and check that it goes to a branch named "no-flash".
Flags: needinfo?(stefan.georgiev)

Updated

2 months ago
Attachment #8857592 - Flags: feedback?(wezhou)

Comment 9

2 months ago
Created attachment 8857638 [details]
experiment.xpi signed

Please see attached.

Comment 10

2 months ago
We have executed the test on Windows, Ubuntu and Mac. We encountered two issues:

1. In case If test-waiting after 3 or 4 restarts the branch field never turn to "test" on all tested platform
2. In case If "no-flash" after removing flash, the branch shows "control' or "test-waiting"
Flags: needinfo?(stefan.georgiev)
(Assignee)

Comment 11

2 months ago
QA has concluded testings (Stefan will send out an e-mail shortly), so I've pushed it to the repo and will file a new bug for deployment:

https://hg.mozilla.org/webtools/telemetry-experiment-server/rev/aef9c8055a672a330260c3219f16798c5db98d14

Comment about QA: QA an issue, bug 1338638, which is unrelated to the experiment code, but which made QA'ing this a bit of pain, since that bug caused the Shavar service to take unusually long to update in certain conditions. This bug existed on Nightlies from Apr 6 - Apr 14 and is now fixed, but it nonetheless does not affect the experiment itself.. Users might just take a bit longer to get into the "test" cohort (users chosen for test are put in a "test-waiting" cohort until we are sure they have received the right flash blocklist).
(Assignee)

Updated

2 months ago
Depends on: 1356713
(Assignee)

Comment 12

2 months ago
This has been deployed on bug 1356713
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
(Assignee)

Updated

2 months ago
User Story: (updated)
You need to log in before you can comment on or make changes to this bug.