content-script lacks tests required to ensure stability and accuracy

[NicholasEllul]

What is this about?

Context

The inpage bridge content-script is a file that the application injects into the various webpages visited within the in-app browser. The moment that the file is included in its javascript environment, it immediately executes while analyzing that the page meets the required criteria for the MetaMask provider to be injected.

Problem

Despite this file playing a key role in determining if MetaMask is able to work on a page, we lack the necessary tests to ensure it's stability and functional accuracy. As described in #8675 it is not straightforward to test this file due to the fact that it will execute it's code the moment it gets imported (see code). This means that in a test file, it will attempt to run it's logic prior to the developer being able to set up the necessary mocks required to mock out the window object.

Potential Solutions

Simple Solution
An initial solution would be to refactor the content-script to not execute as soon as it is imported into a javascript environment. Instead, it should be imported, and require an explicit functional call in order for it to be run. This is the approach taken by extension who exports a shouldInjectProvider that is later called explicitly. This allows for the provider to be explicitly tested as seen in extension here.

Likewise, SDK takes an even more modular approach of abstracting out each function used to determine if the provider should be injected or not. This allows for even more precise testing of the logic.

@NicolasMassart suggested that if we take a similar approach, we can instead invoke the function explicitly in one of the WebView callbacks once the page has loaded (e.g onMessage).

Ideal solution

As this code is duplicated in Extension, Mobile, and SDK, an ideal solution would be to pull this into a single package that is then imported in used across all our products. This would simplify the process and eliminate each team needing to maintain this code individually.

Scenario

No response

Design

No response

Technical Details

No response

Threat Modeling Framework

No response

Acceptance Criteria

No response

Stakeholder review needed before the work gets merged

• Engineering (needed in most cases)
• Design
• Product
• QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
• Security
• Legal
• Marketing
• Management (please specify)
• Other (please specify)

References

No response