Skip to content

fix(keyboard): forward iframe keydown events to parent window#46

Open
chasel34 wants to merge 2 commits into
nexu-io:mainfrom
chasel34:fix/iframe-keyboard-forwarding
Open

fix(keyboard): forward iframe keydown events to parent window#46
chasel34 wants to merge 2 commits into
nexu-io:mainfrom
chasel34:fix/iframe-keyboard-forwarding

Conversation

@chasel34

Copy link
Copy Markdown
Contributor

Problem

Clicking inside a slide or preview iframe moves keyboard focus into the
iframe's own browsing context. The parent page's window.addEventListener("keydown", …)
never receives those events, so arrow-key navigation, the notes toggle (N),
and the fullscreen shortcut (F) all stop working as soon as the iframe is focused.

Fix

On each iframe's onLoad, attach a keydown listener to its contentWindow
that re-dispatches the event on the parent window. No manual cleanup needed:
both iframes are keyed (key=…) and fully remount on content changes,
destroying old listeners automatically.

src/lib/iframe-key.ts — shared helper isEditableTarget(target) guards
the forwarder against leaking keys typed into <input>, <textarea>, or
contenteditable controls inside a slide. It checks tagName and
isContentEditable directly rather than instanceof HTMLElement, which would
always return false across window realms.

Test plan

  • Click inside a deck slide iframe → arrow keys navigate slides, F enters
    fullscreen, N toggles notes.
  • Click inside the preview iframe → F enters fullscreen.
  • Type into an <input> inside a slide → no navigation or fullscreen side-effects.
  • pnpm build passes with no type errors.

🤖 Generated with Claude Code

@lefarcen lefarcen requested a review from Siri-Ray May 16, 2026 14:15
@lefarcen lefarcen added size/S Small change: 20-99 changed lines risk/medium Medium risk change type/bugfix Bug fix labels May 16, 2026
@PerishCode

Copy link
Copy Markdown
Contributor

Thanks for the PR. I checked a local rebase onto the latest main, but I am not going to push directly to this branch yet because the change has a behavior boundary that should be covered first.

What I found:

  • The rebase conflict is mostly mechanical: src/lib/iframe-key.ts needs to move to next/src/lib/iframe-key.ts, and the component imports land under next/src/components/....
  • The higher-risk part is the keyboard forwarding contract. The PR body still has the editable iframe input case unchecked, and that is the main guard preventing parent-level shortcuts from firing while the user types inside iframe content.

Suggested next step:

  • Add a focused Playwright regression under e2e/ui/ that covers both sides of the contract:
    • keydown inside the iframe forwards parent shortcuts such as F / arrow navigation
    • keydown inside an iframe input, textarea, or contenteditable element does not forward those shortcuts
  • Then rebase onto current main using the new next/ + e2e/ workspace shape.

Recommended validation:

  • pnpm exec tsx scripts/guard.ts
  • pnpm -F @html-anything/next typecheck
  • pnpm -F @html-anything/e2e typecheck
  • pnpm -F @html-anything/next build
  • pnpm -F @html-anything/e2e test -- <new-keyboard-test-file>

chasel34 and others added 2 commits May 18, 2026 21:59
When a slide or preview iframe receives focus, keydown events fire in
the iframe's own browsing context and never reach the parent window's
listener. This broke arrow-key navigation and the fullscreen shortcut
(f/F) in DeckViewer, and the fullscreen shortcut in PreviewPane.

Fix: on each iframe's onLoad, attach a keydown listener to its
contentWindow that re-dispatches the event on the parent window.

A shared isEditableTarget() helper (src/lib/iframe-key.ts) guards
against forwarding keys typed into INPUT/TEXTAREA/contentEditable
controls. The naive `instanceof HTMLElement` check is intentionally
avoided — it fails across window realms because the element's prototype
chain belongs to the iframe's HTMLElement, not the parent's.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ontract

Covers both sides of the isEditableTarget guard:
- ArrowRight / f from a plain element are forwarded to the parent window
- ArrowRight / f from an iframe <input> or contenteditable are blocked
- N from a deck iframe body shows the DeckViewer notes panel; N from an
  iframe <input> leaves the panel visible (guard blocks the toggle-back)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@chasel34 chasel34 force-pushed the fix/iframe-keyboard-forwarding branch from ba9be65 to af42f9e Compare May 18, 2026 14:24
@chasel34

Copy link
Copy Markdown
Contributor Author

Rebased onto main and added the Playwright regression.

Rebase — 9dfed8e

Git auto-detected the directory rename; all three files landed correctly:

  • src/lib/iframe-key.tsnext/src/lib/iframe-key.ts
  • patches to src/components/deck-viewer.tsx → applied to next/src/components/deck-viewer.tsx
  • patches to src/components/preview-pane.tsx → applied to next/src/components/preview-pane.tsx

The @/lib/iframe-key import alias works unchanged because next/tsconfig.json maps @/*./src/*.

Tests — af42f9e (e2e/ui/iframe-keyboard.test.ts, 6 cases)

Covers both sides of the isEditableTarget guard across three key bindings:

Key From plain element From <input> From contenteditable
ArrowRight (PreviewPane) ✅ forwarded ✅ blocked ✅ blocked
f (PreviewPane) ✅ forwarded ✅ blocked
n (DeckViewer) ✅ shows notes panel ✅ guard blocks toggle-back

One fixture correction discovered during the run: isDeck() on main matches <section class="slide">, not the old <section class="frame"> format from before the rebase. Updated accordingly.

Verification:

pnpm exec tsx scripts/guard.ts                                      → Guard passed.
pnpm -F @html-anything/next typecheck                               → ✓
pnpm -F @html-anything/e2e typecheck                                → ✓
pnpm -F @html-anything/next build                                   → ✓
pnpm -F @html-anything/e2e test -- e2e/ui/iframe-keyboard.test.ts  → 6 passed (14.4s)

@lefarcen lefarcen added size/M Medium change: 100-299 lines and removed size/S Small change: 20-99 changed lines labels May 18, 2026
@lefarcen

Copy link
Copy Markdown

Hey @Siri-Ray, quick review ping — @chasel34 rebased this onto main, added the e2e/ui/iframe-keyboard.test.ts regression for the iframe keyboard-forwarding boundary, and CI is green on the current head af42f9e. Could you take a look when you have a chance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/medium Medium risk change size/M Medium change: 100-299 lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants