block autoplay shield-study extension #3
Conversation
According to [1], the clientId should be a part of outer common ping format, instead of being a part of payload. [1] https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/data/common-ping.html#common-ping-format
Should also change the config of "xpinstall.signatures.required" and "extensions.legacy.enabled" when the extension hasn't be signed yet.
User can have different ways to interact with the prompt, they can either click a button, hit the
‘escape’ key, or ignore it and do nothing.
Therefore, we add new attribute `interact` with string enum value {interact, escape, ignore}.
In addition, `rememberCheckbox` and `allowAutoPlay` will only be included if the value of `interact` is `interact`.
`responseTime` is measuring how long the prompt shows before users make their response.
`responseTime` is measuring how long the prompt shows before users make their response.
According to our final decision, we would change the number of testing cohorts to 4. 1. control (always allow) 2. block (always block) 3. allow-and-remember 4. allow-and-notRemember
alastor0325
force-pushed
the
develop
branch
from
b203240
to
c7cbfd1
Compare
Modify the description about testing branches.
alastor0325
force-pushed
the
develop
branch
from
c7cbfd1
to
126e97a
Compare
| configure(studyInfo) { | ||
| const feature = this; | ||
| const { variation, isFirstRun } = studyInfo; | ||
| function generateRandomString() { |
There was a problem hiding this comment.
There's a service in Firefox privileged JS to generate UUIDs if you need that.
There was a problem hiding this comment.
I thought that we can only use privileged JS code in experiments API, not in background script?
| } | ||
|
|
||
| function getHostName(url) { | ||
| return url.match(/^(?:https?:\/\/)?(?:[^@\n]+@)?(?:www\.)?([^:\/\n]+)/im)[1]; |
There was a problem hiding this comment.
Could use the standard URL API to tell if a URL is valid, extract the hostname etc. instead of having to do your own regex.
There was a problem hiding this comment.
In about:preferences, user can set any url into "Autoplay - Exception" which might include some formats which URL API doesn't accept. (eg. "123.com" is not a valid input format for URL API.)
Therefore, it means I need to do an additional transformation and check for every input, it would be more complicated. I would prefer to use my own regex rather than doing that.
| return Services.strings.createBundle("chrome://browser/locale/browser.properties"); | ||
| }); | ||
|
|
||
| function _once(target, name) { |
There was a problem hiding this comment.
Hm. I might be misunderstanding the intent here, but addEventListener takes a boolean named once if you just want an event listener to fire a single time:
There was a problem hiding this comment.
Since I want to use await pattern here for waiting until the event happen, _once is just a wrapper helper function.
| const { AddonStudies } = ChromeUtils.import("resource://normandy/lib/AddonStudies.jsm", {}); | ||
| const { EventManager } = ExtensionCommon; | ||
|
|
||
| XPCOMUtils.defineLazyGetter(this, "gBrowserBundle", function() { |
There was a problem hiding this comment.
You don't need to import XPCOMUtils for this anymore, ChromeUtils supports it like this:
ChromeUtils.defineModuleGetter(this, "MyModule",
"resource://MyModule.jsm");
There was a problem hiding this comment.
But here I don't define a module, I defined a getter for the variable. It seems not equal to the usage of defineModuleGetter()?
In [1], we have already made TelemetryController to generate ID, we don't need this function anymore. [1] e45a4c6
update version number in order to get second signed version extention.
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.