Closed Bug 1475099 Opened 3 years ago Closed 2 years ago

[Shield] Opt-Out Study: Block Autoplay, Release 63

Categories

(Shield :: Shield Study, enhancement, P2)

enhancement

Tracking

(firefox63+ fixed)

RESOLVED FIXED
Tracking Status
firefox63 + fixed

People

(Reporter: alwu, Assigned: alwu)

References

()

Details

User Story

PHD : https://docs.google.com/document/d/19enwHXxFmK1Bd71AHLeoEt4qDDpBFjsABJ2qEqHEj9g/edit

Code repo : https://github.com/alastor0325/autoplay-shield-study

Attachments

(4 files, 6 obsolete files)

In order to understand how do people think about the "doorhanger" project which provides a prompt to ask autoplay permission for site, we need to write a shield-study extension to collect related data and response.
Rank: 15
Priority: -- → P2
Hi, Dale,

Since you are the author of bug1461656, may I ask you some questions about the permission prompt? 

Now I'm planing to implement an extension experiements API which would use PermissionUI.jsm and SitePermission.jsm. I want to store the options which user clicked on the prompt.

Therefore, I'm looking for a way to know when the prompt was disappear. If I can know that, then I can query the status of "autoplay-media" permission of the current site to SitePermission.jsm.

So my question is, is there any way could let me know when the prompt was disappear in extension background script? Is there any event I can use?

Thank you.
Flags: needinfo?(dharvey)
It's a preliminary extension workflow for how we collect data when user navigating to new URL, please free feel to leave any comment on there.
https://www.lucidchart.com/invitations/accept/82a70065-d50b-49ca-b93f-409c7ac7885e
Will take a look into this, but going to needinfo Johann in case there is already a way to hook into this properly, Johann is there a way to pick up the result of a doorhanger action currently?
Flags: needinfo?(dharvey) → needinfo?(jhofmann)
There's no way for regular extensions to interact with system doorhangers such as permission prompts. You will have to write a WebExtension Experiment, which is not as hard as it sounds. There's an example repository for this: https://github.com/mozilla/shield-studies-addon-template

Instead of PermissionUI.jsm I think I'd try to hook into PopupNotifications.jsm[0] and wait for a "popupshown" event to happen, then get the notification element and try to listen for events on its buttons (or you go straight for "popuphidden" if you're really only interested in the cases of permission prompts getting dismissed).

I recommend playing around with some of this code in the browser toolbox first:

PopupNotifications.panel.addEventListener("popupshown", console.log)
PopupNotifications.panel.addEventListener("popuphidden", console.log)

And explore how much you can do with the API we have. If you need additional functions to support your experiment in central I'm also happy to take patches (within reason) that support this.

You're certainly in for a bit of browser UI hackery! :)

[0] https://searchfox.org/mozilla-central/source/toolkit/modules/PopupNotifications.jsm
Flags: needinfo?(jhofmann)
And FYI, Nihanth did a pretty unrelated study recently that also used PopupNotifications.jsm, but in a different manner, I thought it still might help you get started in some ways: https://github.com/mozilla/blurts-addon/blob/master/src/privileged/blurts/FirefoxMonitor.jsm
Depends on: 1476701
Depends on: 1476456
Depends on: 1478875
Depends on: 1479589
Hi, Johann,

Could you help me review my extension code [1]? 
In addition, here is my custom telemetry ping schema [2].

Thank you!

[1] https://github.com/alastor0325/autoplay-shield-study
[2] https://goo.gl/jaQSR8
Flags: needinfo?(jhofmann)
(In reply to Alastor Wu [:alwu] from comment #6)
> Hi, Johann,
> 
> Could you help me review my extension code [1]? 
> In addition, here is my custom telemetry ping schema [2].
> 
> Thank you!
> 
> [1] https://github.com/alastor0325/autoplay-shield-study
> [2] https://goo.gl/jaQSR8

Hi Alastor,

I would be happy to review your shield study, I just want to make sure that all other pieces are in place for launching this. You need a bunch of things apart from just the study code, e.g. a PHD, QA support etc. The process is summarized here: https://wiki.mozilla.org/Firefox/Shield#Launch_a_Shield_Study

If your team has an EPM (or a product manager) it's probably their job to set most of this up.

Once you have everything in place, please provide a PR in your repo that merges master into an empty commit (like https://github.com/mozilla/blurts-addon/pull/66), so that I can leave traceable review comments.

Thanks!
Flags: needinfo?(jhofmann) → needinfo?(alwu)
Hi, Johann,

Here is our PHD [1] and we don't request PI for QA yet (because document said that we should pass the code review first).

Here is my PR [2], Thank you!

[1] https://docs.google.com/document/d/19enwHXxFmK1Bd71AHLeoEt4qDDpBFjsABJ2qEqHEj9g/edit
[2] https://github.com/alastor0325/autoplay-shield-study/pull/3
Flags: needinfo?(alwu) → needinfo?(jhofmann)
Thanks! I'll review the PR this week.

FWIW, the previous studies I've worked with did many of the things in parallel, e.g. QA and peer review, but I can review this nonetheless. Please make sure that the Shield team is aware of this bug, has approved your PHD and has scheduled your study (I've cc'ed glind).
Flags: needinfo?(jhofmann)
Attachment #8997885 - Flags: review?(jhofmann)
Status: NEW → ASSIGNED
Summary: implement a shield-study extension for block autoplay → [Shield] Opt-Out Study: Block Autoplay
Comment on attachment 8997885 [details] [diff] [review]
GitHub Pull Request

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

See comments on GitHub :)

Thanks!
Attachment #8997885 - Attachment is patch: true
Attachment #8997885 - Attachment mime type: text/x-github-pull-request → text/plain
Attachment #8997885 - Flags: review?(jhofmann) → review-
Comment on attachment 8997885 [details] [diff] [review]
GitHub Pull Request

>https://github.com/alastor0325/autoplay-shield-study/pull/3
Attachment #8997885 - Flags: review- → review?(jhofmann)
Hi, Johann,

I've modified my patches, could you help me review it again?

Note: the issue that you mentioned we should not change the layout of doorhanger is not fixed in this commit, we still need more time to discuss it.

Thank you!
Attachment #8997885 - Flags: review?(jhofmann) → review+
Comment on attachment 8997885 [details] [diff] [review]
GitHub Pull Request

Hi, Johannh,

I've modified my patches, could you help me do a quick review again?

Thank you!
Attachment #8997885 - Flags: review+ → review?(jhofmann)
Comment on attachment 8997885 [details] [diff] [review]
GitHub Pull Request

This looks fine from a brief look. Note the comment on resetting the pref...
Attachment #8997885 - Flags: review?(jhofmann) → review+
Depends on: 1485160
According [1], "The Shield Study owner is responsible for signing off on the science review and the risk matrix."

[1] https://docs.google.com/document/d/1hOMjZ7l1K0HL8DUp7HCr8BeRO7NGw0SwYjK2nfwtRJQ/edit#
Flags: needinfo?(glind)
User Story: (updated)
Comment on attachment 8997885 [details] [diff] [review]
GitHub Pull Request

Hi, rhelmer,

My extension has been reviewed by firefox peer (johannh) before, but I've commit some changes after finished review.

I would like to ask for another review again, especially for the extension's lifetime management, could you help me review my extension?

Thank you!
Attachment #8997885 - Flags: review?(rhelmer)
According Step5 (VI) [1] , "Needinfo a data steward from the cc list (rweiss@ or sguha@) to validate permissibility of collected data fields".

[1] https://docs.google.com/document/d/1hOMjZ7l1K0HL8DUp7HCr8BeRO7NGw0SwYjK2nfwtRJQ/edit
Ni for comment 18.
Flags: needinfo?(sguha)
Comment on attachment 8997885 [details] [diff] [review]
GitHub Pull Request

The lifetime management part looks OK to me (I just reviewed the code have not run it locally etc), I had a few questions and suggestions in the github issue but I don't see anything blocking.

The only general thing I'd like to say is to think about where the doorhanger appears in the UI and if it makes sense as a location for that sort of notification.

Also ensure that the doorhanger overlaps the privileged UI as this is a hint to the user that this is not being provided by a website or regular extension.
Attachment #8997885 - Flags: review?(rhelmer) → review+
Hi, Michael,

I would like to ask you for signing my extension which has already passed the review but is still in the QA testing process.

As our QA told us that there are some scenarios (Browser Update, Firefox Safe Mode, Experiment expiration date) can only be tested with the signed add-ons, so I need to signed extension first in order to continue the following tests.

Thank you!
Flags: needinfo?(mcooper)
Flags: needinfo?(mcooper)
The data collected looks good.

+r
Flags: needinfo?(sguha)
FYI, this bug will be closed after QA testing finishes, it's still in process.
Hi, Michael,

Could you help me sign this version of extension which I has modified for fixing some bugs? QA needs new signed version of extension so that they can test it on the Release Candidate version. 

Thank you!
Attachment #9011950 - Attachment is obsolete: true
Attachment #9011953 - Attachment is obsolete: true
Flags: needinfo?(mcooper)
:alwu, I notice that this add-on has the same add-on ID and version number as the previous one. To avoid confusion, my policy is to require a version number bump when signing the same add-on again. Can you please change the version number, such as to 2.0.1? Then I can sign it.
Flags: needinfo?(mcooper)
Thank you!
Attachment #9015304 - Attachment is obsolete: true
Flags: needinfo?(mcooper)
Flags: needinfo?(mcooper)
From step5-(viii) [1], "Needinfo the QA resource that has tested your study. QA must explicitly sign off in the bug."

[1] https://docs.google.com/document/d/1hOMjZ7l1K0HL8DUp7HCr8BeRO7NGw0SwYjK2nfwtRJQ/edit#
Flags: needinfo?(andreea.cupsa)
Hi Ilana,
I've updated the PHD according to our review meeting last week. Kindly review and let me know if there are more questions.

https://docs.google.com/document/d/19enwHXxFmK1Bd71AHLeoEt4qDDpBFjsABJ2qEqHEj9g/edit#

Cindy
Flags: needinfo?(isegall)
[Tracking Requested - why for this release]:
Summary: [Shield] Opt-Out Study: Block Autoplay → [Shield] Opt-Out Study: Block Autoplay, Release 63
Proposed study release date is 2018-10-23, running for 2 weeks, per the PHD.
This is a preliminary sign-off for the “Block Autoplay” shield study on Firefox Beta 63.0b13. The official sign-off will be sent after the RC is released to Beta channel (10/16/2018).

Block Autoplay
Targeted: Firefox Release 63

We have finished testing the “Block Autoplay” experiment. 

QA’s recommendation: YELLOW - SHIP IT CONDITIONALLY

Reasoning: 
- From the shield study perspective, the experiment is looking ok. However, we have found and logged several feature issues that were closed as intended behavior, but the QA team does not agree on the resolution of some of them.
https://github.com/alastor0325/autoplay-shield-study/issues/14 - The doorhanger is not dismissed when navigating to a different non-audio page of the same domain
https://github.com/alastor0325/autoplay-shield-study/issues/15 - The doorhanger is wrongly dismissed when you click inside the page on several websites
https://github.com/alastor0325/autoplay-shield-study/issues/16 - The doorhanger is wrongly dismissed on YouTube when you click specific player buttons

- The feature had 2 previously sign offs as YELLOW in Nightly and Beta channels. 

Testing Summary:
- Full Functional test suite: TestRail (https://testrail.stage.mozaws.net/index.php?/plans/view/12724)

Tested Platforms:
- Windows 10 x64
- Mac 10.13.3
- Debian 9.3

Tested Firefox versions:
- Firefox Beta 63.0b9
- Firefox Beta 63.0b10
- Firefox Beta 63.0b13

Regards,
Andreea
Pascal, this is proposed to roll out to 0.5% of release 63. Are you comfortable with that, or would you like a VP to sign off (it's really a question of whether you consider this a "large" quantity of release users as it relates to the risk profile?
Flags: needinfo?(pascalc)
To clarify, my estimate of .5% of the latest version’s release population is based on needing about 85,000 users in a branch to detect a regression on the relevant metrics, times 4 branches (this isn't including retention, I can include that when I re-run it). I used the average number of unique clients on release, version 62 from the week starting on Sept 24 this year (about 68 million). This ends up being about .5% of this population.
Marnie, this is OK for relman provided that you have QA signoff, regards.
Flags: needinfo?(pascalc)
Science: R+
Flags: needinfo?(isegall)
Block Autoplay
Targeted: Firefox Release 63

We have finished testing the “Block Autoplay” experiment. 

QA’s recommendation: YELLOW - SHIP IT CONDITIONALLY

Reasoning: 
- We have found a new regression caused by the Bug 1476555 influencing the wrong increase of the “totalBlockedAudibleMedia” property on pages on which blocked audible media was not blocked. 
        - After discussing with the developer in charge on slack, it was concluded that this property of the “counts” ping will be ignored.
- We still stand by our previous statement that from the shield study perspective the experiment is looking ok. But we have found and logged several feature issues that were closed as intended behavior, but the QA team does not agree on the resolution of some of them.
https://github.com/alastor0325/autoplay-shield-study/issues/14 - The doorhanger is not dismissed when navigating to a different non-audio page of the same domain
https://github.com/alastor0325/autoplay-shield-study/issues/15 - The doorhanger is wrongly dismissed when you click inside the page on several websites
https://github.com/alastor0325/autoplay-shield-study/issues/16 - The doorhanger is wrongly dismissed on YouTube when you click specific player buttons
- The feature had 2 previously sign offs as YELLOW in Nightly and Beta channels. 

Testing Summary:
- Full Functional test suite: TestRail (https://testrail.stage.mozaws.net/index.php?/plans/view/12724)

Tested Platforms:
- Windows 7 x64
- Mac 10.13.3

Tested Firefox versions:
- Firefox RC build1
- Firefox RC build2
Flags: needinfo?(andreea.cupsa)
Hi, Micheal,

This one is the final version, which remove the testing flag in Telemetry ping.
Thank you.
Attachment #9015341 - Attachment is obsolete: true
Flags: needinfo?(mcooper)
Flags: needinfo?(mcooper)
We're live on the English version, but held on the rest.  We were unsure why the PHD asked for all locales when the repo only appears to be EN/FR; it's quite likely that I misunderstood your handling but we would like clarification.

Even then the survey needs translated to French (missed that requirement).

Thanks
Flags: needinfo?(alwu)
(In reply to Rob from comment #41)
> We're live on the English version, but held on the rest.  We were unsure why
> the PHD asked for all locales when the repo only appears to be EN/FR; it's
> quite likely that I misunderstood your handling but we would like
> clarification.
> 
> Even then the survey needs translated to French (missed that requirement).
> 
> Thanks

No matter what location user is, they will all have a chance to watch autoplay media content.

I'm not sure whether the survey has been translated to French. Rosanne, do we have a French verison for survey?
Flags: needinfo?(alwu) → needinfo?(rscholl)
(In reply to Alastor Wu [:alwu] from comment #42)
> (In reply to Rob from comment #41)
> > We're live on the English version, but held on the rest.  We were unsure why
> > the PHD asked for all locales when the repo only appears to be EN/FR; it's
> > quite likely that I misunderstood your handling but we would like
> > clarification.
> > 
> > Even then the survey needs translated to French (missed that requirement).
> > 
> > Thanks
> 
> No matter what location user is, they will all have a chance to watch
> autoplay media content.
> 
> I'm not sure whether the survey has been translated to French. Rosanne, do
> we have a French verison for survey?

No, there is currently no French version of the survey- we weren't aware. There is (just) time to get it localized.
Flags: needinfo?(rscholl)
If we're okay with using English for the very limited/not-very-user-facing strings in the study extension itself, then I'm fine with that decision.  It sounds like we do in fact want all locales.

Which brings us to the issue that it's not realistic to translate the survey for all locales.  I would suggest we do one of the following: 1) pick a limited subset of locales or 2) create another version of the extension that does not have a survey end state for non-English locales.  I think 2) would be faster / more realistic at this point.
Hi, Cindy, 
NI for comment44.
Flags: needinfo?(chsiang)
Hi Rob,
Yes we do want all locales. Let's take your recommendation to to option 2. Do you know how soon we can push that out?
Flags: needinfo?(chsiang) → needinfo?(rrayborn)
Hi, Micheal,
According to comment44, this one is the shield-study without ending survey.
Thank you.
Attachment #9015344 - Attachment is obsolete: true
Attachment #9018761 - Attachment is obsolete: true
Flags: needinfo?(mcooper)
Flags: needinfo?(mcooper)
Hi all, we're live with the non-English locales/non-survey version worldwide.  These versions released almost exactly 1 week apart so all analysts should keep that in mind.
Flags: needinfo?(rrayborn)
Cindy, can you please confirm that:

1. The English version of the study should end 2 weeks after the 10/23/18 launch date, on 11/6/18, and 
2. The non-English locales/non-survey version which launched 10/30/18 should end on 11/13/18.

So, both versions of the study will run for two weeks, correct?
Flags: needinfo?(chsiang)
I would think so but I'd defer it to WCB to give input from analysis point of view.
Flags: needinfo?(chsiang) → needinfo?(wbeard)
(In reply to chsiang from comment #51)
> I would think so but I'd defer it to WCB to give input from analysis point
> of view.

That sounds good to me.
Flags: needinfo?(wbeard)
Alrighty, thanks folks! I've got your updated end dates on our internal calendar.
Per the calendar I have ended the non-En version of the study. All users should unenroll over the next 24 hours.
Thanks, Matt.
Chris can we see the analysis when your'e back from PTO?
Flags: needinfo?(wbeard)
Sounds good
Flags: needinfo?(wbeard)
Component: Audio/Video: Playback → Shield Study
Product: Core → Shield
We've finished the shield-study.
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Flags: needinfo?(glind)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.