Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.

fix(core): preventDefault on Enter at start of ATX heading - #240

Merged
Jocs merged 1 commit into
masterfrom
fix/atx-heading-enter-orphan-selection
May 21, 2026
Merged

fix(core): preventDefault on Enter at start of ATX heading#240
Jocs merged 1 commit into
masterfrom
fix/atx-heading-enter-orphan-selection

Conversation

@Jocs

@Jocs Jocs commented May 21, 2026

Copy link
Copy Markdown
Member

Summary

  • Pressing Enter before the leading # in an ATX heading inserts a new empty paragraph above the heading, but atxHeadingContent.enterHandler forgot to call event.preventDefault(). The browser's native Enter behavior then runs on the contenteditable, splitting/cloning the heading's <span class="mu-content"> into orphan nodes without BLOCK_DOM_PROPERTY. A subsequent click resolves to such an orphan and Selection.getSelection crashes at anchorBlock.path. (setextHeadingContent already calls preventDefault() + stopPropagation() in the same branch — this aligns ATX with that.)
  • Adds a defensive null-guard in Selection.getSelection so an unlinked DOM node returns null (the existing "no selection" outcome) instead of crashing if any other future path produces the same condition.
  • Adds a unit test that drives AtxHeadingContent.prototype.enterHandler with a structurally-typed fake this and asserts preventDefault is called for both offset 0 and offset 1 in an H1.

Reproduction (pre-fix)

  1. In the playground, place the caret before # in # 1. Headings.
  2. Press Enter.
  3. Click on the previous paragraph.
  4. Uncaught TypeError: Cannot read properties of undefined (reading 'path') at Selection.getSelection.

Test plan

  • pnpm --filter @muyajs/core test — 388/388 pass (new spec included)
  • pnpm lint:types — clean
  • Manual: run pnpm dev, reproduce the steps above, confirm no crash and the new empty paragraph is selectable

🤖 Generated with Claude Code

Pressing Enter before the leading `#` inserts an empty paragraph above
the heading. Without preventDefault the browser's native Enter handler
then runs on the contenteditable and clones/splits the heading's
`mu-content` span, producing orphan spans without BLOCK_DOM_PROPERTY.
A later click resolves to such an orphan and Selection.getSelection
crashes at `anchorBlock.path`. Mirror the fix already present in
setextHeadingContent and add a defensive null-check in getSelection.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 21, 2026 05:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a crash triggered by pressing Enter at the start of an ATX heading by (1) preventing the browser’s native Enter behavior in AtxHeadingContent.enterHandler and (2) making Selection.getSelection() resilient to DOM nodes that aren’t linked to a Muya block.

Changes:

  • Call event.preventDefault() + event.stopPropagation() when Enter inserts a paragraph above an ATX heading.
  • Add a null-guard in Selection.getSelection() when BLOCK_DOM_PROPERTY is missing (return null instead of throwing).
  • Add a unit test asserting preventDefault is called for offsets 0 and 1 in an H1 ATX heading.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/core/src/selection/index.ts Adds a defensive guard to avoid crashing when selection resolves to an unlinked DOM node.
packages/core/src/block/content/atxHeadingContent/index.ts Prevents native Enter behavior in the “insert paragraph above heading” branch (aligns with setext behavior).
packages/core/src/block/content/atxHeadingContent/tests/enterHandler.spec.ts Adds regression coverage ensuring preventDefault is called for Enter at the start of an ATX heading.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Jocs
Jocs merged commit 00939d2 into master May 21, 2026
6 of 7 checks passed
@Jocs
Jocs deleted the fix/atx-heading-enter-orphan-selection branch May 21, 2026 10:16
Jocs added a commit that referenced this pull request May 21, 2026
* test(e2e): skip 10k-paragraph perf budget on webkit and firefox

The 60s budget added in #238 was calibrated against chromium (~20s
observed locally) before the cross-browser matrix landed in #239.
WebKit on ubuntu-latest against the Vite dev server consistently runs
108-120s; firefox 62-68s. Both blow the budget on every run, and the
3-attempt retry cycle has been pushing the e2e job past its 15min cap
(#240 was cancelled by the GHA timeout for this reason).

Skip the perf assertion on non-chromium browsers as a short-term fix
so PR CI can stay green. The longer-term plan, already documented in
the file header, is to move @Perf tests to a nightly schedule against
a production bundle.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ci): exclude @Perf tests from PR-time e2e job

The per-browser skip added in the first iteration of this PR turned
out to be incomplete: chromium on GHA ubuntu-latest also overshoots
the 60s budget (~70s observed) once running against the Vite dev
server, not just the webkit ~115s and firefox ~65s seen in the prior
runs. All three browsers blow the budget; only the local M-class
macOS Chromium baseline (~20s) actually fits.

Switch to the project's already-documented plan (see file header):
exclude @perf-tagged tests from PR-time CI via --grep-invert. Future
@Perf tests automatically pick up the same treatment, and the test
file stays free of skip logic. The follow-up to move @Perf onto a
nightly schedule against a production bundle becomes a small
workflow addition rather than a rewrite.

Revert the per-browser test.skip in perf.spec.ts so the local
`pnpm e2e` invocation still runs the regression guard on all three
browsers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants