Closed
Bug 1365714
Opened 6 years ago
Closed 6 years ago
Release Flash plugin Click-to-Play (aka "Ask to Activate")
Categories
(Core Graveyard :: Plug-ins, enhancement, P3)
Core Graveyard
Plug-ins
Tracking
(firefox55 fixed)
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: Felipe, Assigned: Felipe)
References
()
Details
(4 keywords)
User Story
Rollout plan specification: For beta 55, the entire cycle: * deploy domain blocking to everyone (direct landing, ride the trains) * deploy click-to-activate 50/50 on/off ** cohorts must appear in environment.experiments for monitoring For release 55 * domain blocking to everyone (ride the trains) * Deploy click to activate over the release cycle in the following stages: ** 5% of users - two weeks - monitoring engagement and other metrics ** 25% of users - two weeks ** 100% of users before Firefox 56 ** cohorts must appear in environment.experiments for monitoring
Attachments
(2 files, 1 obsolete file)
Bug 1317856 just switched Flash as Click-to-Play on Nightly. This bug is meant to track: - the extra requirements of shipping that to release, through the dependencies - the actual work of switching this on release, maybe directly by changing prefs, maybe with a staged rollout.
Comment 1•6 years ago
|
||
My current thinking: * roll the blocklist out without staging * roll the CtA setting out gradually ** to 50% of beta starting immediately with 55b1 ** to release 55 gradually over 6 weeks, starting at 10% of release Monitoring: * overall page loads (engagement) * users who flip Flash back to enabled * infobar shown/accept/hide rates * notification shown/action rates I filed bug 1362520 on datasets for release monitoring, since that's too much data for simple notebooks.
Comment 2•6 years ago
|
||
Final deployment strategy is in the user story. I am agnostic as to the specific deployment vehicle: system addon, pref experiment, pref rollout are all fine from my perspective. Schedule risk because pref rollout isn't ready is not ok ;-) Felipe and Doug, this is ready for engineering decisions and action.
User Story: (updated)
Flags: needinfo?(felipc)
Flags: needinfo?(dothayer)
Assignee | ||
Comment 3•6 years ago
|
||
Other dependencies are tracked on other bugs, so I'll use this one to implement the rollout
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 7•6 years ago
|
||
mozreview-review |
Comment on attachment 8875897 [details] Bug 1365714 - Part 1 - Adjust prefs to roll Click-to-Play by default on release. https://reviewboard.mozilla.org/r/147284/#review151824
Attachment #8875897 -
Flags: review?(benjamin) → review+
Comment 8•6 years ago
|
||
mozreview-review |
Comment on attachment 8875898 [details] Bug 1365714 - Part 2 - System add-on to control rollout of the CTP feature. https://reviewboard.mozilla.org/r/147286/#review151826 ::: browser/extensions/clicktoplay-rollout/bootstrap.js:17 (Diff revision 2) > +Cu.import("resource://gre/modules/AppConstants.jsm"); > + > +// The amount of people to be part of the rollout > +const TEST_THRESHOLD = { > + "beta": 0.5, // 50% > + "release": 0.1, // 10% Should be 5% for release ::: browser/extensions/clicktoplay-rollout/bootstrap.js:116 (Diff revision 2) > + value = Math.random(); > + Preferences.set(pref, value.toString().substr(0, 8)); > + return value; > +} > + > +function setCohort(cohortName) { I believe this function is missing code to add the cohort to environment/experiments. ::: browser/extensions/clicktoplay-rollout/bootstrap.js:126 (Diff revision 2) > + } > + } catch (e) {} > +} > + > +function watchForPrefChanges() { > + Preferences.observe(PREF_FLASH_STATE, function()) { I don't think the pref observer or this special cohort is necessary: I can distinguish whether users make a change via the environment, and being able to explicitly monitor that per original cohort might be helpful.
Attachment #8875898 -
Flags: review?(benjamin) → review-
Assignee | ||
Comment 9•6 years ago
|
||
mozreview-review |
Comment on attachment 8875898 [details] Bug 1365714 - Part 2 - System add-on to control rollout of the CTP feature. https://reviewboard.mozilla.org/r/147286/#review151836 ::: browser/extensions/clicktoplay-rollout/bootstrap.js:17 (Diff revision 2) > +Cu.import("resource://gre/modules/AppConstants.jsm"); > + > +// The amount of people to be part of the rollout > +const TEST_THRESHOLD = { > + "beta": 0.5, // 50% > + "release": 0.1, // 10% ok ::: browser/extensions/clicktoplay-rollout/bootstrap.js:116 (Diff revision 2) > + value = Math.random(); > + Preferences.set(pref, value.toString().substr(0, 8)); > + return value; > +} > + > +function setCohort(cohortName) { I was gonna add it, but the pref value is already being recorded in TelemetryEnvironment.jsm: `["plugins.ctprollout.cohort", {what: RECORD_PREF_VALUE}],` And I was wondering if that is enough? If it's not enough I can add it using the new setExperimentActive function ::: browser/extensions/clicktoplay-rollout/bootstrap.js:126 (Diff revision 2) > + } > + } catch (e) {} > +} > + > +function watchForPrefChanges() { > + Preferences.observe(PREF_FLASH_STATE, function()) { The problem is that, since I'm using defaultPrefs to set PREF_FLASH_STATE, it's set on every startup. If the user changes it to something else, we'll keep changing it back again. So this different cohort is a way to tell "leave this user alone". What I could do is store in the cohort name the name of the original cohort too. What do you think?
Comment 10•6 years ago
|
||
> `["plugins.ctprollout.cohort", {what: RECORD_PREF_VALUE}],` > > And I was wondering if that is enough? No that's not enough, because I'd have to get the data team to modify main_summary to include this new field. The experiments field is included by default. (In fact you can revert that TelemetryEnvironment change if you like. > If it's not enough I can add it using the new setExperimentActive function please > What I could do is store in the cohort name the name of the original cohort > too. What do you think? That would solve the problem, yes.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8875898 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 14•6 years ago
|
||
mozreview-review |
Comment on attachment 8876203 [details] Bug 1365714 - Part 2 - System add-on to control rollout of the CTP feature. https://reviewboard.mozilla.org/r/147644/#review151926 ::: browser/extensions/clicktoplay-rollout/bootstrap.js:123 (Diff revision 1) > +function setCohort(cohortName) { > + let currentCohort = Preferences.get(PREF_COHORT_NAME, undefined); > + > + if (cohortName != currentCohort) { > + Preferences.set(PREF_COHORT_NAME, cohortName); > + // Only do this if the cohort name is changing, to avoid triggering This needs to be set on every startup: setExperimentActive is a non-persistent API.
Attachment #8876203 -
Flags: review?(benjamin) → review-
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 17•6 years ago
|
||
mozreview-review |
Comment on attachment 8876203 [details] Bug 1365714 - Part 2 - System add-on to control rollout of the CTP feature. https://reviewboard.mozilla.org/r/147644/#review151948
Attachment #8876203 -
Flags: review?(benjamin) → review+
Comment 18•6 years ago
|
||
Pushed by felipc@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1731ad31e6a5 Part 1 - Adjust prefs to roll Click-to-Play by default on release. r=bsmedberg https://hg.mozilla.org/integration/mozilla-inbound/rev/4f5ed57189ad Part 2 - System add-on to control rollout of the CTP feature. r=bsmedberg
Comment 19•6 years ago
|
||
Important risks in terms of testing: * people shouldn't switch cohorts. Once we've picked that you're going to be on or off, it should stay that way. * the cohort needs to show up in the main ping environment.experiments section. about:telemetry should show you that.
Comment 20•6 years ago
|
||
All test has passed on the provided Beta TRY build. I can provide the results upon request. When restarting the browser, the prefs are not reverted to the initial value. User action are remembered when changing the Flash options from about:addons. The "plugins.ctprollout.cohort" is changed respectively depending on the cohorts.
![]() |
||
Comment 21•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1731ad31e6a5 https://hg.mozilla.org/mozilla-central/rev/4f5ed57189ad
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 22•6 years ago
|
||
I've expanded on the plan for rollout of "click-to-play" Flash in the release notes: https://developer.mozilla.org/en-US/Firefox/Releases/55#Plugins Does that cover it for now?
Keywords: dev-doc-needed → dev-doc-complete
Updated•1 year ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•