fix(@angular-devkit/build-angular): handle handle load event for multiple stylesheets and CSP nonces

The `load` event for each stylesheet may not always be triggered by Google Chrome's handling. Refer to: https://crbug.com/1521256

This results in the media attribute persistently being set to print, leading to distorted styles in the UI. To address this issue, we substitute the onload logic by replacing `link.addEventListener('load', ...` with `document.documentElement.addEventListener('load', ...` and filtering for link tags.

Closes #26932
This commit is contained in:
Alan Agius 2024-01-24 13:00:25 +00:00 committed by Alan Agius
parent f49b2363db
commit c93ea15278

View File

@ -21,23 +21,34 @@ const CSP_MEDIA_ATTR = 'ngCspMedia';
/** /**
* Script text used to change the media value of the link tags. * Script text used to change the media value of the link tags.
*
* NOTE:
* We do not use `document.querySelectorAll('link').forEach((s) => s.addEventListener('load', ...)`
* because this does not always fire on Chome.
* See: https://github.com/angular/angular-cli/issues/26932 and https://crbug.com/1521256
*/ */
const LINK_LOAD_SCRIPT_CONTENT = [ const LINK_LOAD_SCRIPT_CONTENT = [
`(() => {`, '(() => {',
// Save the `children` in a variable since they're a live DOM node collection. ` const CSP_MEDIA_ATTR = '${CSP_MEDIA_ATTR}';`,
// We iterate over the direct descendants, instead of going through a `querySelectorAll`, ' const documentElement = document.documentElement;',
// because we know that the tags will be directly inside the `head`. ' const listener = (e) => {',
` const children = document.head.children;`, ' const target = e.target;',
// Declare `onLoad` outside the loop to avoid leaking memory. ` if (!target || target.tagName !== 'LINK' || !target.hasAttribute(CSP_MEDIA_ATTR)) {`,
// Can't be an arrow function, because we need `this` to refer to the DOM node. ' return;',
` function onLoad() {this.media = this.getAttribute('${CSP_MEDIA_ATTR}');}`, ' }',
// Has to use a plain for loop, because some browsers don't support
// `forEach` on `children` which is a `HTMLCollection`. ' target.media = target.getAttribute(CSP_MEDIA_ATTR);',
` for (let i = 0; i < children.length; i++) {`, ' target.removeAttribute(CSP_MEDIA_ATTR);',
` const child = children[i];`,
` child.hasAttribute('${CSP_MEDIA_ATTR}') && child.addEventListener('load', onLoad);`, // Remove onload listener when there are no longer styles that need to be loaded.
` }`, ' if (!document.head.querySelector(`link[${CSP_MEDIA_ATTR}]`)) {',
`})();`, ` documentElement.removeEventListener('load', listener);`,
' }',
' };',
// We use an event with capturing (the true parameter) because load events don't bubble.
` documentElement.addEventListener('load', listener, true);`,
'})();',
].join('\n'); ].join('\n');
export interface InlineCriticalCssProcessOptions { export interface InlineCriticalCssProcessOptions {
@ -57,6 +68,7 @@ interface PartialHTMLElement {
hasAttribute(name: string): boolean; hasAttribute(name: string): boolean;
removeAttribute(name: string): void; removeAttribute(name: string): void;
appendChild(child: PartialHTMLElement): void; appendChild(child: PartialHTMLElement): void;
insertBefore(newNode: PartialHTMLElement, referenceNode?: PartialHTMLElement): void;
remove(): void; remove(): void;
name: string; name: string;
textContent: string; textContent: string;
@ -155,7 +167,7 @@ class CrittersExtended extends Critters {
// `addEventListener` to apply the media query instead. // `addEventListener` to apply the media query instead.
link.removeAttribute('onload'); link.removeAttribute('onload');
link.setAttribute(CSP_MEDIA_ATTR, crittersMedia[1]); link.setAttribute(CSP_MEDIA_ATTR, crittersMedia[1]);
this.conditionallyInsertCspLoadingScript(document, cspNonce); this.conditionallyInsertCspLoadingScript(document, cspNonce, link);
} }
// Ideally we would hook in at the time Critters inserts the `style` tags, but there isn't // Ideally we would hook in at the time Critters inserts the `style` tags, but there isn't
@ -195,7 +207,11 @@ class CrittersExtended extends Critters {
* Inserts the `script` tag that swaps the critical CSS at runtime, * Inserts the `script` tag that swaps the critical CSS at runtime,
* if one hasn't been inserted into the document already. * if one hasn't been inserted into the document already.
*/ */
private conditionallyInsertCspLoadingScript(document: PartialDocument, nonce: string): void { private conditionallyInsertCspLoadingScript(
document: PartialDocument,
nonce: string,
link: PartialHTMLElement,
): void {
if (this.addedCspScriptsDocuments.has(document)) { if (this.addedCspScriptsDocuments.has(document)) {
return; return;
} }
@ -203,9 +219,9 @@ class CrittersExtended extends Critters {
const script = document.createElement('script'); const script = document.createElement('script');
script.setAttribute('nonce', nonce); script.setAttribute('nonce', nonce);
script.textContent = LINK_LOAD_SCRIPT_CONTENT; script.textContent = LINK_LOAD_SCRIPT_CONTENT;
// Append the script to the head since it needs to // Prepend the script to the head since it needs to
// run as early as possible, after the `link` tags. // run as early as possible, before the `link` tags.
document.head.appendChild(script); document.head.insertBefore(script, link);
this.addedCspScriptsDocuments.add(document); this.addedCspScriptsDocuments.add(document);
} }
} }