Skip to content

fix(v2): keep the RTL caret on the boundary WebKit will not measure - #3963

Open
Nathaniel-260 wants to merge 6 commits into
superdoc:mainfrom
Nathaniel-260:fix/rtl-caret-webkit-collapsed-range
Open

fix(v2): keep the RTL caret on the boundary WebKit will not measure#3963
Nathaniel-260 wants to merge 6 commits into
superdoc:mainfrom
Nathaniel-260:fix/rtl-caret-webkit-collapsed-range

Conversation

@Nathaniel-260

@Nathaniel-260 Nathaniel-260 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #3943

The bug

Type in a right-to-left paragraph in Safari and the caret sits one position
behind where typing continues, from the moment the last character is a space. It
catches up as soon as a non-space character is typed. Hebrew and Arabic authors
put a space between every two words, so the caret is in the wrong place for a
large part of normal writing. Chromium and Firefox are correct.

Cause

Two things have to meet.

1. WebKit will not measure the boundary. For a collapsed Range at the end
of a text node, WebKit returns an empty client-rect list where the other engines
return the caret. getBoundingClientRect() is no better in that state: it
reports an all-zero rect. A non-collapsed range over the very same character is
measured correctly by every engine, WebKit included.

Sweeping dir × white-space × trailing character, all at the end-of-text-node
boundary:

direction white-space last character WebKit
rtl pre / pre-wrap / break-spaces space or tab empty
rtl any, normal included digit or Latin letter empty
ltr any, normal included digit empty
rtl pre NBSP correct
ltr pre space or tab correct

So the trigger is not "a trailing collapsible space" but "an end-of-text-node
caret on a bidi-level or preserved-whitespace boundary". It also fires at the end
of any text node, not only at the end of a paragraph — and the painter emits one
<span> per run, so a formatting boundary right after a space reaches it too.

2. The caret resolver's fallback assumes LTR. When the collapsed range comes
back empty, the engine falls back to an adjacent character's rect and takes the
next character's left, or the previous character's right. Those are the
logical edges in an LTR run. In an RTL run the logical end of a glyph is its
left edge, so the fallback returns the boundary before the trailing space —
one position back. (The LTR trailing-digit case above is invisible only because
the LTR edge happens to be the right answer there.)

Why the fix is here

The caret resolver is in @superdoc/docx-engine, which is not in this
repository, so the correction is applied one level below it: the measurement the
browser refuses to produce is supplied to any caller that asks for it, and the
engine's fallback is then never reached.

No DOM shape avoids the browser bug, so a painter-side fix is not available:
span-wrapping the text, appending a zero-width inline-block, a ZWSP span, a
<br>, or putting more content after the space all still return an empty list.

What the change does

webkit-collapsed-caret-rect.js wraps Range.getClientRects /
getBoundingClientRect so a collapsed range the browser refuses to measure is
answered from the neighbouring character's rect and that character's own
direction. Three properties keep it narrow, since this patches a DOM built-in
inside an embedded editor:

  • It only answers for text inside a mounted SuperDoc runtime, resolved
    through the shell's existing RUNTIME_ROOT_ATTRIBUTE. A range anywhere else in
    the host page gets the browser's own result byte for byte, so host code that
    reads "no rects" as "not rendered" keeps seeing exactly what it sees today.
  • It is installed only after the quirk is measured in the live browser, and
    the probe covers all three trigger families, so a WebKit that fixes only one of
    them does not silently drop the workaround for the others. Chromium, Firefox,
    a fixed future WebKit, jsdom and SSR run unpatched.
  • Installation cannot throw. A frozen Range.prototype (SES/Lockdown) or an
    instrumented DOM degrades to "not installed" rather than rejecting the
    engine-load promise and dropping the editor to its fail-closed stub.

Choosing the edge

The caret sits at the logical end of the character before it: the right edge of
a left-to-right character, the left edge of a right-to-left one. Deciding which
edge that is needs the character itself, not the geometry and not the paragraph.

The paragraph's direction is wrong whenever an RTL paragraph ends in Latin text
or in a number. Comparing the neighbouring glyph rects recovers a word, but not
a one-character run: "עמוד 5" ends in a one-character left-to-right run whose
rect sits exactly where a continuing right-to-left run's rect would sit, so the
two are geometrically indistinguishable — and the pixel tolerance such a
comparison needs breaks down under zoom, which the engine renders as a CSS
transform.

So the edge comes from the character's Bidi_Class, and the rules that decide
it are the Unicode Bidirectional Algorithm's own:

  • W1 — a non-spacing mark takes the class of the character before it, so
    ☺️ (emoji + variation selector) and é (e + combining acute) are read
    through to what they sit on rather than by the mark's own block.
  • I1/I2 — numbers are raised to an even, left-to-right level at both
    paragraph directions. This matters most for Arabic and Persian, whose native
    digits live inside the Arabic block but lay out left-to-right, so a block test
    alone misplaces the caret after every Arabic page number, date and price.
  • W2 + W5 — a terminator joins an adjacent European number, so 50% and
    100€ stay one left-to-right run after Hebrew. After an Arabic letter the same
    digits are re-read as an Arabic number, W5 no longer applies, and مرحبا 50%
    ends right-to-left. Both engines lay it out that way.
  • N0 — a paired bracket takes the direction of the strong text it encloses,
    with the text before the pair deciding when that run goes against the
    paragraph. שלום abc(def) — a Latin parenthetical inside Hebrew, which Hebrew
    technical and legal writing is full of — is the case this covers.
  • N1/N2 and L1 — anything else neutral takes the direction shared by its two
    sides, and the paragraph's direction otherwise. The paragraph direction is read
    from the containing block, not from the run span, because the painter puts
    dir on individual run spans.

The character classes are derived mechanically from DerivedBidiClass.txt and
BidiBrackets.txt (Unicode 17.0.0) rather than approximated by a general
category, because the two cut across each other exactly where it counts: NKo and
Adlam write their digits right-to-left, ½ and are numbers that are not
ordered as numbers, 620 decimal digits from Devanagari to Khmer are plain
left-to-right, the Arabic comma sits in a right-to-left block without being
right-to-left, and a .docx symbol run (Wingdings, Symbol) lands in private use,
which Unicode defaults to left-to-right.

Because no set of categories can be made exact, the last test is inverted: the
neutral classes are listed, and anything matching none of the tests is
left-to-right — which is what Unicode gives every code point it does not say
otherwise about. audit-classes.mjs puts all 297,334 assigned code points
through the module's own classifier and reports zero disagreements with
DerivedBidiClass.txt.

Measured

Two harnesses, both against real browsers through Playwright.

The rule, at the end-of-text caret in Hebrew, Arabic, Persian, NKo, Adlam and
Aramaic, scored against Chromium's own answer for the same caret. Two sets: 35
general boundaries, and 21 bracket boundaries.

                                general   brackets
by the paragraph's direction     8/16       -
by the neighbouring glyphs      13/16       -
the first revision of this rule 25/35     14/21
by the character's Bidi_Class   35/35     21/21

Of the seven bracket boundaries the rule used to miss, six are ones WebKit
refuses, so the shim was reached and answered wrongly: שלום abc(def) by 5.3px,
the full-width and CJK pairs by 16px.

The engine path, driving the engine's own caret resolution over 8528 caret
positions in Chromium and WebKit — 58 cases, inside and outside a runtime root,
at eight zoom levels, since the engine renders zoom as a CSS transform and a
scale multiplies every rect coordinate:

zoom   refused  wrong as shipped  repaired
0.5    45       16                45
0.75   45       16                45
1      45       16                45
1.1    45       16                45
1.25   45       16                45
1.5    45       16                45
2      45       16                45
3      45       16                45

Every boundary WebKit refuses now matches Chromium, at every zoom. Nothing either
engine already measured changes, and nothing outside a SuperDoc runtime changes
at all. A single caret, WebKit, Arial 16px: x 358.94 → 352.27, 6.67px toward the
line start, onto the left edge of the space just typed.

Verified on WebKit and Chromium through Playwright rather than on Safari for
macOS; the engines are the same, but I could not test the WKWebView host the
issue reporter uses.

Review round

Three findings on the first revision, all about the direction rule, all valid and
all now fixed and covered:

  1. \p{N} was standing in for "orders left-to-right". It is not: NKo and
    Adlam digits are Bidi_Class R, and ½, and the Aegean numerals are
    neutral. Chromium confirms all five. The number test is now Bidi_Class EN ∪ AN.

  2. Marks were classified by their own block instead of inheriting from the
    character before them. A Hebrew line ending in ☺️ put the caret on the wrong
    side; UBA rule W1 is now implemented, including the start-of-text case.

  3. Astral characters were classified as whole code points but measured as one
    UTF-16 code unit.
    Measured across six astral cases, both engines widen a
    range that splits a surrogate pair, and the one-code-unit rect came back
    byte-identical to the whole-code-point rect in each — so this does not
    reproduce, and the previous revision's astral answers already matched
    Chromium. The pair is now spanned explicitly anyway, since the DOM counts
    range offsets in code units and is not obliged to widen. That change is
    hardening, not a bug fix, and it is pinned by a test.

    Worth noting on the second finding: the example as stated — an Arabic or
    Hebrew letter followed by its own diacritic — was already correct, because
    those marks sit inside ֐-ࣿ. The case that actually justified
    implementing W1 is שלום ❤️, a Hebrew line ending in an emoji with a
    variation selector, which was 22px out and which WebKit refuses.

Following the same thread found four more of the same kind, also fixed. Rule N0
was missing entirely, so a bracket was only ever a neutral. And: Hebrew
and Arabic punctuation is strongly right-to-left without being a letter; the
Arabic comma and 45 other code points sit inside a right-to-left block without
being right-to-left; and private use, Roman numerals and Indic spacing marks are
left-to-right without being letters. The audit that found them runs over every
assigned code point in Unicode.

A second pass over the same ground found the classification still guessing, and
one guess was live. Decimal digits: only ASCII, Persian, fullwidth and a few
enclosed forms are European numbers, only the Arabic-Indic, Rumi and Hanifi ones
are Arabic numbers, and NKo and Adlam digits are right-to-left — which leaves 620
digits from forty-odd scripts as ordinary left-to-right characters that were
being read as neutral. שלום ६ was 7.4px out in Chromium's terms, on a boundary
WebKit refuses. That is what prompted inverting the default, and with it the
audit above comes back clean.

That pass also found the searches unbounded: they are linear, the caret is
resolved once per placement, and 20,000 characters of unbroken neutrals measured
at 750 seconds of main-thread work. Cutting them off at a fixed distance would
have fixed that by changing the answer — a neutral run longer than the cutoff
stops seeing the strong characters around it, and a wide bracket pair stops being
a pair — so the pass was made once and kept on the text instead.

A third round caught what that traded away. Every keystroke replaces the text,
so an analysis kept on the whole text misses on each one and reads the whole node
again: 11.7 ms of main-thread work per keystroke in a 20,000-character paragraph,
29 ms in a 50,000-character one. The caret at the end of a right-to-left text node
is exactly the boundary WebKit refuses, so that landed on essentially every
keystroke in Hebrew or Arabic.

So nothing is worked out up front. Each rule walks from the caret outward and
writes what it found back over the run it passed — every position that walk
crossed shares the answer, which is what makes that sound. And what a character
has behind it — its own class, the character a mark sits on, the nearest strong
character and strong letter before it, the left end of its terminator run — cannot
be changed by an edit after it, so an edit hands that half on and drops the rest.
One resolution costs the distance to the nearest strong character, one character
in ordinary text; all the resolutions over one text together cost a single pass
over it; and a keystroke never walks a run that has already been walked.

per keystroke, appending at the end of the node whole-node pass now
20,000 characters of ordinary text 11.7 ms 0.077 ms
50,000 characters of ordinary text 29.0 ms 0.205 ms
20,000 characters of unbroken neutrals 12.6 ms 0.109 ms
20,000 characters of unbroken terminators 0.262 ms
20,000 characters of unbroken combining marks 0.123 ms

Resolving a caret at every offset of a 20,000-character node, which is what the
kept walks are for, costs 48 ms in total — one pass, however the resolutions are
spread over it. The answer is what the algorithm says at every length either way:
the audit, the two boundary sets and the engine sweep below are unchanged by all
of this.

Only the paragraph-independent half is worked out here, so a character that
carries its own direction still resolves without reading the paragraph's, which
forces a style recalc.

And two defects not reachable from the caret path but real in exported or
internal code:
resolveCollapsedCaretGeometry threw on a non-string text rather than
returning null, and strongSideIsRtl looped forever on a negative index because
it tested at === 0 while walking away from zero.

One more, found by probing rather than by audit: the repair reads the rect of the
character before the caret, and a zero-width character has no rect to read.
WebKit refuses the boundary after שלום followed by a zero-width space, an RLM
or an LRM, so the shim was reached, found nothing to measure, and declined —
leaving the caret exactly where the bug put it. It now looks past neighbours with
no glyph box, bounded at 16 steps because each one is a forced layout.

Blast radius

Patching a DOM built-in inside a library that lives in someone else's page
deserves its own pass, and one turned up five things worth fixing:

  • The patched methods could throw. Installation was guarded; the methods it
    installs were not. getClientRects is specified never to throw for a valid
    range, and a host that has instrumented closest or getComputedStyle — an
    extension, a hardened realm, a test stub — could turn every Range on the page
    into a throwing API, including ranges that have nothing to do with SuperDoc.
    Everything after the native call is now guarded and falls back to the browser's
    own answer.
  • closest() does not cross a shadow boundary, so text inside a shadow root
    under the runtime root was not recognised as SuperDoc's and the bug survived
    there untouched. SuperDoc mounts painter content inside one in at least one
    supported embedding — which is why the shell reads pointer targets through
    composedPath(). Ownership now climbs out through shadow hosts.
  • A host that replaces Range.prototype.getClientRects rather than wrapping
    it silently undid the workaround for the rest of the page's life, because the
    installed mark sat on the prototype rather than on the function. It sits on the
    function now, so the next editor reinstates it. Reinstating over a host's own
    wrapper is harmless: the inner patch answers first, so the outer one sees rects
    and passes them through.
  • length and item were enumerable own properties on the returned rect
    list, where a real DOMRectList has a non-enumerable length accessor and
    item on its prototype — so JSON.stringify and Object.keys saw a shape this
    API has nowhere else. Indexing, item(), for...of, spread and Array.from
    already matched and still do.
  • A window that can never be measured was re-probed on every editor
    construction.
    Not caching "unknown" is deliberate — a window may simply have
    had no layout yet — but each probe forces a layout and delivers two childList
    records to any host observing document.body. It is now bounded.

Each of those has a test, and each test fails if the guarantee is removed.

Known gap

The workaround sees one text node. A number split across formatting runs — 1,
in one span and 234 in the next — is resolved from the part it can see, so rule
W4 cannot join the separator to a number living in the next node. Measured at
4.5px on that shape, on a boundary WebKit refuses. Closing it means giving the
rule the text on both sides of the node boundary, which is a larger change than
this one and is better done in the engine's own resolver, where the whole line is
already in hand.

Tests

webkit-collapsed-caret-rect.test.js, 74 tests: the edge rule (Hebrew,
Arabic, Latin, NKo, Adlam, Aramaic; an RTL paragraph ending in a word, in one
digit, in one letter, in an Arabic-Indic digit, in a percentage, in an Arabic
percentage, in a comma, in a fraction, in a Roman numeral, in a private-use
character; combining marks on Latin, on Hebrew, on an emoji and at the start of
text; W2, W5 on both sides, N1; surrogate pairs and which offset is measured;
degenerate input), what an edit may and may not keep (a replaced character, a
surrogate pair the edit completed, and what the edit moved), the cost of
sweeping a run forwards, of sweeping it backwards and of typing into it, quirk
detection including a partially-fixed engine and a
no-layout environment, and installation — passthrough, host-page DOM left alone,
the paragraph direction winning over an inline run span's, the zero-width
sentinel WebKit puts in front of a glyph, the DOMRectList shape, idempotency,
uninstall, a frozen prototype, an instrumented DOM, a host that replaces the
patched method outright, text inside a shadow root, a text node with no parent,
and a window that can never be measured.

Each guarantee is mutation-checked. Twenty-four mutations — removing W1, W2, N0,
N1, BD16's canonical equivalence, the EN/AN test, the right-to-left block
exclusions, the code-point-aligned measurement, the look-past-invisible search and
its bound, the patched methods' guard, the shadow climb, the rect-list shape, the
probe budget, the inverted default and the degenerate-input guard; and, for this
round, reusing the text's analysis, handing a prefix on across an edit, handing on
only the unchanged part, working a split code point out again, not handing on
what the edit moved, stopping a walk at an answer it meets, writing a walk's
answer over the run it passed, and keeping the left end of a terminator run — each
turns the suite red, and each is caught by the test written for it.

The three cost tests opt out of the suite's retry: 2, and that is worth saying
because it caught me: they measure work that is only done once, so a retry runs
the same text through a module that has already worked it out and passes however
slow the first attempt was. Sized so the gap is not marginal either — a
200,000-character node swept at 5,000 carets costs 73 ms as it stands and over
twelve seconds if any of the kept walks is removed.

v2-integration.test.js gains one test, so that deleting the install call fails
the suite.

Checks

  • vp test run --root ./packages/superdoc — 165 files, 2783 tests, 2780 pass.
    Two files are red on this checkout, both pre-existing and both Windows-only,
    and neither is reachable from src/core/v2-integration/:
    src/core/superdoc-ui.test.ts (3 tests) builds a map keyed by
    relative(SRC, path) and compares it against forward-slash literals, which
    cannot match on Windows; scripts/pack-sealed.test.mjs fails to transform at
    all, though node --check passes on it. Both are untouched here, both fail the
    same way on their own, and both would pass on the Linux runner.
  • vp fmt --check and vp lint — clean on every changed file. The one
    no-control-regex warning the neutral set used to raise is now scoped and
    explained rather than left standing: Bidi_Class B, S and WS cover tab, the line
    and paragraph breaks and the file and record separators, so the C0 controls
    belong in that set.
  • No public API surface added, so no tests/consumer-typecheck/ fixture is
    needed: the new module is internal and unreachable through the package's
    exports map.

The browser half of this cannot have an in-repo test, and it is worth saying why
rather than leaving a reviewer to ask for one: packages/superdoc/vite.config.js
runs the suite under happy-dom, whose Range.prototype.getClientRects returns
zero rects for every range, and jsdom does not define it at all. Under either,
the quirk probe declines to install. So the unit tests drive a hand-built fake
window, and the real-engine evidence above comes from Playwright.

A note for anyone reproducing on Windows: pnpm run <script> fails here before
it reads any config, because pnpm's verify-deps-before-run shells the root
prepare script, which is POSIX sh.
pnpm --config.verify-deps-before-run=false run <script> or calling
./node_modules/.bin/vp directly both work.

The durable fix

This belongs in the engine's caret resolver, which should take the logical edge
of the neighbouring character instead of assuming LTR. The rule here
(resolveCollapsedCaretGeometry) is written to be lifted straight into it, and
this module can then be deleted and the engine floor raised past it. Happy to
follow up there, or to reshape this if you would rather carry the workaround
somewhere else.

WebKit returns no client rects for a collapsed Range at the end of a text
node on a bidi or preserved-whitespace boundary, and an all-zero rect from
getBoundingClientRect. The caret resolver falls back to an adjacent
character's left-to-right edge, so in a right-to-left paragraph the caret
is painted one position behind where typing continues — between every two
words, for a Hebrew or Arabic author.

Supply the measurement the browser withholds: answer such a collapsed
range from the neighbouring character's rect and that character's own
direction, following the bidirectional algorithm for numbers, for
terminators that join a number, and for neutrals, which take the
paragraph's direction rather than the run span's.

Confined to text inside a mounted SuperDoc runtime, installed only after
the quirk is measured in the live browser, and unable to throw during
startup.

Fixes superdoc#3943
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/superdoc/src/core/v2-integration/webkit-collapsed-caret-rect.js Outdated
Comment thread packages/superdoc/src/core/v2-integration/webkit-collapsed-caret-rect.js Outdated
Comment thread packages/superdoc/src/core/v2-integration/webkit-collapsed-caret-rect.js Outdated
The review found three places where the direction rule approximated the
Unicode Bidirectional Algorithm by general category or code-point block
instead of reading it. Measured against Chromium's own caret for the same
boundary, in a browser that implements the algorithm:

  - `\p{N}` stood in for "orders left-to-right", which NKo and Adlam
    digits are not (Bidi_Class R), and `½` and `①` are not either
    (neutral). Five cases, 8.6px to 16px out.
  - A combining mark was classified by its own block rather than
    inheriting from the character it sits on. `שלום ❤️` was 22px out,
    and WebKit refuses that boundary, so it was live. Note that the
    example given in review — an Arabic or Hebrew letter followed by its
    own diacritic — was already correct: those marks sit inside
    ֐-ࣿ, so the block test agreed with W1 by construction.
  - Astral characters were classified as whole code points but measured
    as one UTF-16 code unit. This one does not reproduce: both engines
    widen a range that splits a surrogate pair, and the one-unit rect
    came back identical to the whole-code-point rect in each. The range
    now spans the pair anyway, since the DOM counts offsets in code
    units and is not obliged to widen. Hardening, not a bug fix.

Following the same thread found more of the same kind, all live and all
measured: rule N0 was missing entirely, so `שלום abc(def)` — a Latin
parenthetical inside Hebrew — was 5.3px out and the full-width and CJK
pairs 16px; Hebrew and Arabic punctuation is strongly right-to-left
without being a letter; the Arabic comma and 45 other code points sit
inside a right-to-left block without being right-to-left; private use,
Roman numerals and Indic spacing marks are left-to-right without being
letters; and rule W2 makes a European number Arabic after an Arabic
letter, so `مرحبا 50%` ends right-to-left where `שלום 50%` does not.

The classes are now derived mechanically from DerivedBidiClass.txt and
BidiBrackets.txt (Unicode 17.0.0), and the rule applies W1, W2, W5,
I1/I2, N0, N1/N2 and L1 in order.

Separately, a zero-width character before the caret has no glyph box to
read, and WebKit refuses the boundary after a trailing space followed by
a zero-width space, an RLM or an LRM — so the workaround was reached,
found nothing to measure, and left the caret where the bug put it. It
now looks past neighbours with no box, bounded because each step forces
a layout.

Patching a DOM built-in inside an embedded library also needed its own
pass:

  - The patched methods had no exception guard. `getClientRects` is
    specified never to throw for a valid range, and a host that has
    instrumented `closest` or `getComputedStyle` could turn every Range
    on the page into a throwing API.
  - `closest()` does not cross a shadow boundary, so text inside a
    shadow root under the runtime root was not recognised and the bug
    survived there. Ownership now climbs out through shadow hosts.
  - The installed mark sat on the prototype, so a host that replaced
    `getClientRects` rather than wrapping it silently undid the
    workaround for the life of the page. It sits on the function now.
  - `length` and `item` were enumerable own properties, where a real
    DOMRectList has neither.
  - A window that can never be measured was re-probed on every editor
    construction, each probe forcing a layout and delivering two
    childList records to any host observer. Now bounded.

Measured: 35/35 on the general boundary set and 21/21 on the bracket set
against Chromium, up from 25/35 and 14/21; and 8528 caret positions
driven through the engine's own resolution in Chromium and WebKit — 58
cases at eight zoom levels — with all 45 boundaries WebKit refuses
repaired at every zoom and nothing the browser already measured moved.

Refs superdoc#3943
A second review pass found the classification was still guessing in
places, and one of those guesses was live.

Decimal digits were the serious one. Only ASCII, Persian, fullwidth and
a few enclosed forms are European numbers, and only the Arabic-Indic,
Rumi and Hanifi ones are Arabic numbers; NKo and Adlam digits are
right-to-left. That leaves 620 decimal digits — Devanagari, Bengali,
Thai, Lao, Khmer, Myanmar and forty more scripts — as plain Bidi_Class L,
and they were falling through to "neutral" and taking the paragraph's
direction. Measured in Chromium, "שלום ६" put the caret 7.4px out, and
WebKit refuses that boundary, so it was reached and answered wrongly.

The wider problem was the shape of the test. Guessing which characters
are left-to-right from general categories cannot be made exact, because
the categories cut across Bidi_Class: `½` and `①` are numbers that are
neutral, private use and Indic spacing marks are left-to-right without
being letters. So the last test is inverted — the neutral classes are
listed, and everything else is left-to-right, which is what Unicode gives
every code point it does not say otherwise about. The two blocks where
the default is something else, the right-to-left scripts and the currency
symbols, are both resolved before it.

An audit over all 297334 assigned code points now reports the module
classifying every one of them exactly as DerivedBidiClass.txt says. The
previous pass claimed the same and was wrong: it decided which code
points were assigned with a table that shipped one Unicode version behind
the data file, which hid 66 code points inside the right-to-left blocks
that are not right-to-left — the Arabic honorific ligatures, the Arabic
Extended-C signs and the noncharacters among them. Nothing reads a second
source for that any more.

Also from the same pass:

  - The neutral, terminator, bracket and mark searches were unbounded and
    linear, and the caret is resolved once per placement, so an unbroken
    run of neutrals made key-repeat quadratic: 20,000 characters of them
    measured at 750 seconds. They are bounded now, and past the bound the
    paragraph decides, which is what the neutral rules give for a run
    that long anyway.
  - `resolveCollapsedCaretGeometry` threw on a non-string `text` instead
    of returning null, though its own length handling implied it should
    tolerate one. Not reachable from the caret path, which never passes
    one, but it is an exported function.
  - `strongSideIsRtl` looped forever on a negative index, because it
    tested `at === 0` while walking away from zero. Also unreachable —
    the public entry point rejects a negative offset — but a landmine.
  - Bidi_Class NSM is general category Mn or Me except for five Indic
    vowel signs that are Mn with Bidi_Class L; they are excluded now.

One known gap stays, and is worth naming rather than hiding: the
workaround sees one text node, so a number split across formatting runs —
"1," in one span and "234" in the next — is resolved from the part it can
see, and rule W4 cannot join them. Measured at 4.5px on that shape.
Closing it means giving the rule the text on both sides of the node
boundary, which is a larger change than this one.

Refs superdoc#3943

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 2 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/superdoc/src/core/v2-integration/webkit-collapsed-caret-rect.js Outdated
The distance cutoff added in the previous commit was the wrong trade and
the review is right to reject it. It was there because the neutral,
terminator, mark and bracket searches are each linear in the text they
walk, and the caret is resolved once per placement, so an unbroken run of
neutrals made key-repeat quadratic — 20,000 characters of them measured
at twelve minutes of main-thread work. Cutting the walks off at 512
characters fixed that by changing the answer: a neutral run longer than
the cutoff stopped seeing the strong characters around it, and a bracket
pair wider than it stopped being a pair. That is the same class of defect
the workaround exists to fix, traded for speed.

The work is bounded by doing it once instead. Everything the rules need
from the rest of the text — the nearest strong character on each side,
the character a mark sits on, whether a terminator touches a European
number, and where the bracket pairs are — is computed in a single pass
over the text and cached on it, so each resolution is a handful of array
reads however long the text is. Only the paragraph-independent half is
precomputed, so a character that carries its own direction still resolves
without reading the paragraph's and forcing a style recalc.

Measured on the case that prompted the cutoff, a text node of nothing but
terminators, one resolution per offset:

     2000 characters:  24.1 ms
     4000 characters:  13.8 ms
    20000 characters:  33.5 ms

Linear, where the same benchmark took 750 seconds before the cutoff. And
the answer is now what the algorithm says at any length: the test that
asserted the truncated behaviour has been replaced by one that puts two
Hebrew letters 1200 characters apart with the caret in the neutral run
between them, which N1 gives their direction and not the paragraph's.

Nothing else moves. The classification audit still reports the module
agreeing with DerivedBidiClass.txt on all 297334 assigned code points;
the rule still scores 35/35 on the general boundary set and 21/21 on the
bracket set against Chromium; the engine harness still repairs all 45
boundaries WebKit refuses across 8528 caret positions at eight zoom
levels; and all seventeen mutations are still caught.

Refs superdoc#3943

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 2 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/superdoc/src/core/v2-integration/webkit-collapsed-caret-rect.js Outdated
@Nathaniel-260

Copy link
Copy Markdown
Contributor Author

Reproduced the quirk independently, so the premise of this PR is measured here rather than carried over from #3943's matrix: Playwright's WebKit 26.0 (browser build 2248, playwright-core 1.58.0) on Windows 10, 16px sans-serif, Range.getClientRects() on a collapsed range at the end of the text node.

direction white-space node collapsed @ end last char, non-collapsed
rtl pre "שלום " [] x=1235.5 w=4.4
rtl pre "שלום\t" [] x=1236.4 w=3.5
rtl pre-wrap "שלום " [] x=1235.5 w=4.4
rtl pre "שלום\u00A0" x=1235 x=1235 w=5
ltr pre "abc " x=38 x=33 w=5.2

The three documented failures reproduce and the two documented successes reproduce, in the same configuration and with the same split between collapsible whitespace and U+00A0. The detection probe in this PR fires in this build for the same reason: its first case is that first row.

One boundary worth recording

A node whose entire content collapses away — a lone space or tab under white-space: normal — returns no rect at every offset, not only at the end, and has no client rect of its own either:

node native [] at any character with a glyph box node's own box
rtl normal " " every offset no none
rtl normal "\t" every offset no none
rtl normal " " every offset no none
ltr normal " " every offset no none

There is no neighbouring glyph to read an edge from and no box to fall back to, so resolveCollapsedCaretGeometry declines and the wrapper returns the native (empty) list. That is the right answer — a node that paints nothing has no caret position to report — but it is the one input class where the workaround cannot help, and it looked worth stating explicitly rather than leaving to be rediscovered as a gap.

No interaction with #3950

Since #3950 replaces the empty line's &nbsp; placeholder with U+200B and touches the same caret geometry, I measured it: WebKit answers it in every configuration, so it never reaches this path (the wrapper returns the native rects whenever they have height).

direction white-space node collapsed @ end
rtl pre "שלום\u200B" x=1239
rtl normal "שלום\u200B" x=1239
rtl normal "\u200B" alone x=1272 — content-box right edge
ltr normal "\u200B" alone x=8 — content-box left edge
rtl normal "\u00A0" alone (today) x=1267

The quirk is specific to collapsible whitespace, not to zero-advance characters: a zero-width character is measured, and on an empty line the caret lands on the correct side in both directions. The two changes are independent.

Keeping a whole-node analysis on the text answered the last round's review but
traded one quadratic case for another: every keystroke replaces the text, so the
analysis misses on each one and the whole node is read again. Measured at 11.7 ms
of main-thread work per keystroke in a 20,000-character paragraph and 29 ms in a
50,000-character one -- and the caret at the end of a right-to-left text node is
exactly the boundary WebKit refuses, so that landed on essentially every
keystroke in Hebrew or Arabic.

Nothing is worked out up front now. Each rule walks from the caret outward and
writes what it found back over the run it passed, since every position that walk
crossed shares the answer. And what a character has behind it -- its own class,
the character a mark sits on, the nearest strong character and strong letter
before it, the left end of its terminator run -- cannot be changed by an edit
after it, so an edit hands that half on and drops the rest.

One resolution costs the distance to the nearest strong character, which is one
character in ordinary text; all the resolutions over one text together cost a
single pass over it; and a keystroke never walks a run that has already been
walked. Per keystroke, appending at the end of the node:

    20,000 characters of ordinary text      11.7 ms -> 0.077 ms
    50,000 characters of ordinary text      29.0 ms -> 0.205 ms
    20,000 characters of unbroken neutrals  12.6 ms -> 0.109 ms

The answer is unchanged at every length: the classifier still agrees with
DerivedBidiClass.txt on all 297,334 assigned code points, the rule still scores
35/35 on the general boundary set and 21/21 on the bracket set against Chromium,
and the engine harness still repairs all 45 boundaries WebKit refuses across 8528
caret positions at eight zoom levels.

Five tests cover what an edit may and may not keep, and three cover the cost --
sweeping a run forwards, sweeping it backwards, and typing into it. Those three
opt out of the suite's retry, because a retried cost test can never fail: the
retry runs the same text through a module that has already worked it out. Each of
the twenty-four mutations is caught by the suite.

Refs superdoc#3943

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 2 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/superdoc/src/core/v2-integration/webkit-collapsed-caret-rect.js Outdated
…nstead of timing them

Two review findings on the previous round, both valid.

An edit that took away or replaced the low surrogate of a cached astral character left its high surrogate carrying the whole character's class, so a Hebrew line ending in Phoenician alf that lost the pair's second half kept a right-to-left answer for a lone surrogate, which Unicode gives Bidi_Class L. The code-point boundary the carry stops at is now taken in both texts, the old and the new, and a test covers the removed and the replaced low surrogate.

The three cost tests measured wall-clock time against a one-second budget with retries off, which a loaded CI runner could miss with nothing wrong. The module now counts the positions its walks visit — one increment per position walked, nothing in production reads it — and the tests read that count: a pass over the text against a pass per caret sits thousands of counts apart, and the same on every machine. The node shrank from 200,000 to 20,000 characters since the count needs no clock gap, and the file runs in less than half the time.

Six mutations — the Math.min, stopping a walk at an answer it meets in strongBefore and in terminatorRunFirst, writing a walk's answer over the run it passed, the carry as a whole and the terminator run's carry alone — each turn the suite red, and each is caught by the test written for it.
@Nathaniel-260
Nathaniel-260 force-pushed the fix/rtl-caret-webkit-collapsed-range branch from 8f8a7d3 to 4f7707d Compare September 2, 2026 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

V2: caret is drawn one position back after a trailing space in RTL text (WebKit only)

1 participant