render: the bidi-override marker overwrites the preceding character #257

New issue

Open

opened 2026-09-12 01:21:53 +09:00 by natsukium · 0 comments

natsukium commented

2026-09-12 01:21:53 +09:00

Owner

Summary

The Trojan-Source bidi marker (REQ-909) is painted on the cell that precedes the bidi codepoint,
so the marker covers a legitimate character instead of occupying a cell of its own. The preceding
character becomes unreadable.

Reproduction

printf 'for \342\201\250manyara.tail4108.ts.net\342\201\251 end\n'

(U+2068 FIRST STRONG ISOLATE / U+2069 POP DIRECTIONAL ISOLATE — the pair fluent-bundle emits around
every placeable, so any Fluent-localized CLI produces them; fj auth login is one.)

Expected: for <marker> manyara.tail4108.ts.net <marker> end — every real character still legible.

Actual: felis renders for█manyara.tail4108.ts.net█ end. The space before manyara and the final t
of net are replaced by the yellow marker block. Verified with a window capture on macOS
(felis-macos-gui-debug).

Cause

U+2066–U+2069 and U+202A–U+202E are all zero-width per unicode-width
(confirmed: '\u{2068}'.width() == Some(0)). Grid::put_grapheme therefore treats them as combining
marks — extends_previous_grapheme returns true on width == 0
(crates/felis-grid/src/editing.rs:447) — and folds them into the previous cell's cluster.

The renderer then finds a bidi override inside that cluster
(cell_contains_bidi_override, crates/felis-render-wgpu/src/instances.rs:1070) and repaints the
whole cell with BIDI_MARKER_BG / BIDI_MARKER_FG (instances.rs:506). The cell it repaints is the
one holding the base character, which is why the base character disappears.

Notes for a fix

The requirement is a visible marker on the cell whose grapheme contains the codepoint
(docs/explanation/security-model.md "Text rendering"); nothing requires destroying a neighbouring
glyph, and losing a character is itself a legibility problem for the reader the requirement is meant
to protect. Two directions:

Stop folding bidi overrides into the previous cluster (exempt bidi::is_override from the
width == 0 fold) and give each one its own single-width marker cell. This costs a column per
codepoint but keeps every real character on screen.
Keep the fold and draw the marker as a decoration that does not replace the base glyph (an
underline/outline rather than a background+fg swap).

The first matches how the marker reads today (a block) and keeps cell_contains_bidi_override's
cluster walk for the base-char-plus-RLO case that motivated iter_contains_override.

Whichever lands needs a renderer test alongside bidi_override_cell_paints_with_warning_marker
(instances.rs:2199) that asserts the preceding cell still draws its own glyph.

## Summary The Trojan-Source bidi marker (REQ-909) is painted on the cell that *precedes* the bidi codepoint, so the marker covers a legitimate character instead of occupying a cell of its own. The preceding character becomes unreadable. ## Reproduction ```sh printf 'for \342\201\250manyara.tail4108.ts.net\342\201\251 end\n' ``` (U+2068 FIRST STRONG ISOLATE / U+2069 POP DIRECTIONAL ISOLATE — the pair `fluent-bundle` emits around every placeable, so any Fluent-localized CLI produces them; `fj auth login` is one.) Expected: `for` `<marker>` `manyara.tail4108.ts.net` `<marker>` ` end` — every real character still legible. Actual: felis renders `for█manyara.tail4108.ts.net█ end`. The space before `manyara` and the final `t` of `net` are replaced by the yellow marker block. Verified with a window capture on macOS (`felis-macos-gui-debug`). ## Cause U+2066–U+2069 and U+202A–U+202E are all zero-width per `unicode-width` (confirmed: `'\u{2068}'.width() == Some(0)`). `Grid::put_grapheme` therefore treats them as combining marks — `extends_previous_grapheme` returns `true` on `width == 0` (`crates/felis-grid/src/editing.rs:447`) — and folds them into the previous cell's cluster. The renderer then finds a bidi override inside that cluster (`cell_contains_bidi_override`, `crates/felis-render-wgpu/src/instances.rs:1070`) and repaints the whole cell with `BIDI_MARKER_BG` / `BIDI_MARKER_FG` (`instances.rs:506`). The cell it repaints is the one holding the *base* character, which is why the base character disappears. ## Notes for a fix The requirement is a *visible marker on the cell whose grapheme contains the codepoint* (`docs/explanation/security-model.md` "Text rendering"); nothing requires destroying a neighbouring glyph, and losing a character is itself a legibility problem for the reader the requirement is meant to protect. Two directions: - Stop folding bidi overrides into the previous cluster (exempt `bidi::is_override` from the `width == 0` fold) and give each one its own single-width marker cell. This costs a column per codepoint but keeps every real character on screen. - Keep the fold and draw the marker as a decoration that does not replace the base glyph (an underline/outline rather than a background+fg swap). The first matches how the marker reads today (a block) and keeps `cell_contains_bidi_override`'s cluster walk for the base-char-plus-RLO case that motivated `iter_contains_override`. Whichever lands needs a renderer test alongside `bidi_override_cell_paints_with_warning_marker` (`instances.rs:2199`) that asserts the *preceding* cell still draws its own glyph.

natsukium added the

type/bug

label

2026-09-12 01:24:27 +09:00

natsukium referenced this issue

2026-09-12 01:31:32 +09:00

render: the bidi marker fires on Fluent's routine FSI/PDI isolates #258