Skip to content

Fix composer crash when a list_item has no list around it (MAILSPRING-CLIENT-EV) - #2813

Open
bengotow wants to merge 3 commits into
masterfrom
claude/awesome-ritchie-za95zx
Open

Fix composer crash when a list_item has no list around it (MAILSPRING-CLIENT-EV)#2813
bengotow wants to merge 3 commits into
masterfrom
claude/awesome-ritchie-za95zx

Conversation

@bengotow

Copy link
Copy Markdown
Collaborator

Fixes MAILSPRING-CLIENT-EVError: Cannot read properties of null (reading 'type') in Commands$2.splitNodeByPath.

What the Sentry report shows

The stack is entirely inside vendored Slate, with no app frames:

slate.js:11520:7  Editor.withoutNormalizing
slate.js:8142:14                                <- the frame that matters
slate.js:11357:31 Editor.method [as splitNodeByPath]
slate.js:11236:12 Editor.command
slate.js:11464:14 Editor.run
slate.js:11430:22 next
slate.js:5801:20  onCommand
slate-react.js:5385:55 Editor.command
slate.js:11229:14 Editor.command
slate.js:8022:18  Commands$2.splitNodeByPath

Mapping those line numbers against the pinned builds (slate @ cd6f40e8, slate-react @ 0.45.1-react) pins them down exactly:

  • slate.js:8022:18 is type: node.type in Commands.splitNodeByPath, right after var node = document.getDescendant(path).
  • slate.js:8142:14 is editor.splitNodeByPath(parentPath, index) — the "middle child" branch inside Commands.unwrapNodeByPath's withoutNormalizing block.

So: unwrapNodeByPath was called on a node whose parentPath does not resolve to a descendant.

Root cause

getDescendant returns null for the empty root path by definition (if (!path || !path.size) return null). parentPath is PathUtils.lift(path), so the only way to hit that is a path with a single element — a node that is a direct child of the document.

The node in question is a list_item. Email HTML regularly contains an <li> with no <ul>/<ol> around it, and our HTML deserializer maps <li>list_item regardless of its parent, so an orphaned list_item can end up as a top-level block.

From there:

  1. slate-edit-list's getCurrentItem() answers "is the cursor in a list?" by returning any parent block whose type is list_item — it never checks that the item is actually inside a list.
  2. onEnter (in an empty item) and onBackspace (at its start) therefore call unwrapList(), which calls editor.unwrapNodeByKey(item.key).
  3. unwrapNodeByPath computes parentPath = lift([N]) = []. document.assertNode([]) succeedsNode#getNode returns this for an empty path — so nothing asserts. parentIndex is undefined, isFirst/isLast are computed against the document's children.
  4. When the orphan is neither the first nor the last child of the document, the else branch runs splitNodeByPath([], index)getDescendant([])nullnode.type throws.

The document should never hold that shape: slate-edit-list's schema declares a list parent for list_item, and normalization rewrites an orphan to a div. The reason it survives is a second bug:

Editor#withoutNormalizing sets tmp.normalize = false, runs the callback, then restores the flag — without a try/finally:

withoutNormalizing(fn) {
  var value = this.tmp.normalize;
  this.tmp.normalize = false;
  fn(controller);              // if this throws...
  this.tmp.normalize = value;  // ...this never runs
  normalizeDirtyPaths(this);
}

Almost every structural command runs inside withoutNormalizing, and exceptions thrown from a React event handler don't unmount anything — we report them to Sentry and the composer stays open. So after the first Slate error of a session, that editor silently stops normalizing, and invalid structures accumulate instead of being repaired. That is what lets an orphaned list_item reach the keyboard handlers.

Reproduction

Confirmed against the pinned dependency builds:

raw      orphan li middle + Enter      CRASH: Cannot read properties of null (reading 'type')
guarded  orphan li middle + Enter      passed to next() -> doc(div("a"),list_item(div("")),div("b"))

and the full chain, starting from a healthy editor:

tmp.normalize before: true
caught: simulated slate crash inside a command
tmp.normalize after : false
after paste: doc(div("hello","pasted"),list_item(div("stray")),div("tail"),div("world"))
orphan list_item survived? true
Enter CRASH: Cannot read properties of null (reading 'type')

(Without the leaked flag, the same paste is normalized to div("stray") and the crash is unreachable.)

The fix

base-block-plugins.tsx — wrap EditListPlugin so key events fall through to the default handling whenever the current item is not really inside a list (getCurrentItem() finds an item but getCurrentList() does not). The orphan then behaves like any other block.

patch-slate-normalizing.ts (new) — restore tmp.normalize when the withoutNormalizing callback throws, so one caught Slate error can no longer disable document repair for the rest of the session. This is the enabling condition for this crash and very likely for other structural Slate errors in the same Sentry project.

Testing

New spec app/spec/components/composer-editor/base-block-plugins-spec.tsx covers both. Every assertion was also run directly against the real pinned slate / @bengotow/slate-edit-list builds:

  • Enter / Backspace / Tab in an orphaned list_item no longer throw and fall through to the next handler, leaving the document untouched.
  • An orphaned list_item that is the document's only node no longer throws (it hit a different Slate assertion before).
  • Real list behaviour is byte-identical before and after the guard: Enter in an empty item still exits the list, Enter mid-text still splits the item, Backspace at item start still unwraps, Tab still indents, and non-list blocks are untouched.
  • withoutNormalizing restores the flag after a throw, including when nested; the non-throwing path is unchanged.

Generated by Claude Code

…-CLIENT-EV)

Slate throws "Cannot read properties of null (reading 'type')" from
Commands.splitNodeByPath when the composer contains a `list_item` block that is a
direct child of the document — an `<li>` deserialized from email HTML with no
`<ul>`/`<ol>` around it.

slate-edit-list's `getCurrentItem()` answers "is the cursor in a list?" by
returning any parent block whose type is `list_item`, without checking that it is
actually inside a list. So Enter (in an empty item) and Backspace (at its start)
route into `unwrapList()`, which calls `editor.unwrapNodeByKey(item.key)`. The
orphan's path has a single element, so `unwrapNodeByPath` lifts it to the empty
root path; `document.assertNode([])` accepts that (Node#getNode returns the
document itself for an empty path) so nothing asserts, and when the orphan is
neither the first nor the last child of the document the "middle child" branch
calls `splitNodeByPath([], index)`. There `document.getDescendant([])` returns
null by definition, and reading `node.type` throws — matching the reported frames
exactly (slate.js:8142 -> 8022).

Wrap EditListPlugin so key events fall through to the default handling whenever
the current item is not really inside a list.

The document should never hold that shape: slate-edit-list's schema declares a
list parent for `list_item` and normalization rewrites an orphan to a `div`. But
`Editor#withoutNormalizing` sets `tmp.normalize = false`, runs the callback and
restores the flag *without* a try/finally, so any command that throws leaves
normalization disabled for the rest of that editor's life. Exceptions from React
event handlers don't unmount anything, so after the first Slate error the composer
stays open and stops repairing its document — which is what lets the orphan
survive. Patch `withoutNormalizing` to restore the flag on the way out so a single
error can't cascade.

Verified against the pinned slate (cd6f40e8) and @bengotow/slate-edit-list
(b868e108) builds: the unguarded plugin reproduces the exact stack, the guarded
one falls through, and every in-list behaviour (split item, exit list, indent,
backspace-unwrap) is byte-identical before and after.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QfnK1t6VeRn37exPfshj7P
@indent-staging

indent-staging Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Warning

Indent Zero is shutting down on August 7th. Please migrate over to Indent 2.0 to continue getting PR reviews.

PR Summary

Fixes MAILSPRING-CLIENT-EV — a composer crash triggered when email HTML contains an <li> with no surrounding <ul>/<ol>. The PR adds a keydown-time guard around slate-edit-list so it can't call unwrapList on an orphaned list_item (whose one-element path lifts to the empty root and makes splitNodeByPath read .type off null), and monkey-patches Slate's Editor#withoutNormalizing so a single command-time exception can't leave tmp.normalize = false and stop the document from ever repairing itself.

  • base-block-plugins.tsx: EditListPlugin is now { ...EditListPluginBase, onKeyDown } — the wrapper falls through to next() when getCurrentItem(value) && !getCurrentList(value) (i.e. the cursor sits in a list_item that isn't inside a real list); utils/changes/schema still resolve so all existing call sites are unchanged
  • patch-slate-normalizing.ts (new): wraps Editor.prototype.withoutNormalizing in a try/catch that captures this.tmp.normalize before the call and restores it (then re-throws) if the callback throws, preventing a single Slate error from cascading
  • conversion.tsx: side-effect imports ./patch-slate-normalizing alongside the existing ./patch-chrome-ime
  • base-block-plugins-spec.tsx (new): specs cover Enter/Backspace/Tab on an orphaned list_item, a solo orphan, empty-item still exits the list, mid-text Enter still splits, and tmp.normalize is restored when withoutNormalizing's callback throws

Issues

All clear! No issues remaining. 🎉

2 issues already resolved
  • The spec's top-level import '../../../src/components/composer-editor/patch-slate-normalizing' isn't needed by the EditListPlugin tests (they build the editor with plugins: [] and never normalize) and it mutates Editor.prototype globally for the rest of the spec run — worth dropping to keep test isolation and only rely on it inside the Slate withoutNormalizing patch describe. (fixed by commit 5562004)
  • The wrapped EditListPlugin.onKeyDown now runs the startBlock/getCurrentItem/getCurrentList guard on every keystroke, whereas the base slate-edit-list plugin only cares about Enter/Tab/Backspace — cost is negligible, but an early-exit on event.key would match the base plugin's own dispatch and shrink the guard's surface. (fixed by commit 5562004)

CI Checks

All CI checks passed on 89df081.

Custom Rules 3 rules evaluated, 3 passed, 0 failed

Passing This is a longer title to see what happens when they are too long to fit
Passing B
Passing Ben Rule

View all rules

@indent

indent Bot commented Aug 20, 2026

Copy link
Copy Markdown
PR Summary

Fixes MAILSPRING-CLIENT-EV, a composer crash (Cannot read properties of null (reading 'type')) that happens when an orphaned list_item (an <li> deserialized from email HTML with no surrounding <ul>/<ol>) reaches the editor and the user presses Enter/Backspace/Tab in it. In that state slate-edit-list's handlers call the raw unwrapList, which lifts the orphan's one-element path to the empty root path and makes vendored Slate read .type off null. The change adds two defensive layers so the orphan is handled like a normal block and so a single Slate exception can no longer permanently disable normalization.

  • base-block-plugins.tsx: wrap EditList's onKeyDown so that when the cursor is in a list_item that is not inside a list (getCurrentItem truthy, getCurrentList null), the event falls through to default handling; valid-list behavior is unchanged and schema/normalizeNode/utils/changes are preserved via object spread.
  • patch-slate-normalizing.ts (new, imported from conversion.tsx): monkeypatch Editor#withoutNormalizing to restore tmp.normalize in a catch and rethrow, so a thrown command no longer leaves normalization disabled for the rest of the session.
  • Adds a spec covering the orphan Enter/Backspace/Tab cases, normal list split/exit behavior, and the normalization-restore-on-throw patch.

Issues

No issues found.

CI Checks

All CI checks passed on 89df081.

Comment thread app/spec/components/composer-editor/base-block-plugins-spec.tsx Outdated
Comment thread app/src/components/composer-editor/base-block-plugins.tsx
claude added 2 commits August 20, 2026 14:51
- Only import patch-slate-normalizing inside the describe block that needs it,
  instead of at module scope, so it doesn't mutate Editor.prototype for every
  other spec in the run.
- Early-exit EditListPlugin's onKeyDown on the same three keys slate-edit-list
  itself dispatches on (Enter/Tab/Backspace), so the guard's extra tree walks
  don't run on every keystroke.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QfnK1t6VeRn37exPfshj7P
The repo pins @types/jasmine 1.x, which doesn't declare `beforeAll` (a Jasmine 2.0
addition), so tsc failed with TS2304. A `describe` body runs once and synchronously
before its `it`s, so requiring the patch there keeps the same scoping without
needing a setup hook.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QfnK1t6VeRn37exPfshj7P
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.

2 participants