Closed
Bug 1475099
Opened 6 years ago
Closed 6 years ago
[Shield] Opt-Out Study: Block Autoplay, Release 63
Categories
(Shield :: Shield Study, enhancement, P2)
Shield
Shield Study
Tracking
(firefox63+ fixed)
RESOLVED
FIXED
People
(Reporter: alwu, Assigned: alwu)
References
()
Details
User Story
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.
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Rank: 15
Priority: -- → P2
Assignee | ||
Comment 1•6 years ago
|
||
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)
Assignee | ||
Comment 2•6 years ago
|
||
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
Comment 3•6 years ago
|
||
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)
Comment 4•6 years ago
|
||
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)
Comment 5•6 years ago
|
||
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
Assignee | ||
Comment 6•6 years ago
|
||
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)
Comment 7•6 years ago
|
||
(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)
Assignee | ||
Comment 8•6 years ago
|
||
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)
Comment 9•6 years ago
|
||
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)
Comment 10•6 years ago
|
||
Attachment #8997885 -
Flags: review?(jhofmann)
Updated•6 years ago
|
Status: NEW → ASSIGNED
Updated•6 years ago
|
Summary: implement a shield-study extension for block autoplay → [Shield] Opt-Out Study: Block Autoplay
Comment 11•6 years ago
|
||
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-
Assignee | ||
Comment 12•6 years ago
|
||
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)
Assignee | ||
Comment 13•6 years ago
|
||
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!
Updated•6 years ago
|
Attachment #8997885 -
Flags: review?(jhofmann) → review+
Assignee | ||
Comment 14•6 years ago
|
||
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 15•6 years ago
|
||
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+
Assignee | ||
Comment 16•6 years ago
|
||
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)
Assignee | ||
Updated•6 years ago
|
User Story: (updated)
Assignee | ||
Comment 17•6 years ago
|
||
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)
Assignee | ||
Comment 18•6 years ago
|
||
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
Comment 20•6 years ago
|
||
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+
Assignee | ||
Comment 21•6 years ago
|
||
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)
Comment 22•6 years ago
|
||
Updated•6 years ago
|
Flags: needinfo?(mcooper)
Assignee | ||
Comment 24•6 years ago
|
||
FYI, this bug will be closed after QA testing finishes, it's still in process.
Assignee | ||
Comment 25•6 years ago
|
||
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)
Comment 26•6 years ago
|
||
: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)
Assignee | ||
Comment 27•6 years ago
|
||
Thank you!
Attachment #9015304 -
Attachment is obsolete: true
Flags: needinfo?(mcooper)
Comment 28•6 years ago
|
||
Updated•6 years ago
|
Flags: needinfo?(mcooper)
Assignee | ||
Comment 29•6 years ago
|
||
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)
Comment 30•6 years ago
|
||
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)
Comment 31•6 years ago
|
||
[Tracking Requested - why for this release]:
status-firefox63:
--- → affected
tracking-firefox63:
--- → ?
Summary: [Shield] Opt-Out Study: Block Autoplay → [Shield] Opt-Out Study: Block Autoplay, Release 63
Comment 32•6 years ago
|
||
Proposed study release date is 2018-10-23, running for 2 weeks, per the PHD.
Updated•6 years ago
|
Comment 33•6 years ago
|
||
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
Comment 34•6 years ago
|
||
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)
Comment 35•6 years ago
|
||
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.
Comment 36•6 years ago
|
||
Marnie, this is OK for relman provided that you have QA signoff, regards.
Flags: needinfo?(pascalc)
Comment 38•6 years ago
|
||
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)
Assignee | ||
Comment 39•6 years ago
|
||
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)
Comment 40•6 years ago
|
||
Updated•6 years ago
|
Flags: needinfo?(mcooper)
Comment 41•6 years ago
|
||
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)
Assignee | ||
Comment 42•6 years ago
|
||
(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)
Comment 43•6 years ago
|
||
(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)
Comment 44•6 years ago
|
||
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.
Comment 46•6 years ago
|
||
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)
Assignee | ||
Comment 47•6 years ago
|
||
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)
Comment 48•6 years ago
|
||
Updated•6 years ago
|
Flags: needinfo?(mcooper)
Comment 49•6 years ago
|
||
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)
Comment 50•6 years ago
|
||
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)
Comment 51•6 years ago
|
||
I would think so but I'd defer it to WCB to give input from analysis point of view.
Flags: needinfo?(chsiang) → needinfo?(wbeard)
Comment 52•6 years ago
|
||
(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)
Comment 53•6 years ago
|
||
Alrighty, thanks folks! I've got your updated end dates on our internal calendar.
Comment 54•6 years ago
|
||
Per the calendar I have ended the non-En version of the study. All users should unenroll over the next 24 hours.
Comment 55•6 years ago
|
||
Thanks, Matt.
Chris can we see the analysis when your'e back from PTO?
Flags: needinfo?(wbeard)
Updated•6 years ago
|
Component: Audio/Video: Playback → Shield Study
Product: Core → Shield
Comment 57•6 years ago
|
||
Here's a link to the summary of my analysis:
https://gist.github.com/wcbeard/2264264bc4ce673402ab165afdf08b16
Assignee | ||
Comment 58•6 years ago
|
||
We've finished the shield-study.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: needinfo?(glind)
Resolution: --- → FIXED
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•