feat: Add data collection opt out [#1416]

Description

This MR introduces a new preference, only for firefox user.

This preference is located in the advanced tab and allows users to opt-out of data tracking.
This is a requirement from the AMO following the introduction of the IPM feature and our integration with moengage.

For more details on the feature please check the issue or the spec MR that also includes discussing edge cases and decisions around those.

It works 🧟‍♂️

CleanShot_2023-11-27_at_14.30.14

Technical decisions/explainations

This MR has 3 commits doings the following:

The new pref

Three things to do here:

• Add the boilerplate JS for a new pref
• Add the HTML markup in the desktop options page
• Use CSS to only show the HTML in firefox

Improving the Jest setup

I really wanted to be able to add some units tests for this feature so I gave it a shot. First I had to improve our Jest setup to be able to actually write the test:

1. Improving webextension mocking

We were only relying on some manual mocks done in mocks/js/polyfill.js, these are missing most of the webext APIs. If we are going to write some unit tests we just need a lot more webext APIs to be mocked or polyfill in a node/jest enviornment. I did look around and ended up picking jest-webextension-mock. The reasons for this choice are:

• It is open source, somewhat popular and maintained (cf. npm or the number of repo using it in GitHub, I did find extensions with 100K+ users using this for their testing)
• I looked at the source code and it is really replacing all webext API methods with jest.fn() which is good enough for us

Other options:

• sinon-chrome, which is what AdGuard is using. That repo hasn't been touched in over 5 years (except for some minor dep upgrades), it's also relying on sinon which is really an unneeded dependency when using jest, as jest already has all the mocking/spying/stubbing features.
• Doing all the mocking ourselves (i.e. extending the current polyfills): I don't think it's worth it. If we really want to have control over this then we should fork jest-webextension-mock (I actually believe it would be really great to do this as this lib could use more active stewardship and Adblock/eyeo is the perfect place for that and add some visibility to our work in the webextension development world).

Remarks:

• jest-webextension-mock is missing some APIs, so I created a mocks/js/jest-polyfill.js that adds the methods we miss from the library. I also made an upstream PR and hit up the guy on twitter, let's see how it goes, seems like he is now a Sr. Director of Product so i'm not sure he has much time for maintaining an open source repository 😞 (One of the argument for eventually forking this repo)
• mocks/js/polyfill.js is used in build/webext/config/local.mjs so I assume it's not safe to delete it. I'm curious why we need this in the build when we already have webextension-polyfill 🤔 ?

All in all I believe jest-webextension-mock is a great fit for us right now and we'd be able to keep that polyfilling setup for a long time by just extending the things we need.

2. Fetch

Using fetch in jest (and asserting around fetch) is a big topic with a lot of different opinions. I don't think we need to get into those right now, so to keep things simple I just tell jest to use the native fetch from NodeJS and write my assertion by using some basic spies on the global fetch function.

When it comes to working with data fetching in tests or dev envs, i'm a really big fan of MSW which I used on several projects in the past with great success, but I digress, let's keep things simple for now and we talk more about data fetching in tests when/if we run into this problem more frequently.

3. The info module

old version before we merged !947 (merged) that really improved the situation

The info module is generated at build time, thus it is not available when running tests in jest. I noticed that we had this `mocks/js/config/info.mjs` file that is not used anywhere in the codebase, so I repurposed it as our mock for the info module in jest environment (which, looking at where the file is located, seems to have been the intention with this file).

I had to change this file from .mjs to .js because jest does not really support ESM right now. I tried to stick to an ESM format, but ran into some tricky bits as our project is not set as a Node ESM project (we use a mix of commonJS and ESM and rely on our bundler to solve that). It was much easier to switch to commonJS.

As the info module is a webpack alias, I use the jest moduleNameMapper config, which more or less does the same thing as a webpack alias.

Since !947 (merged) it's much easier to mock the info module as it's not anymore a module that is only dynamically generated at build time and only referenced via a webpack alias. Now that there is an actual JS module + file for it, mocking it is straightforward.

This stuff could also go in the jest setup file and we could provide some helpers to control the info module, but for now we can make do with a simple mock inlined in the test file, and add more abstraction if/when we feel it's necessary.

The test

Here we unit test at the "highest level" possible: we check that the http request that we must disable when data collection opt out is disabled is being done or not done as expected. There is only a single function where this HTTP call is made: sendPing() in src/ipm/background/telemetry.ts.

This is because we are testing this function that we needed to extend our polyfilling, handle fetch, and mock the info module in jest execution environment, as all of these things are dependencies for the sendPing function. Finally, we also need to be able to mock Prefs, as this is what is deciding if we should send the http request or not.

To keep things simple I mock Prefs in the file directly. We'll most likely run into this again when writing unit tests but for now, since it Prefs mocking is only required in one file, I don't want to write jest configs/abstraction/test helpers for Prefs related stuff, let's just cross that bridge when we get there.

Random remarks:

ESlint config override weirdsies

We may have something weird going on with our eslint config: I noticed that for commonJS files we add an /* eslint-env node */ comment at the top. I did try to do the same change by adjusting the eslint config and adding an override with a env: { node: true } but that didn't work. Moreover, our eslint/standard config has the following:

overrides: [
  {
    // Let ESLint know that we use Jest for our specs.
    files: ["*.spec.ts"],
    env: {
      jest: true
    }
  }
]

• Pretty sure the files should be **/*.spec.ts and not *.spec.ts
• Removing that part of the config files does not change anything
• In general, using overrides in eslint/standrad.js did not seem to work with the few things I tried.

I'm probably missing something obvious here 🤔

To extract or not extract the Jest setup from this MR?

I don't have a strong opinion on this one. We could remove the jest setup and test from this MR, however I believe that every code that gets checked in should have tests (to the best of our abilities) associated with it. If my choices/changes in the config are causing too much discussion and blocking the MR then let's remove them from this MR and have that separately.

Related to #1416 (closed)