fix(core): stop the browser spellchecking code blocks and inline code - #253
Merged
Conversation
The canvas offered exactly one answer for the whole document -- `spellcheck` on `<openleaf-editor>` -- so an author who wanted their prose checked also got a red squiggle under every identifier. Source view's textarea was the only place in the editor where code was left alone. `codeSpellcheckPlugin()` decorates code blocks and inline `<code>` with `spellcheck="false"`, unconditionally: the host attribute is an answer about prose, and code is not prose whichever way it was set. It is a decoration rather than an attribute in the schema's `toDOM`, because `serializeHtml` serializes with `DOMSerializer.fromSchema` -- the same `toDOM` that renders the canvas writes the stored HTML, so the obvious one-line fix would have put editor chrome into every saved document and every fidelity comparison. A test asserts the attribute never reaches `value`. `changedRange` and the top-level widening are now shared with the visual-aids plugin rather than copied, and `changedRange` learned to read a step whose `StepMap` is empty. Adding or removing a mark moves no position, so toggling `<code>` on a word looked like a transaction that had changed nothing and the decoration did not appear until the author typed in the same block. WebKit reads `spellcheck` from the editing host rather than per element, so Safari and iOS are unchanged. The docs say so. Signed-off-by: Peyton Nowlin <peytonn98@googlemail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 070b02b793
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex caught a regression the first commit introduced and, underneath it, a defect that was already there. `changedRange` learning to read mark steps meant visual aids now rebuild on a transaction that previously returned early. That exposed the same `DecorationSet.find` hazard the spellcheck plugin already guarded against: `find` reports a decoration that merely *touches* the queried range, so the empty-paragraph aid on the block after the widened range was removed as stale and the rebuild -- which scans only that range -- never put it back. Toggling a mark in the paragraph before an empty one made its aid disappear. The pre-existing half is the same shape without the mark step: typing in the paragraph after an empty one already dropped that aid on `main`. Rather than copy the wholly-inside filter into the second plugin, the whole map-widen-remove-rebuild dance moves into `rebuildChanged`, which both decoration plugins in core now call. Both failure modes have a test; each fails without the filter. Also bumps the demo's core bundle claim from 124 KB to 125 KB. The new plugin put the measured size far enough past the claim that the docs gate's 1% survival band no longer covered it -- CI measured 125.3 KB against a 124 KB claim. Signed-off-by: Peyton Nowlin <peytonn98@googlemail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
The canvas offered exactly one answer for the whole document —
spellcheckon<openleaf-editor>, copied onto the editable region. So an author who wantedtheir prose checked also got a red squiggle under every
getElementById.Source view's textarea (
packages/element/src/index.ts,area.spellcheck = false) was the only place in the editor where code was left alone.The fix
codeSpellcheckPlugin()decorates code blocks and inline<code>withspellcheck="false", and does it unconditionally. The host attribute isthe author's answer about their prose; code is not prose whichever way they
answered, so this is not tied to it.
Why a decoration and not
toDOMThe obvious fix is
['pre', { spellcheck: 'false' }, …]in the schema. That iswrong:
serializeHtmlserializes withDOMSerializer.fromSchema, so the sametoDOMthat renders the canvas writes the stored HTML. A schema fix wouldput editor chrome into every saved document and into every fidelity comparison.
Two tests pin this — one on
serializeHtml, one on the element'svalue.Two decoration-bookkeeping bugs fixed along the way
DecorationSet.findreports decorations that merely touch the queriedrange. The neighbouring block is the common case: a code block ending
exactly where the rebuilt range starts, or an empty paragraph starting
exactly where it ends. Either was removed as stale and never rebuilt, because
nodesBetweendoes not revisit a block the range only abuts. The attribute —or, in visual aids, the empty-block guide — fell off the block next to the
one being edited. This half was already live on
mainfor visual aids:<p></p><p>hi</p>, type in the second paragraph, and theol-empty-blockdecoration disappears.
changedRangecould not see a mark step. Adding or removing a mark movesno position, so its
StepMapis empty and reading the maps alone madetoggling
<code>on a word look like a transaction that had changed nothing— the decoration did not appear until the author typed in the same block. It
now folds in the range of a step whose map is the identity.
Those two interact, and the interaction was a regression in the first commit of
this PR, caught by Codex:
once mark steps reach the rebuild, toggling a mark in the paragraph before an
empty one made that paragraph's aid vanish. Fixed in
5a8cc1c.Refactor included
The whole map → widen → remove → rebuild dance now lives in
rebuildChanged(
packages/core/src/decoration-range.ts), called by both decoration plugins incore, rather than hand-written once per plugin.
changedRangeand thewiden-to-top-level helper moved out of
visual-aids.tsinto the same module.Hoisting rather than copying the wholly-inside filter is deliberate: a copy
leaves the same hazard waiting for the third plugin.
What this does not fix
WebKit applies
spellcheckfrom the editing host rather than per element, soSafari and every iOS browser still underline code. The only workaround is a
node view that takes the text out of the editable tree, which costs far more
than it buys. This is stated in the source, in the changelog, and in all four
user-facing docs.
Documentation
Updated in the same branch, per AGENTS.md:
README.md— the Translations bullet, which is where thespellcheckattribute is described.
docs/api-reference.md— thespellcheckattribute row.docs/integrating-openleaf.md— the attributes paragraph.packages/element/README.md— the same paragraph in the package copy.packages/core/README.md— a new section next to the other "install thisplugin if you build the view yourself" sections.
CHANGELOG.md— two entries under## Unreleased→### Fixed, one per bug.demo/index.html— the core bundle badge, 124 KB → 125 KB. The new pluginput the measured size far enough past the claim that the docs gate's 1%
survival band no longer covered it (CI measured 125.3 KB against a 124 KB
claim, and failed). Budget headroom is unaffected: 125.2/127 KB locally.
node scripts/check-docs.mjspasses.Verification
pnpm verify:quick— everything passed, including the docs gate and thebundle budgets.
pnpm test— 1762 passed, 92 files.pnpm test:e2e— 1047 passed across all three engines. Two failures,both pre-existing on
mainand unrelated (verified by stashing thisbranch and re-running them against a clean tree; the firefox one is also why
main's own CI is currently red):[firefox] placeholder.spec.ts—getComputedStyle(el, '::before').contentreturns the unresolved
attr(data-placeholder)in Firefox rather than thesubstituted string, so the assertion never matches. Test-side.
[webkit] demo.spec.ts › serves a promo video the browser can actually decode— the promo asset is H.264-only and Playwright's WebKit build hasno proprietary codecs, so
loadedmetadatanever fires.Each of the three new regression tests fails if the fix it covers is reverted;
I checked by removing the filter and re-running.
No new dependencies.
🤖 Generated with Claude Code