fix(frontend): remove white flash on comments iframe load

on dark host pages the widget flashed an opaque white rectangle while loading.
the iframe element carries color-scheme from the theme param, but its document
had none until remark.tsx ran, and a mismatched color-scheme makes the embedded
canvas opaque instead of transparent. broken since #2023 added the element-side
color-scheme to fix a firefox dark-mode bug.

set the document's color-scheme from the theme param in an inline head script,
before first paint, using the same rule as create-iframe.ts. that closes the long
window but not the surface browsers paint before the document is parsed, which
webkit renders white and chromium hides behind paint holding. so also create the
iframe hidden and reveal it when the document posts inited, with a timeout
fallback so a failed bootstrap cannot leave the widget invisible.

the reveal lives in createIframe rather than embed.ts so the profile modal, the
other caller, gets it too. that modal focuses its iframe on open, and a hidden
element cannot take focus, so focus now fires from the reveal instead of a timer.

covered by a unit test for the reveal paths and the event.source guard, and by
e2e for the document's color-scheme and the iframe's visibility before inited,
after inited, and after the fallback.

.github/workflows/ci-backend.yml

@@ -7,6 +7,9 @@ on:
     paths:
       - ".github/workflows/ci-backend.yml"
       - "backend/**"
+      - "Dockerfile"
+      - "docker-init.sh"
+      - ".dockerignore"
       - "!backend/scripts/**"
       - "!**.md"
   pull_request:
@@ -14,6 +17,9 @@ on:
     paths:
       - ".github/workflows/ci-backend.yml"
       - "backend/**"
+      - "Dockerfile"
+      - "docker-init.sh"
+      - ".dockerignore"
       - "!backend/scripts/**"
       - "!**.md"
 

.github/workflows/docker.yml

@@ -2,7 +2,7 @@ name: docker
 
 on:
   workflow_run:
-    workflows: [backend]
+    workflows: [backend, frontend]
     types: [completed]
 
 concurrency:

frontend/apps/remark42/app/components/root/root.spec.tsx

@@ -0,0 +1,74 @@
+import '@testing-library/jest-dom';
+import { waitFor } from '@testing-library/preact';
+
+import { render } from 'tests/utils';
+import * as api from 'common/api';
+import * as postMessage from 'utils/post-message';
+import type { User } from 'common/types';
+import type { StoreState } from 'store';
+
+import { ConnectedRoot } from './root';
+
+const stateStub: Partial<StoreState> = {
+  comments: {
+    sort: '-active',
+    isFetching: false,
+    childComments: {},
+    topComments: [],
+    pinnedComments: [],
+    allComments: {},
+    activeComment: null,
+  },
+  collapsedThreads: {},
+  theme: 'light',
+  info: { url: 'test-url', count: 0, read_only: false },
+  hiddenUsers: {},
+  bannedUsers: [],
+  user: null,
+};
+
+describe('<ConnectedRoot />', () => {
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  it('reports iframe height only after the initial user fetch settles', async () => {
+    let resolveUser!: (user: User | null) => void;
+    jest.spyOn(api, 'getUser').mockImplementation(
+      () =>
+        new Promise((resolve) => {
+          resolveUser = resolve;
+        })
+    );
+    // keep comments loading so only the user fetch controls the first height report
+    jest.spyOn(api, 'getPostComments').mockImplementation(() => new Promise(() => undefined));
+    const updateIframeHeight = jest.spyOn(postMessage, 'updateIframeHeight').mockImplementation(() => undefined);
+
+    render(<ConnectedRoot />, stateStub);
+
+    // while the global preloader is shown, no height must be sent to the parent page,
+    // otherwise the parent shrinks the iframe to the preloader size and it blinks
+    expect(updateIframeHeight).not.toHaveBeenCalled();
+
+    resolveUser(null);
+
+    await waitFor(() => expect(updateIframeHeight).toHaveBeenCalled());
+  });
+
+  it('falls back to reporting iframe height when the user fetch hangs', () => {
+    jest.useFakeTimers();
+    try {
+      jest.spyOn(api, 'getUser').mockImplementation(() => new Promise(() => undefined));
+      jest.spyOn(api, 'getPostComments').mockImplementation(() => new Promise(() => undefined));
+      const updateIframeHeight = jest.spyOn(postMessage, 'updateIframeHeight').mockImplementation(() => undefined);
+
+      render(<ConnectedRoot />, stateStub);
+      expect(updateIframeHeight).not.toHaveBeenCalled();
+
+      jest.advanceTimersByTime(5000);
+      expect(updateIframeHeight).toHaveBeenCalled();
+    } finally {
+      jest.useRealTimers();
+    }
+  });
+});

frontend/apps/remark42/app/components/root/root.tsx

@@ -1,5 +1,4 @@
 import { h, Component, Fragment } from 'preact';
-import { useEffect } from 'preact/hooks';
 import { useSelector } from 'react-redux';
 import { IntlShape, useIntl, FormattedMessage, defineMessages } from 'react-intl';
 import clsx from 'clsx';
@@ -112,8 +111,29 @@ export class Root extends Component<Props, State> {
     isSettingsVisible: false,
   };
 
+  heightObserver: ResizeObserver | null = null;
+  heightFallbackTimeout: number | null = null;
+
+  startHeightReporting = () => {
+    if (this.heightObserver) {
+      return;
+    }
+
+    updateIframeHeight();
+    this.heightObserver = new ResizeObserver(() => updateIframeHeight());
+    this.heightObserver.observe(document.body);
+  };
+
   componentDidMount() {
-    const userloading = this.props.fetchUser().finally(() => this.setState({ isUserLoading: false }));
+    // if the user fetch hangs, report the preloader height anyway so the parent page
+    // is not left with an unbounded iframe
+    this.heightFallbackTimeout = window.setTimeout(this.startHeightReporting, 5000);
+
+    const userloading = this.props.fetchUser().finally(() =>
+      // start reporting iframe height only after the global preloader is replaced with
+      // real content, so the parent page never shrinks the iframe to the preloader size
+      this.setState({ isUserLoading: false }, this.startHeightReporting)
+    );
 
     Promise.all([userloading, this.props.fetchComments()]).finally(() => {
       setTimeout(this.checkUrlHash);
@@ -123,6 +143,13 @@ export class Root extends Component<Props, State> {
     window.addEventListener('message', this.onMessage);
   }
 
+  componentWillUnmount() {
+    if (this.heightFallbackTimeout !== null) {
+      window.clearTimeout(this.heightFallbackTimeout);
+    }
+    this.heightObserver?.disconnect();
+  }
+
   checkUrlHash = (e: Event & { newURL: string }) => {
     const hash = e ? `#${e.newURL.split('#')[1]}` : window.location.hash;
 
@@ -325,14 +352,6 @@ export function ConnectedRoot() {
   const props = useSelector(mapStateToProps);
   const actions = useActions(boundActions);
 
-  useEffect(() => {
-    const observer = new ResizeObserver(() => updateIframeHeight());
-
-    updateIframeHeight();
-    observer.observe(document.body);
-    return () => observer.disconnect();
-  }, []);
-
   return (
     <div className={clsx(styles.root, props.theme === 'dark' ? styles.themeDark : styles.themeLight, props.theme)}>
       <Root {...props} {...actions} intl={intl} />

frontend/apps/remark42/app/profile.ts

@@ -22,7 +22,15 @@ function createElement<K extends keyof HTMLElementTagNameMap>(
 
 function createFragment(params: Profile & Record<string, string | unknown>) {
   removeIframe();
-  iframe = createIframe({ ...params, page: 'profile', styles: styles.iframe });
+  // the iframe is hidden until it reveals itself, and a hidden element cannot take
+  // focus, so focus from the reveal rather than on a timer
+  const created: HTMLIFrameElement = createIframe({
+    ...params,
+    page: 'profile',
+    styles: styles.iframe,
+    onReveal: () => created.isConnected && created.focus(),
+  });
+  iframe = created;
 
   if (!root) {
     root = createElement('div', styles.root);
@@ -31,7 +39,6 @@ function createFragment(params: Profile & Record<string, string | unknown>) {
 
   root.appendChild(iframe);
   setStyles(root, styles.rootShown);
-  setTimeout(() => iframe?.focus());
 }
 
 function animateAppear(): void {

frontend/apps/remark42/app/utils/create-iframe.test.ts

@@ -0,0 +1,95 @@
+import { createIframe } from './create-iframe';
+
+const REVEAL_TIMEOUT = 5000;
+
+function postInited(source: MessageEventSource | null) {
+  window.dispatchEvent(new MessageEvent('message', { data: { inited: true }, source }));
+}
+
+describe('createIframe', () => {
+  beforeEach(() => {
+    jest.useFakeTimers();
+    document.body.innerHTML = '';
+  });
+
+  afterEach(() => {
+    jest.useRealTimers();
+  });
+
+  it('starts hidden', () => {
+    const iframe = createIframe({ site_id: 'remark' });
+    expect(iframe.style.visibility).toBe('hidden');
+  });
+
+  it('lets caller styles override visibility', () => {
+    const iframe = createIframe({ site_id: 'remark', styles: { visibility: 'visible' } });
+    expect(iframe.style.visibility).toBe('visible');
+  });
+
+  it('reveals when its own document reports inited', () => {
+    const iframe = createIframe({ site_id: 'remark' });
+    document.body.appendChild(iframe);
+
+    postInited(iframe.contentWindow);
+    expect(iframe.style.visibility).toBe('visible');
+  });
+
+  it('ignores inited from a foreign source', () => {
+    const iframe = createIframe({ site_id: 'remark' });
+    const other = document.createElement('iframe');
+    document.body.append(iframe, other);
+
+    postInited(other.contentWindow);
+    expect(iframe.style.visibility).toBe('hidden');
+
+    postInited(window);
+    expect(iframe.style.visibility).toBe('hidden');
+  });
+
+  it('reveals on timeout when inited never arrives', () => {
+    const iframe = createIframe({ site_id: 'remark' });
+    document.body.appendChild(iframe);
+
+    jest.advanceTimersByTime(REVEAL_TIMEOUT - 1);
+    expect(iframe.style.visibility).toBe('hidden');
+
+    jest.advanceTimersByTime(1);
+    expect(iframe.style.visibility).toBe('visible');
+  });
+
+  it('calls onReveal after the iframe becomes visible', () => {
+    const onReveal = jest.fn(() => {
+      expect(iframe.style.visibility).toBe('visible');
+    });
+    const iframe = createIframe({ site_id: 'remark', onReveal });
+    document.body.appendChild(iframe);
+
+    postInited(iframe.contentWindow);
+    expect(onReveal).toHaveBeenCalledTimes(1);
+  });
+
+  it('calls onReveal on the timeout path too', () => {
+    const onReveal = jest.fn();
+    createIframe({ site_id: 'remark', onReveal });
+
+    jest.advanceTimersByTime(REVEAL_TIMEOUT);
+    expect(onReveal).toHaveBeenCalledTimes(1);
+  });
+
+  it('drops the listener and the timer on reveal', () => {
+    const onReveal = jest.fn();
+    const iframe = createIframe({ site_id: 'remark', onReveal });
+    document.body.appendChild(iframe);
+
+    postInited(iframe.contentWindow);
+    postInited(iframe.contentWindow);
+    jest.advanceTimersByTime(REVEAL_TIMEOUT);
+
+    expect(onReveal).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not put onReveal into the iframe query', () => {
+    const iframe = createIframe({ site_id: 'remark', onReveal: () => undefined });
+    expect(iframe.getAttribute('src')).not.toContain('onReveal');
+  });
+});

frontend/apps/remark42/app/utils/create-iframe.ts

@@ -1,9 +1,34 @@
 import { BASE_URL } from 'common/constants.config';
+import { parseMessage } from 'utils/post-message';
 import { setStyles, setAttributes, StylesDeclaration } from 'utils/set-dom-props';
 
-type Params = { [key: string]: unknown; __colors__?: Record<string, string>; styles?: StylesDeclaration };
+type Params = {
+  [key: string]: unknown;
+  __colors__?: Record<string, string>;
+  styles?: StylesDeclaration;
+  onReveal?: () => void;
+};
 
-export function createIframe({ __colors__, styles, ...params }: Params) {
+/**
+ * How long to wait for the iframe document to report itself as inited before
+ * showing it anyway. Only reached when the document fails to bootstrap.
+ */
+const REVEAL_TIMEOUT = 5000;
+
+/**
+ * Creates the remark42 iframe for the given params.
+ *
+ * The returned iframe starts hidden and becomes visible once its document posts
+ * `inited`, or after REVEAL_TIMEOUT if that never arrives. Until then it holds a
+ * global `message` listener and a timer, both dropped on the first reveal.
+ *
+ * `onReveal` runs right after the iframe becomes visible. Anything that needs a
+ * visible iframe, such as focus(), belongs there rather than on a timer.
+ *
+ * `styles` is applied last, so a caller passing `visibility` overrides the hiding
+ * and brings the white flash back.
+ */
+export function createIframe({ __colors__, styles, onReveal, ...params }: Params) {
   const iframe = document.createElement('iframe');
   const query = new URLSearchParams(params as Record<string, string>).toString();
 
@@ -21,8 +46,35 @@ export function createIframe({ __colors__, styles, ...params }: Params) {
     margin: 0,
     overflow: 'hidden',
     colorScheme: params.theme === 'dark' ? 'dark' : 'light',
+    // an iframe whose color-scheme differs from its document's gets an opaque canvas
+    // painted in the document's scheme, and browsers paint a default surface before
+    // the document is parsed at all, which shows as a white flash on dark host pages.
+    // reveal only on `inited`, which the document posts from its body script; the head
+    // script has applied the matching scheme by then, so the two always agree on reveal.
+    visibility: 'hidden',
     ...styles,
   });
 
+  hideUntilInited(iframe, onReveal);
+
   return iframe;
 }
+
+function hideUntilInited(iframe: HTMLIFrameElement, onReveal?: () => void) {
+  function reveal() {
+    window.removeEventListener('message', handleMessage);
+    window.clearTimeout(timeout);
+    iframe.style.visibility = 'visible';
+    onReveal?.();
+  }
+
+  function handleMessage(event: MessageEvent) {
+    if (event.source === iframe.contentWindow && parseMessage(event).inited === true) {
+      reveal();
+    }
+  }
+
+  const timeout = window.setTimeout(reveal, REVEAL_TIMEOUT);
+
+  window.addEventListener('message', handleMessage);
+}

frontend/apps/remark42/app/utils/post-message.ts

@@ -1,6 +1,7 @@
 import type { Theme, Profile } from 'common/types';
 
 type ParentMessage = {
+  /** the iframe document bootstrapped: drives placeholder removal and iframe reveal */
   inited?: true;
   scrollTo?: number;
   height?: number;

frontend/apps/remark42/templates/iframe.ejs

@@ -10,6 +10,25 @@
       if (window.location.search === '?selfClose') {
         window.close();
       }
+      (function () {
+        // the parent sets color-scheme on the iframe element from the theme param. this document
+        // must match it before the first paint, otherwise the canvas is painted opaque in its own
+        // scheme instead of staying transparent, which flashes white on dark host pages.
+        try {
+          var pairs = window.location.search.substring(1).split('&');
+          var theme = '';
+
+          for (var i = 0; i < pairs.length; i++) {
+            var pair = pairs[i].split('=');
+            if (pair[0] === 'theme') {
+              theme = decodeURIComponent(pair[1] || '');
+              break;
+            }
+          }
+
+          document.documentElement.style.colorScheme = theme === 'dark' ? 'dark' : 'light';
+        } catch (e) {}
+      })();
       (function () {
         function isCSSVariablesSupported() {
           if (typeof window === `undefined`) {

frontend/e2e/tests/iframe-theme.spec.ts

@@ -0,0 +1,83 @@
+import { test, expect, type Page } from '@playwright/test'
+
+// the parent page sets color-scheme on the iframe element from the theme param. if the iframe
+// document does not carry the same color-scheme before its bundle runs, the canvas is painted
+// opaque white instead of staying transparent. block the bundle to freeze the document in that
+// pre-script state and assert the inline head script has already applied the scheme.
+test.describe('Iframe color scheme', () => {
+	test.beforeEach(async ({ page }) => {
+		await page.route(/remark\.m?js$/, (route) => route.abort())
+	})
+
+	const cases = [
+		{ name: 'dark theme', query: '?site_id=remark&theme=dark', expected: 'dark' },
+		{ name: 'light theme', query: '?site_id=remark&theme=light', expected: 'light' },
+		{ name: 'no theme falls back to light', query: '?site_id=remark', expected: 'light' },
+	]
+
+	for (const { name, query, expected } of cases) {
+		test(name, async ({ page }) => {
+			await page.goto(`/web/iframe.html${query}`)
+
+			const inline = await page.evaluate(() => document.documentElement.style.colorScheme)
+			expect(inline).toBe(expected)
+
+			const computed = await page.evaluate(() => getComputedStyle(document.documentElement).colorScheme)
+			expect(computed).toBe(expected)
+		})
+	}
+})
+
+// browsers paint a default surface for an iframe before its document is parsed, and that surface
+// is opaque when the element carries a color-scheme the document does not have yet. WebKit shows
+// it as a white flash on dark host pages. the parent keeps the iframe hidden until the document
+// reports itself inited, so the surface is never presented.
+test.describe('Iframe reveal', () => {
+	// REVEAL_TIMEOUT in app/utils/create-iframe.ts. the fallback timer starts when the
+	// iframe is created, during page load, so any assertion with a deadline at or past
+	// this value can be satisfied by the fallback alone and says nothing about the
+	// message path. bound the message-path assertions well under it.
+	const REVEAL_TIMEOUT = 5000
+	const MESSAGE_REVEAL_BUDGET = 1500
+
+	const visibility = (page: Page) =>
+		page.evaluate(() => {
+			const iframe = document.querySelector<HTMLIFrameElement>('#remark42 iframe')
+			return iframe ? iframe.style.visibility : 'no-iframe'
+		})
+
+	test('stays hidden until the document reports inited', async ({ page }) => {
+		await page.route(/\/web\/iframe\.html/, (route) => route.abort())
+		const start = Date.now()
+		await page.goto('/web/')
+		await page.waitForSelector('#remark42 iframe', { state: 'attached' })
+
+		expect(await visibility(page)).toBe('hidden')
+		// a slow run could have let the fallback fire, which would make the assertion
+		// above pass or fail for the wrong reason. fail loudly instead of flaking.
+		expect(Date.now() - start).toBeLessThan(REVEAL_TIMEOUT)
+	})
+
+	// must reveal from the inited message, not the fallback: a broken message listener would
+	// leave the widget invisible for 5s on every load. the fallback timer starts when the
+	// iframe is created, partway through goto(), so bounding only the poll leaves the
+	// navigation window unmeasured. time the whole thing.
+	test('is revealed by the inited message, well before the fallback', async ({ page }) => {
+		const start = Date.now()
+		await page.goto('/web/')
+
+		await expect.poll(() => visibility(page), { timeout: MESSAGE_REVEAL_BUDGET }).toBe('visible')
+		expect(Date.now() - start).toBeLessThan(REVEAL_TIMEOUT)
+		await expect(page.locator('#remark42 iframe')).toBeVisible()
+	})
+
+	// the aborted document never reports its height, so the iframe box stays empty and
+	// toBeVisible() would fail on geometry. assert the property the fallback actually sets.
+	test('is revealed by the timeout when inited never arrives', async ({ page }) => {
+		await page.route(/\/web\/iframe\.html/, (route) => route.abort())
+		await page.goto('/web/')
+		await page.waitForSelector('#remark42 iframe', { state: 'attached' })
+
+		await expect.poll(() => visibility(page), { timeout: REVEAL_TIMEOUT * 2 }).toBe('visible')
+	})
+})