Commit d1fc13d5 authored by Robert Knight's avatar Robert Knight

Propagate `profileAppUrl` config to guest iframes

Propagate the `profileAppUrl` config setting from the initial frame where
Hypothesis is loaded to guest frames. The boot script expects this setting in
all non-sidebar frames, along with `sidebarAppUrl` and `notebookAppUrl`, because
it doesn't differentiate between host and guest frames when loading resources.
Ideally we would fix that, but it requires a more substantial refactoring that
I don't want to do just now.
parent f422054b
......@@ -99,16 +99,24 @@ export async function injectClient(
// We could potentially do this by allowing these settings to be part of
// the "annotator" config (see `annotator/config/index.js`) which gets passed
// to the constructor.
const { assetRoot, notebookAppUrl, sidebarAppUrl } =
const { assetRoot, notebookAppUrl, profileAppUrl, sidebarAppUrl } =
parseJsonConfig(document);
const injectedConfig = {
...config,
assetRoot,
// FIXME - We propagate these settings because the boot script expects them,
// but they shouldn't actually be needed when launching the client in a
// frame as a guest only (ie. no sidebar). A caveat is that the
// `<link>` element generated using the `sidebarAppUrl` value does also get
// used for other purposes by the annotator entry point.
notebookAppUrl,
profileAppUrl,
sidebarAppUrl,
// Tell the client that it should load as a guest only (no sidebar).
subFrameIdentifier: frameId ?? generateHexString(10),
};
......
......@@ -90,6 +90,7 @@ describe('HypothesisInjector integration test', () => {
assert.match(config.subFrameIdentifier, /[a-f0-9]+/);
assert.notOk(config.assetRoot);
assert.notOk(config.profileAppUrl);
assert.notOk(config.notebookAppUrl);
assert.notOk(config.sidebarAppUrl);
});
......@@ -112,6 +113,7 @@ describe('HypothesisInjector integration test', () => {
hostJSONConfig = {
clientUrl: 'chrome-extension://abc/client/build/boot.js',
assetRoot: 'chrome-extension://abc/client',
profileAppUrl: 'chrome-extension://abc/client/profile.html',
notebookAppUrl: 'chrome-extension://abc/client/notebook.html',
sidebarAppUrl: 'chrome-extension://abc/client/sidebar.html',
};
......@@ -127,6 +129,10 @@ describe('HypothesisInjector integration test', () => {
const config = JSON.parse(configElement.textContent);
assert.equal(config.assetRoot, 'chrome-extension://abc/client');
assert.equal(
config.profileAppUrl,
'chrome-extension://abc/client/profile.html'
);
assert.equal(
config.notebookAppUrl,
'chrome-extension://abc/client/notebook.html'
......
......@@ -133,7 +133,9 @@ function assetURL(config, path) {
/**
* Bootstrap the Hypothesis client.
*
* This triggers loading of the necessary resources for the client
* This triggers loading of the necessary resources for the client in a host
* or guest frame. We could in future simplify booting in guest-only frames
* by omitting resources that are only needed in the host frame.
*
* @param {Document} doc
* @param {AnnotatorConfig} config
......
......@@ -33,6 +33,8 @@ if (isBrowserSupported()) {
apiUrl: sidebarConfig.apiUrl,
});
} else {
// nb. If new asset URLs are added here, the browser extension and
// `hypothesis-injector.ts` need to be updated.
const annotatorConfig = /** @type {AnnotatorConfig} */ (config);
const notebookAppUrl = processUrlTemplate(
annotatorConfig.notebookAppUrl || '__NOTEBOOK_APP_URL__'
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment