Enforce non-empty configuration when boot script is run from the browser extension

[acelaya]

This is an attempt to work around what's described in hypothesis/browser-extension#1179

This would cover only the usage of the browser extension in webs. For PDFs, a similar check needs to be done in the pdfjs-init, where we can directly prevent the loading of the boot script if there's no config.

Considerations on this PR / topics to discuss.

• I have decided to throw an error when the browser extension executes the boot script and no config is found. Another option would be to silently early-exit.
• The logic to determine if the script was run by the browser extension couple a bit the client with the chrome extension. Not sure if there's a better way to do this though.
• I haven't covered this with tests, as we don't seem to have a test for the index file. Perhaps we could extract the content of index to a function that can be tested and also imported from the index, leaving the index to a simple import-and-run-function script.

Testing steps

1. Checkout this branch
2. Build and publish: make build && npx yalc publish.
3. On your local browser-extension repo, install the client you just published: npx yalc add hypothesis.
4. Fake the config not being loaded, by commenting out injectConfig from the injectIntoHTML function:

   /**
    * @param {Tab} tab
    * @param {object} config
    */
   async function injectIntoHTML(tab, config) {
   - await injectConfig(tab.id, config);
   + // await injectConfig(tab.id, config);
     return executeClientBootScript(tab.id);
   }

5. Build the dev extension make build SETTINGS_FILE=settings/<custom.json>
6. Load your local extension in the browser.
7. Try to activate the extension on a page. It should not load the sidebar, and you should see the error in the browser console.

[codecov]

Codecov Report

Merging #5392 (4c99404) into main (1c07780) will not change coverage.
The diff coverage is n/a.

❗ Current head 4c99404 differs from pull request most recent head c7fde3c. Consider uploading reports for the commit c7fde3c to get more accurate results

@@           Coverage Diff           @@
##             main    #5392   +/-   ##
=======================================
  Coverage   99.19%   99.19%           
=======================================
  Files         237      237           
  Lines        9403     9403           
  Branches     2243     2243           
=======================================
  Hits         9327     9327           
  Misses         76       76           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[robertknight]

For PDFs, a similar check needs to be done in the pdfjs-init, where we can directly prevent the loading of the boot script if there's no config.

pdfjs-init.js adds the configuration itself, before loading the boot script. So currently it is not possible for the config to be missing. Also in that context, since the script is run via an ordinary <script> tag rather than via chrome.scripting.executeScript, I'm not sure that chrome.runtime.id will be available.

[robertknight]

The logic to determine if the script was run by the browser extension couple a bit the client with the chrome extension. Not sure if there's a better way to do this though.

Since the code is super simple and easy to change, I think this is OK. If we find it causes a problem we can revise it.

[robertknight]

LGTM in principle. See suggestion for revised message.

[robertknight]

It is possible the page may contain its own Hypothesis configuration. What we really want to check for here is the config that corresponds to the specific extension being used. One way to do this might be to add an attribute to the configuration <script> indicating the ID of the extension

[acelaya]

Yeah, you are right. In fact I run into this while testing on a page that precisely had its own config and it took me some attempts to realize what was going on 😅

I like the idea of the attribute with the extension ID. I'm going to merge this as is, address your suggestion in the browser extension, and then come back here and improve it.

Thanks!.

[robertknight]

Suggested change

	'Could not bootstrap hypothesis as there is no configuration'
	'Could not start Hypothesis extension as configuration is missing'

Tweaked this wording to capitalize Hypothesis, mention the extension specifically, and prefer start or launch as probably more meaningful than "bootstrap" for users rather than developers of Hypothesis.