Skip to content

read ownerDocument off the Node prototype - #163

Draft
myabc wants to merge 1 commit into
bigskysoftware:mainfrom
myabc:fix/owner-document-shadowing
Draft

read ownerDocument off the Node prototype#163
myabc wants to merge 1 commit into
bigskysoftware:mainfrom
myabc:fix/owner-document-shadowing

Conversation

@myabc

@myabc myabc commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Drive-by from a non-maintainer — take it or leave it, and I'm happy to close it if you'd rather fix this differently.

The bug

saveAndRestoreFocus reads ctx.target.ownerDocument to find the document that owns the focus. That read is shadowable.

<form> is the one element in HTML marked [LegacyOverrideBuiltIns] in the WebIDL: its named-property getter (which exposes controls by name/id) is installed ahead of the prototype chain rather than behind it. So <input name="ownerDocument"> inside a form makes form.ownerDocument return that HTMLInputElement, not the Document. This is the same class of bug that 8fba6e9 hardened the duck-typed is.* helpers against — <input name="nodeType"> shadowing Node.prototype.nodeType.

When the morph target is such a form, doc is an HTMLInputElement, doc.activeElement is undefined, is.inputElement(undefined) is false, and saveAndRestoreFocus early-returns. Focus and selection restoration silently stop happening — no throw, no warning.

Where it came from

74fc41b (in #156) changed this from the global document to ctx.target.ownerDocument. The intent is right — focus lives in the target's document, which may not be ours, and that's what makes the cross-realm iframe test pass. The property just happens to be shadowable, and the global it replaced was not.

A/B, container-move scenario with moveBefore disabled so the native move can't preserve focus on its own:

on 83246cb (current main):  form shadows ownerDocument   activeIsFocused: false
                            control (no shadowing)       activeIsFocused: true
on 8fba6e9 (before 74fc41b): form shadows ownerDocument  activeIsFocused: true
                            control                      activeIsFocused: true

8fba6e9 is the commit immediately before 74fc41b, so the regression is that commit and nothing older.

The fix

Read ownerDocument through Node.prototype's getter instead of off the instance. The descriptor is cached once at module scope, so there's no getOwnPropertyDescriptor call per morph.

The getter is also realm-safe — it brand-checks the internal slot, not the realm the node was created in — so it keeps working for nodes from an iframe document, which is what 74fc41b was after in the first place. I verified that directly against a node from an iframe realm, not just in the same-realm case.

Regression test

test/restore-focus.js gets one test modelled on the existing IDed-container move tests. It disables moveBefore on the parents first — without that the native move preserves focus by itself and the test passes unfixed, which would make it worthless.

It genuinely catches the bug:

  • on unmodified 83246cb: 214 passed, 1 failedAssertionError: expected false to equal true
  • with the fix: 214 passed, 0 failed

Verification

npm run typecheck                      # silent
npm run format:check                   # All matched files use Prettier code style!
npm run test:chrome                    # 214 passed, 0 failed, 5 skipped; Code coverage: 100 %
node test/lib/ensure-full-coverage.js  # exit 0

Coverage stays at 100 %. dist/ is untouched — this doesn't need a rebuild.

Note

There are a few more document.activeElement reads (morphNode, ignoreAttribute, ignoreValueOfActiveElement, createActiveElementAndParents) that still resolve against the host document and are therefore inert cross-realm, plus a related shadowing hazard in findIdElements. Those are a separate change and I'd rather not pile them onto this one. Happy to open a follow-up if this direction looks right to you.

`saveAndRestoreFocus` resolves the document that owns the focus by
reading `ctx.target.ownerDocument`. `<form>` is [LegacyOverrideBuiltIns],
so a named control such as `<input name="ownerDocument">` installs an own
property on the form that shadows `Node.prototype.ownerDocument`.

When the morph target is such a form, `doc` is an `HTMLInputElement`,
`doc.activeElement` is `undefined`, and the function early-returns, so
focus and selection restoration silently stop happening.

Add an `ownerDocumentOf` helper that reads through the prototype getter,
caching the descriptor once at module scope. The getter is also
realm-safe: it brand-checks the internal slot rather than the realm.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant