fix: skip leafless subtrees when normalizing DOM points#2899
Draft
christianhg wants to merge 1 commit into
Draft
fix: skip leafless subtrees when normalizing DOM points#2899christianhg wants to merge 1 commit into
christianhg wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: bd7c064 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
📦 Bundle Stats —
|
| Metric | Value | vs main (59d03e2) |
|---|---|---|
| Internal (raw) | 799.3 KB | +208 B, +0.0% |
| Internal (gzip) | 152.7 KB | +34 B, +0.0% |
| Bundled (raw) | 1.41 MB | +210 B, +0.0% |
| Bundled (gzip) | 316.9 KB | +44 B, +0.0% |
| Import time | 99ms | -1ms, -1.4% |
@portabletext/editor/behaviors
| Metric | Value | vs main (59d03e2) |
|---|---|---|
| Internal (raw) | 467 B | - |
| Internal (gzip) | 207 B | - |
| Bundled (raw) | 424 B | - |
| Bundled (gzip) | 171 B | - |
| Import time | 2ms | -0ms, -2.3% |
@portabletext/editor/plugins
| Metric | Value | vs main (59d03e2) |
|---|---|---|
| Internal (raw) | 2.7 KB | - |
| Internal (gzip) | 894 B | - |
| Bundled (raw) | 2.5 KB | - |
| Bundled (gzip) | 827 B | - |
| Import time | 7ms | +0ms, +0.5% |
@portabletext/editor/selectors
| Metric | Value | vs main (59d03e2) |
|---|---|---|
| Internal (raw) | 80.1 KB | - |
| Internal (gzip) | 14.7 KB | - |
| Bundled (raw) | 75.5 KB | - |
| Bundled (gzip) | 13.5 KB | - |
| Import time | 8ms | +0ms, +1.5% |
@portabletext/editor/traversal
| Metric | Value | vs main (59d03e2) |
|---|---|---|
| Internal (raw) | 26.9 KB | - |
| Internal (gzip) | 5.3 KB | - |
| Bundled (raw) | 26.7 KB | - |
| Bundled (gzip) | 5.2 KB | - |
| Import time | 6ms | -0ms, -0.5% |
@portabletext/editor/utils
| Metric | Value | vs main (59d03e2) |
|---|---|---|
| Internal (raw) | 29.7 KB | - |
| Internal (gzip) | 6.2 KB | - |
| Bundled (raw) | 27.1 KB | - |
| Bundled (gzip) | 5.8 KB | - |
| Import time | 6ms | -0ms, -0.4% |
🗺️ . · ./behaviors · ./plugins · ./selectors · ./traversal · ./utils · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @portabletext/markdown
Compared against main (59d03e2e)
| Metric | Value | vs main (59d03e2) |
|---|---|---|
| Internal (raw) | 53.8 KB | - |
| Internal (gzip) | 9.8 KB | - |
| Bundled (raw) | 348.9 KB | - |
| Bundled (gzip) | 96.3 KB | - |
| Import time | 38ms | -1ms, -1.3% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
christianhg
force-pushed
the
fix-normalize-dom-point-leafless-subtrees
branch
from
447c0b4 to
093adf1
Compare
`getEditableChildAndIndex` treated any element with children as content, so a leafless subtree (a table's <colgroup> being the canonical case) became a dead-end: `normalizeDOMPoint` descended into it, the resulting point on a <col> resolved to nothing in `toSelectionPoint`, `suppressThrow` swallowed the failure, and the entire selection sync silently skipped, leaving the model stale while the screen showed e.g. a whole-table selection after a triple-click. The child-seeking loop now also skips elements whose subtree holds no editable content (no text and no rendered block/inline object), backtracking to a sibling exactly as it already does for `contenteditable="false"` elements. Zero-width leaves keep matching via their text content, and void objects keep matching via their rendered attributes, so void-adjacent positions are unaffected. Pinned by two tests on a container table rendered with a plain <colgroup>: an element-level range spanning the table maps to first-cell start -> last-cell end, and a collapsed element-level point before the colgroup maps into the first cell (both fail without the fix: one maps wrong, one maps to null).
christianhg
force-pushed
the
fix-normalize-dom-point-leafless-subtrees
branch
from
093adf1 to
bd7c064
Compare
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.
Triple-clicking a table in the playground shows a whole-table selection on screen while the editor's model keeps a stale collapsed caret, so Delete and typing act on something other than what's visibly selected. Clicking near the table's edges similarly parks carets that never map into the model (EDEX-1118 reported the same family from the deployed playground in June: typing after such a click injects content into the last cell, then below the table).
The diagnosis:
getEditableChildAndIndextreats any element with children as content. A table's<colgroup>has children (the<col>s) but no editable content, sonormalizeDOMPoint's descent walks into it and dead-ends: the resulting point on a<col>resolves to nothing intoSelectionPoint,suppressThrow: trueswallows the failure, and the wholeselectionchangesync silently skips, leaving the model stale. Any leafless subtree inside a container render triggers it;<colgroup>is just the canonical case every table renders.The fix extends the child-seeking skip condition: elements whose subtree holds no editable content (no text, no rendered block/inline object) are skipped with backtracking, exactly as
contenteditable="false"elements already are. The predicate is deliberately conservative: zero-width leaves keep matching through their text content and void objects through their rendered attributes, so void-adjacent positions behave exactly as before.Two tests pin it on a container table rendered with a plain
<colgroup>(no attribute tricks), both driven through the realselectionchangesync: an element-level range spanning the table must map to first-cell start to last-cell end, and a collapsed element-level point before the colgroup must map into the first cell. Both fail without the fix (one maps wrong, one maps tonull), verified red. Green on chromium, firefox, and webkit; the full chromium editor browser suite passes 1690/1690.Follows #2898 in the same subsystem: that one stopped the validator from destroying equivalent DOM selections, this one makes the mapping produce the right answer in the first place. The remaining piece of the cluster is EDEX-1512 (canonicalizing collapsed carets parked on structural elements).