Commit e2547867 authored by Robert Knight's avatar Robert Knight

Stop loading annotator.css bundle into host page

With the exception of highlight styles, the styles in annotator.css are
now only used by UI elements contained within shadow roots. As a result,
we can stop loading annotator.css as a stylesheet into the host page,
preventing these styles from affecting the host page.

 - Stop loading annotator.css into the host page as a stylesheet.
   Instead add it as a preload link instead, ready for use by shadow
   roots.

 - Move highlights styles into a separate bundle and load that in the
   host page.

Fixes https://github.com/hypothesis/client/issues/2979
parent d61c6cf5
......@@ -18,6 +18,7 @@ gulp.task('build-css', () =>
buildCSS([
// Hypothesis client
'./src/styles/annotator/annotator.scss',
'./src/styles/annotator/highlights.scss',
'./src/styles/annotator/pdfjs-overrides.scss',
'./src/styles/sidebar/sidebar.scss',
......
......@@ -2,9 +2,10 @@
* Load stylesheets for annotator UI components into the shadow DOM root.
*/
function loadStyles(shadowRoot) {
// Find the preloaded stylesheet added by the boot script.
const url = /** @type {HTMLLinkElement|undefined} */ (
document.querySelector(
'link[rel="stylesheet"][href*="/build/styles/annotator.css"]'
'link[rel="preload"][href*="/build/styles/annotator.css"]'
)
)?.href;
......@@ -13,6 +14,10 @@ function loadStyles(shadowRoot) {
}
const linkEl = document.createElement('link');
// This needs to match the `crossorigin` attribute on the preloaded stylesheet.
linkEl.crossOrigin = 'anonymous';
linkEl.rel = 'stylesheet';
linkEl.href = url;
shadowRoot.appendChild(linkEl);
......
......@@ -3,14 +3,24 @@ import { createShadowRoot } from '../shadow-root';
describe('annotator/util/shadow-root', () => {
let applyFocusVisiblePolyfill;
let container;
let preloadedStylesheet;
beforeEach(() => {
// Create the stylesheet preload that would normally be added by the boot script.
preloadedStylesheet = document.createElement('link');
preloadedStylesheet.rel = 'preload';
preloadedStylesheet.as = 'style';
preloadedStylesheet.href = '/build/styles/annotator.css';
document.head.append(preloadedStylesheet);
container = document.createElement('div');
applyFocusVisiblePolyfill = window.applyFocusVisiblePolyfill;
window.applyFocusVisiblePolyfill = sinon.stub();
});
afterEach(() => {
preloadedStylesheet.remove();
container.remove();
window.applyFocusVisiblePolyfill = applyFocusVisiblePolyfill;
});
......@@ -39,7 +49,7 @@ describe('annotator/util/shadow-root', () => {
it('does not inject stylesheets into the shadow root if style is not found', () => {
const link = document.querySelector(
'link[rel="stylesheet"][href*="/build/styles/annotator.css"]'
'link[rel="preload"][href*="/build/styles/annotator.css"]'
);
// Removing the `rel` attribute is enough for the URL to not be found
link.removeAttribute('rel');
......
......@@ -120,6 +120,14 @@ function preloadUrl(doc, type, url) {
doc.head.appendChild(link);
}
/**
* @param {SidebarAppConfig|AnnotatorConfig} config
* @param {string} path
*/
function assetURL(config, path) {
return config.assetRoot + 'build/' + config.manifest[path];
}
/**
* @param {Document} doc
* @param {SidebarAppConfig|AnnotatorConfig} config
......@@ -129,7 +137,7 @@ function preloadUrl(doc, type, url) {
*/
function injectAssets(doc, config, assets, { forceModuleReload } = {}) {
assets.forEach(path => {
const url = config.assetRoot + 'build/' + config.manifest[path];
const url = assetURL(config, path);
if (url.match(/\.css/)) {
injectStylesheet(doc, url);
} else {
......@@ -169,6 +177,9 @@ export function bootHypothesisClient(doc, config) {
// Register the URL of the notebook app which the Hypothesis client should load.
injectLink(doc, 'notebook', 'html', config.notebookAppUrl);
// Preload the styles used by the shadow roots of annotator UI elements.
preloadUrl(doc, 'style', assetURL(config, 'styles/annotator.css'));
// Register the URL of the annotation client which is currently being used to drive
// annotation interactions.
injectLink(
......@@ -189,7 +200,7 @@ export function bootHypothesisClient(doc, config) {
'scripts/annotator.bundle.js',
'styles/annotator.css',
'styles/highlights.css',
'styles/pdfjs-overrides.css',
],
......
......@@ -35,6 +35,7 @@ describe('bootstrap', () => {
// Annotation layer
'scripts/annotator.bundle.js',
'styles/annotator.css',
'styles/highlights.css',
'styles/pdfjs-overrides.css',
// Sidebar app
......@@ -103,13 +104,31 @@ describe('bootstrap', () => {
runBoot('annotator');
const expectedAssets = [
'scripts/annotator.bundle.1234.js#ts=123',
'styles/annotator.1234.css',
'styles/highlights.1234.css',
'styles/pdfjs-overrides.1234.css',
].map(assetUrl);
assert.deepEqual(findAssets(iframe.contentDocument), expectedAssets);
});
it('preloads assets used wihin shadow roots in the annotation layer', () => {
runBoot('annotator');
const preloadLinks = [
...iframe.contentDocument.querySelectorAll('link[rel=preload]'),
];
preloadLinks.sort((a, b) => a.href.localeCompare(b.href));
assert.equal(preloadLinks.length, 1);
assert.equal(
preloadLinks[0].href,
'https://marginal.ly/client/build/styles/annotator.1234.css'
);
assert.equal(preloadLinks[0].as, 'style');
assert.equal(preloadLinks[0].crossOrigin, 'anonymous');
});
it('creates the link to the sidebar iframe', () => {
runBoot('annotator');
......
......@@ -16,7 +16,6 @@
@use './components/NotebookModal';
@use './components/Toolbar';
@use './components/WarningBanner';
@use './highlights';
// Sidebar
.annotator-frame {
......
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