Observe document, not documentElement, in waitForDomIdle

[morisil]

The bug

waitForDomIdle installs its MutationObserver on document.documentElement. Right after a navigation commits there is a window in which the new document does not have a document element yet, so that argument is null and the page throws:

TypeError: Failed to execute 'observe' on 'MutationObserver': parameter 1 is not of type 'Node'

Because waitForDomIdle is one leg of waitUntilLoaded, this surfaces on the path where it hurts most — a click that navigates. Found in Umwelt, submitting a Google search form: the click went through and the browser landed on the results page, but the caller got an exception instead of a Navigation, and its recorded location stayed on the page the tab had already left.

The fix

Observe document instead. A Document is always a Node, and subtree: true from it covers everything documentElement would have, including a document element that appears — or is replaced — later. Strictly wider coverage, no null to guard.

Test

waitForDomIdle should report idle on a document with no document element removes the document element outright, standing in for that post-commit window (the real timing is a race and would make a flaky test; the precondition it produces is identical).

Verified it is a real regression test — with the old line restored it fails with the exact production error:

<test-failure test="...WaitUntilLoadedTest.waitForDomIdle should report idle on a document with no document element()" platform="jvm">
Error evaluating expression: ... TypeError: Failed to execute 'observe' on 'MutationObserver': parameter 1 is not of type 'Node'.
</test-failure>

Full WaitUntilLoadedTest suite green on jvmTest with the fix in place.

🤖 Generated with Claude Code

[claude]

Review: Observe document, not documentElement, in waitForDomIdle

Overview

Fixes a real production bug: right after a navigation commits, document.documentElement can transiently be null, so MutationObserver.observe(null, …) throws TypeError: parameter 1 is not of type 'Node'. Since waitForDomIdle is one leg of waitUntilLoaded, this could surface exactly on the click-that-navigates path — a bad failure mode, since the navigation itself succeeded but the caller got an exception instead of a Navigation. The fix switches the observed target to document (always a Node), with subtree: true giving strictly wider coverage than the old documentElement target, including elements that appear/are replaced later.

Code quality

• Minimal, surgical diff — exactly the line that needs to change, plus a doc comment explaining why, matching this repo's convention of only commenting non-obvious rationale.
• The new KDoc addition to waitForDomIdle clearly explains the transient window and why document is strictly wider than documentElement.

Test coverage

• Good instinct recognizing that the real timing window is a race and would make a flaky test, and instead engineering an equivalent, deterministic precondition (document.removeChild(document.documentElement)).
• Per the PR description, this was verified as a true regression test (fails with the old code, reproducing the exact production error) — the right TDD discipline for this repo.
• The test reuses the existing simple.html fixture and follows the file's established runInBrowser + // given / // when / // then pattern.

Minor suggestions (non-blocking)

• A brief comment noting that document.removeChild(document.documentElement) doesn't throw synchronously might help a reader unfamiliar with the DOM API — removing the root element is legal since Document can have zero children, but that's not obvious at a glance.
• Since the fix is "observe a strictly wider target," it may be worth confirming (not blocking) that observing document doesn't pick up mutations CDP wouldn't have cared about before — in practice this seems harmless since waitForDomIdle only cares about whether anything changed, not what changed.

Risk assessment

Low risk. The change is narrowly scoped to a single observe() call, backed by a doc explanation and a targeted regression test. No API surface changes, and no behavioral change for the common case (old and new targets converge once documentElement exists, since subtree: true already covered everything under it).

Nice fix — small, well-tested, with the root cause and trade-off documented right where a future reader will look for them.