Compatibility/System Addon/Override Policies and Workflows: Difference between revisions

From MozillaWiki
Jump to navigation Jump to search
(updated to suggest window.eval instead of window.wrappedJSObject.eval)
(added automated testing section/link)
 
(20 intermediate revisions by 2 users not shown)
Line 1: Line 1:
== Preamble ==
= Preamble =


There is no fixed guideline or a checklist on when to add an override or not. Instead, this project is rather dynamic and depends on the individual case. This document provides some help in making that decision.
If important sites are broken in Firefox, getting them fixed is our highest priority (even if by the user agent intervening on behalf of the user). However, there's a balance to be struck: we do not have the bandwidth to fix the entire web, and patching certain problem may disincentivize developers from fixing their bugs.


In general, decisions are made rather fast and we are able to roll out new overrides to the users quickly. ['''ToDo''': Who makes the decision? I feel like more than one person should be involved, but I am not sure who. Maybe a quick discussion on the webcompat bug to see if anyone has objections?]
As a guiding principle, if deploying a site patch or override would dramatically improve user experience or fix a top site it should be seriously considered. At the same time, we make efforts to remove the override as soon as possible, working with the site developers where possible.


The user is more important than the web and the web is more important than browser vendors. If sites are broken, getting them working again should have the highest priority. On the other hand, if we add overrides every time something breaks, we might not be able to convince the site developers to roll out a fix since the site will be working in their tests.
Consult [[Compatibility/Interventions_Releases|Intervention Releases]] for more information on the process of shipping interventions.


== Override or not? ==
= To Override or not? =


We try to make this decision mainly based on the sites popularity. There is no point in adding an override for a small site with a high likelihood our suggestions will never be implemented. Ultimately, all overrides should be a '''temporary measure, not a long-term fix'''.
Our goal is to fix top sites, or sites that will have the most impact for Firefox users.


* If a site is very popular, a lot of users may be affected by the bug and we should put an override in place before going into outreach (or, even better, do both at the same time) to reduce the user impact.
* It's important to consider a site's popularity on a by-country level, in addition to global level.
* We have no fixed threshold on the sites rank that qualifies for an override. However, we might set a soft limit in the future if we have some experience.
* We have no fixed threshold on the sites rank that qualifies for an override. However, we might set a soft limit in the future when we have more experience deploying site patches.
* It's important to consider a site's popularity on a by-country level, not a global level. Some sites may be very popular in one market, but completely irrelevant to others. While this particular case might not have a huge global impact, losing users from one market can be painful.
* Trending sites should also be considered, even when they don't rank highly in global usage metrics.
* Some sites may be very popular for a short period of time, but not popular at all in a global statistic. Examples are marketing sites for events, projects from raising startups, ... It's important to consider those as well.


== Generic Process of adding an override ==
= Types of Interventions =


While the code of our Go Faster Addon is hosted [https://github.com/mozilla/webcompat-addon on GitHub], additional steps need to be done for adding an override.
== User Agent overrides ==


# There should be an issue on GitHub or Bugzilla. In that bug, diagnosis needs to be done first to validate that the issue is indeed caused by a user agent detection (if you plan on adding an UA override), or can be fixed with injecting JS or CSS.
=== Designing the override ===
# If a user agent detection issue is confirmed, there should be some investigations on the least invasive UA override possible.
# When the "minimum viable product" was found, a pull request against the [https://github.com/mozilla/webcompat-addon GitHub repo] should be opened.
# At the same time, a test case for validating the need of this override should be written. ['''Note''': this is an ongoing project and there will be more details on that in the future.] That way, we will get notified if the site got fixed and the user agent is no longer needed. ['''ToDo''': will our testing framework also include a time based notification so we don't forget that we need to remove old overrides?]
# If both the override and the test are validated, the PR should get merged.
# If the PR is merged, a new version of the Go Faster addon should be released and rolled out to the users. ['''ToDo''': clarify how.]
 
== Types of Overrides ==
 
=== User Agent overrides ===
 
==== Designing the override ====


* Since we are able to set a very specific scope for overrides, we aim to limit the scope of overrides as much as possible. If only a specific site in a specific subdirectory is affected, it's much better to override just that segment instead of overriding the UA for the entire site.
* Since we are able to set a very specific scope for overrides, we aim to limit the scope of overrides as much as possible. If only a specific site in a specific subdirectory is affected, it's much better to override just that segment instead of overriding the UA for the entire site.
* We try to touch the user agent as little as possible. If adding a segment like "mobile" or "iPhone" solves an issue, this is should be preferred over replacing the entire UA. That way, Firefox still might be recognized by the sites statistics.
* We try to touch the user agent as little as possible. If adding a segment like "mobile" or "iPhone" solves an issue, this is should be preferred over replacing the entire UA. That way, Firefox still might be recognized by the sites statistics.


==== Technical details ====
=== Technical details ===


User agent overrides are defined within [https://github.com/mozilla/webcompat-addon/blob/master/src/content/data/ua_overrides.jsm src/content/data/ua_overrides.jsm] of the addon sources. In this file, you will only find an array containing all active user agent overrides. Each object can have three attributes:
User agent overrides are defined within [https://github.com/mozilla/webcompat-addon/blob/master/src/data/ua_overrides.js src/data/ua_overrides.js] of the addon sources. In this file, you will only find an array containing all active user agent overrides. Each object can have three attributes:


* ''baseDomain'', required: The base domain that further checks and user agents override are applied to. This does not include subdomains.
* ''baseDomain'', required: The base domain that further checks and user agents override are applied to. This does not include subdomains.
Line 70: Line 58:
</syntaxhighlight>
</syntaxhighlight>


=== CSS Injections ===
== CSS Injections ==


==== Designing the injection ====
=== Designing the injection ===


As with user agent overrides, CSS injections should be designed to use as little code as possible. Due to the nature of these overrides, it is hard to provide exact guidelines.
As with user agent overrides, CSS injections should be designed to use as little code as possible. Due to the nature of these overrides, it is hard to provide exact guidelines.
Line 78: Line 66:
Injected CSS files will be in the first place of the cascading chain, so be careful to use selectors with a high specificity, or use the ''!important'' keyword.
Injected CSS files will be in the first place of the cascading chain, so be careful to use selectors with a high specificity, or use the ''!important'' keyword.


==== Technical details ====
=== Technical details ===


CSS injections are defined within individual files within the [https://github.com/mozilla/webcompat-addon/blob/master/src/webextension/injections/css/ src/webextension/injections/css/] directory of the addon sources. In this folder, you will find a collection of files, one file per override or injection. For easier cross-reference, please stick to the ''bugNNNNN-short-description.css'' file name schema.
CSS injections are defined within individual files within the [https://github.com/mozilla/webcompat-addon/blob/master/src/webextension/injections/css/ src/webextension/injections/css/] directory of the addon sources. In this folder, you will find a collection of files, one file per override or injection. For easier cross-reference, please stick to the ''bugNNNNN-short-description.css'' file name schema.


In addition, the injection file has to be registered in the manifest file located at [https://github.com/mozilla/webcompat-addon/blob/master/src/webextension/manifest.json src/webextension/manifest.json]. Details on available parameters can be found [https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/content_scripts in the MDN web docs].
In addition, the injection file has to be registered in the constant array located at the top of [https://github.com/mozilla/webcompat-addon/blob/master/src/webextension/background.js#L7 src/webextension/background.js]. Details on available parameters can be found [https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/content_scripts in the MDN web docs].


=== JS Injections ===
== JS Injections ==


==== Designing the injection ====
=== Designing the injection ===


As with user agent overrides, JS injections should be designed to use as little code as possible. Due to the nature of these overrides, it is hard to provide exact guidelines.
As with user agent overrides, JS injections should be designed to use as little code as possible. Due to the nature of these overrides, it is hard to provide exact guidelines.


The injected JS will be executed in its own context per default, check [https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Content_scripts#Content_script_environment the MDN web docs] for details. If it is required to run JS in the websites context, JS code can be wrapped inside an ''eval'' call:
The injected JS will be executed in its own context per default, check [https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Content_scripts#Content_script_environment the MDN web docs] for details. If it is required to run JS in the websites context, functions can be exported via [https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.exportFunction exportFunction]:


<syntaxhighlight lang="js">
<syntaxhighlight lang="js">
window.eval(`(function() { // used to require window.wrappedJSObject.eval
Object.defineProperty(window.wrappedJSObject, "isTestFeatureSupported", {
   /* your code here */
  get: exportFunction(function() {
}());`);
    return true;
  }, window),
 
   set: exportFunction(function() {}, window)
});
</syntaxhighlight>
</syntaxhighlight>


==== Technical details ====
=== Technical details ===


JS injections are defined within individual files within the [https://github.com/mozilla/webcompat-addon/blob/master/src/webextension/injections/js/ src/webextension/injections/js/] directory of the addon sources. In this folder, you will find a collection of files, one file per override or injection. For easier cross-reference, please stick to the ''bugNNNNN-short-description.js'' file name schema.
JS injections are defined within individual files within the [https://github.com/mozilla/webcompat-addon/blob/master/src/webextension/injections/js/ src/webextension/injections/js/] directory of the addon sources. In this folder, you will find a collection of files, one file per override or injection. For easier cross-reference, please stick to the ''bugNNNNN-short-description.js'' file name schema.


In addition, the injection file has to be registered in the manifest file located at [https://github.com/mozilla/webcompat-addon/blob/master/src/webextension/manifest.json src/webextension/manifest.json]. Details on available parameters can be found [https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/content_scripts in the MDN web docs].
In addition, the injection file has to be registered in the array located in [https://github.com/mozilla/webcompat-addon/blob/master/src/data/injections.js rc/data/injections.js]. Details on available parameters can be found [https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/content_scripts in the MDN web docs].
 
=== Using "custom functions" for more complex overrides
 
Some injections may require additional logic inside the WebExtension to make them work. An example of this could be a custom blocking ''browser.webRequest.onBeforeRequest'' handler to detect and alter the usage of a popular JavaScript library. To use customized logic, define a ''customFunc'' in your injection definition:
 
<syntaxhighlight lang="js">
{
  id: "bug1551672",
  platform: "android",
  domain: "Sites using PDK 5 video",
  bug: "1551672",
  data: {
    urls: ["https://*/*/tpPdk.js", "https://*/*/pdk/js/*/*.js"],
    types: ["script"],
  },
  customFunc: "pdk5fix",
}
</syntaxhighlight>
 
Then, head to ''src/lib/custom_functions.js'' and make sure to define the function you just named. Suppose you need additional logic for clearing up when disabling the intervention, for example, to unregister a handler. In that case, you can define a second function with the ''Disable'' suffix, ''pdk5fixDisable'' in this example.
 
Custom functions are executed within the background script and have full permissions to do anything that a WebExtension background script can do.
 
= Automated Testing =
 
See https://wiki.mozilla.org/Compatibility/System_Addon/Automated_Testing


== Open points for further discussion ==
= Open points for further discussion =


Some cases might never be added to this decision guide, but discussed with the team on the individual case:
Some cases might never be added to this decision guide, but discussed with the team on the individual case:


* Should we override/inject if a site adds a "Your browser is not supported" note? What if the warning is really annoying or even blocking the site, what if it's just a note?
* Should we override/inject if a site adds a "Your browser is not supported" note? What if the warning is really annoying or even blocking the site, what if it's just a note?
* How do we make sure that our override is indeed working? What do we need to test?
* Do we override websites that require a login? How can we ensure that we don't break the site with overriding the user agent or injecting code?
* Do we override websites that require a login? How can we ensure that we don't break the site with overriding the user agent or injecting code?


[[Category:Web Compatibility]]
[[Category:Web Compatibility]]

Latest revision as of 20:34, 13 June 2023

Preamble

If important sites are broken in Firefox, getting them fixed is our highest priority (even if by the user agent intervening on behalf of the user). However, there's a balance to be struck: we do not have the bandwidth to fix the entire web, and patching certain problem may disincentivize developers from fixing their bugs.

As a guiding principle, if deploying a site patch or override would dramatically improve user experience or fix a top site it should be seriously considered. At the same time, we make efforts to remove the override as soon as possible, working with the site developers where possible.

Consult Intervention Releases for more information on the process of shipping interventions.

To Override or not?

Our goal is to fix top sites, or sites that will have the most impact for Firefox users.

  • It's important to consider a site's popularity on a by-country level, in addition to global level.
  • We have no fixed threshold on the sites rank that qualifies for an override. However, we might set a soft limit in the future when we have more experience deploying site patches.
  • Trending sites should also be considered, even when they don't rank highly in global usage metrics.

Types of Interventions

User Agent overrides

Designing the override

  • Since we are able to set a very specific scope for overrides, we aim to limit the scope of overrides as much as possible. If only a specific site in a specific subdirectory is affected, it's much better to override just that segment instead of overriding the UA for the entire site.
  • We try to touch the user agent as little as possible. If adding a segment like "mobile" or "iPhone" solves an issue, this is should be preferred over replacing the entire UA. That way, Firefox still might be recognized by the sites statistics.

Technical details

User agent overrides are defined within src/data/ua_overrides.js of the addon sources. In this file, you will only find an array containing all active user agent overrides. Each object can have three attributes:

  • baseDomain, required: The base domain that further checks and user agents override are applied to. This does not include subdomains.
  • applications: Array of applications this override is valid in. Defaults to ["firefox"], can be one or more of:
    • firefox: Firefox Desktop (regardless of the operating system)
    • fennec: Firefox for Android
  • uriMatcher: Function that gets the requested URI passed in the first argument and needs to return boolean whether or not the override should be applied. If not provided, the user agent override will be applied every time.
  • uaTransformer, required: Function that gets the original Firefox user agent passed as its first argument and needs to return a string that will be used as the the user agent for this URI.

Examples

Gets applied for all requests to mozilla.org and subdomains made on Firefox Desktop:

  {
    baseDomain: "mozilla.org",
    uriMatcher: (uri) => uri.includes("/app/"),
    uaTransformer: (originalUA) => `Ohai Mozilla, it's me, ${originalUA}`
  }

Applies to *.example.com/app/* on Firefox for Android:

  {
    baseDomain: "example.com",
    applications: ["fennec"],
    uriMatcher: (uri) => uri.includes("/app/"),
    uaTransformer: (originalUA) => originalUA.replace("Firefox", "Otherfox")
  }

CSS Injections

Designing the injection

As with user agent overrides, CSS injections should be designed to use as little code as possible. Due to the nature of these overrides, it is hard to provide exact guidelines.

Injected CSS files will be in the first place of the cascading chain, so be careful to use selectors with a high specificity, or use the !important keyword.

Technical details

CSS injections are defined within individual files within the src/webextension/injections/css/ directory of the addon sources. In this folder, you will find a collection of files, one file per override or injection. For easier cross-reference, please stick to the bugNNNNN-short-description.css file name schema.

In addition, the injection file has to be registered in the constant array located at the top of src/webextension/background.js. Details on available parameters can be found in the MDN web docs.

JS Injections

Designing the injection

As with user agent overrides, JS injections should be designed to use as little code as possible. Due to the nature of these overrides, it is hard to provide exact guidelines.

The injected JS will be executed in its own context per default, check the MDN web docs for details. If it is required to run JS in the websites context, functions can be exported via exportFunction:

Object.defineProperty(window.wrappedJSObject, "isTestFeatureSupported", {
  get: exportFunction(function() {
    return true;
  }, window),

  set: exportFunction(function() {}, window)
});

Technical details

JS injections are defined within individual files within the src/webextension/injections/js/ directory of the addon sources. In this folder, you will find a collection of files, one file per override or injection. For easier cross-reference, please stick to the bugNNNNN-short-description.js file name schema.

In addition, the injection file has to be registered in the array located in rc/data/injections.js. Details on available parameters can be found in the MDN web docs.

=== Using "custom functions" for more complex overrides

Some injections may require additional logic inside the WebExtension to make them work. An example of this could be a custom blocking browser.webRequest.onBeforeRequest handler to detect and alter the usage of a popular JavaScript library. To use customized logic, define a customFunc in your injection definition:

{
  id: "bug1551672",
  platform: "android",
  domain: "Sites using PDK 5 video",
  bug: "1551672",
  data: {
    urls: ["https://*/*/tpPdk.js", "https://*/*/pdk/js/*/*.js"],
    types: ["script"],
  },
  customFunc: "pdk5fix",
}

Then, head to src/lib/custom_functions.js and make sure to define the function you just named. Suppose you need additional logic for clearing up when disabling the intervention, for example, to unregister a handler. In that case, you can define a second function with the Disable suffix, pdk5fixDisable in this example.

Custom functions are executed within the background script and have full permissions to do anything that a WebExtension background script can do.

Automated Testing

See https://wiki.mozilla.org/Compatibility/System_Addon/Automated_Testing

Open points for further discussion

Some cases might never be added to this decision guide, but discussed with the team on the individual case:

  • Should we override/inject if a site adds a "Your browser is not supported" note? What if the warning is really annoying or even blocking the site, what if it's just a note?
  • Do we override websites that require a login? How can we ensure that we don't break the site with overriding the user agent or injecting code?