mirror of
https://github.com/penpot/penpot.git
synced 2026-10-02 08:46:15 +00:00
257 lines
14 KiB
Markdown
257 lines
14 KiB
Markdown
# Handoff — v3 editor IME input (candidate window placement + premature commit)
|
|
|
|
> Context for continuing the IME work on branch `poc-japanese-text`.
|
|
> Rewritten 2026-07-19 after the premature-commit fix was confirmed with the
|
|
> real IME. Companion to `handoff-japanese-text.md` (the overall feature
|
|
> handoff); this file covers only the in-flight IME editor work.
|
|
|
|
## Problem statement
|
|
|
|
When typing Japanese with an IME in the v3 (WASM) text editor:
|
|
|
|
1. composition must not be interrupted (the user must be able to compose kana
|
|
and convert to kanji) — **FIXED, user-confirmed with real fcitx5-mozc**;
|
|
2. the IME candidate/suggestion popup must appear next to the current input
|
|
point, in both horizontal and vertical layouts — **partially working;
|
|
remaining work below**.
|
|
|
|
The user's environment is Linux/X11 + Chromium + **fcitx5 with mozc**
|
|
(`fcitx5 -d` + `/usr/lib/mozc/mozc_server`; the previous handoff said
|
|
ibus-mozc — that was wrong).
|
|
|
|
## The two root causes found for premature commit
|
|
|
|
Real IMEs abort the composition and commit the pending kana on ANY DOM
|
|
disturbance around the composing element. Two distinct triggers were found and
|
|
removed:
|
|
|
|
1. **Store dispatch in `on-composition-update`.**
|
|
`sync-wasm-text-editor-content!` dispatched
|
|
`dwt/v2-update-text-shape-content` per kana → React re-render of the editor
|
|
component mid-composition (shape content/name/dimensions, foreignObject
|
|
attrs, CSS vars). Removed: the WASM `text_editor_composition_update`
|
|
already writes the preview into WASM shape state and marks it touched, so
|
|
`request-render` alone paints the preview. The store is synced once on
|
|
`compositionend`.
|
|
2. **Style writes on the keydown-229 that precedes `compositionstart`.**
|
|
The old design repositioned the capture surface on the first keydown of an
|
|
IME sequence (`keyCode 229 && !isComposing`), believing that was "before
|
|
the composition". It is not safe: by the time keydown-229 reaches the DOM,
|
|
the IME has already established its composition context, and the style
|
|
write still aborts real mozc (CDP tolerates it — see Verification below).
|
|
The user confirmed the abort persisted after fix 1 alone; it stopped after
|
|
the redesign in fix 2.
|
|
|
|
## Current design (working tree)
|
|
|
|
All in `frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs` +
|
|
`v3_editor.scss`:
|
|
|
|
- **Idle positioning**: the capture surface (`#text-editor-wasm-input`) sits
|
|
ON the WASM caret at all times. `schedule-ime-caret!` runs after every
|
|
caret-affecting operation (click, pointer-up, double-click, handled keydown,
|
|
on-input, paste, cut, composition end, focus, mount) — never during a
|
|
composition (`composing-ref` guard, set in `on-composition-start`, cleared
|
|
in `on-composition-end`). No handler touches the surface or the store
|
|
between `compositionstart` and `compositionend`.
|
|
- `schedule-ime-caret!` waits two `requestAnimationFrame`s (so the pending
|
|
WASM render can rebuild the layout the caret rect reads) and then retries up
|
|
to 30 frames because the rect can be unavailable right after mount.
|
|
**Gotcha: rAF does not fire in occluded/background windows** (see Tooling).
|
|
- `update-ime-caret!` (unchanged semantics): horizontal → `left/top` at caret,
|
|
`font-size` = caret rect height; vertical → `writing-mode: vertical-rl`,
|
|
right-edge anchor `left = (x+width) - origin.x - origin.width`,
|
|
`font-size` = caret rect width. Returns nil when it could not position
|
|
(no node/origin/rect) — the scheduler retries on nil.
|
|
- **`pointer-events: none` on the surface** (`v3_editor.scss`): clicks always
|
|
land on the outer overlay div, so `offsetX/offsetY` remain in the shape's
|
|
local space regardless of where the surface currently sits. The hover text
|
|
cursor class moved to the outer div.
|
|
- **`focus-editor!`**: programmatic `.focus()` on the surface does NOT
|
|
reliably fire focus/focusin in Chromium (contenteditable inside SVG
|
|
foreignObject): the node becomes `document.activeElement` but React's
|
|
`on-focus` never dispatches, so the WASM editor stayed unfocused and every
|
|
caret-rect read returned null (this made the vertical popup anchor to the
|
|
default full-size surface with no writing-mode — the "jumping" popup).
|
|
`focus-editor!` therefore focuses the node AND calls
|
|
`wasm.api/text-editor-focus` + `schedule-ime-caret!` explicitly. Used by the
|
|
mount effect and `on-pointer-down` (which also `preventDefault`s so
|
|
mousedown cannot move focus to the body).
|
|
Note `text-editor/text-editor-focus` THROWS when the shape id is not in
|
|
WASM state — a possible mount race to keep in mind.
|
|
- `on-composition-end`: WASM commit + store sync + `reset-input-node` +
|
|
`schedule-ime-caret!` (surface moves to the caret past the committed text).
|
|
There is no full-size "reset" anymore; the surface stays caret-positioned
|
|
for the whole editing session.
|
|
|
|
Supporting changes (unchanged from before, already built):
|
|
|
|
- `frontend/src/app/render_wasm/text_editor.cljs`:
|
|
`text-editor-get-cursor-rect` reads 4 f32 (x/y/w/h) from
|
|
`_text_editor_get_cursor_rect`. Coordinates are ABSOLUTE page coords (selrect
|
|
origin included); `update-ime-caret!` subtracts the foreignObject origin
|
|
(`origin-ref`, kept fresh each render, includes the valign-adjusted y).
|
|
- `render-wasm/src/wasm/text_editor.rs`: cursor rect independent of the caret
|
|
blink phase (null only when the editor lacks focus). The WASM artifact with
|
|
this change is built and loaded in the ws1 devenv.
|
|
|
|
## Verification status
|
|
|
|
### Horizontal — real IME, PASSED (2026-07-19)
|
|
|
|
Automated real-IME run (see Tooling): typed `nihongo`, Space-converted,
|
|
Return-committed through actual fcitx5-mozc:
|
|
|
|
- ONE composition `n→に→にほ→にほんご→日本語`, no aborts, no fragment commits;
|
|
committed text landed in the shape; store synced at compositionend; no crash.
|
|
- Surface style byte-identical through the whole composition; a
|
|
MutationObserver over the entire foreignObject subtree recorded ZERO
|
|
attribute/childList mutations between compositionstart and compositionend.
|
|
- The fcitx candidate window tracked the preedit (window x advanced 866→967 at
|
|
constant y, matching the DOM caret anchor at the end of the preedit).
|
|
- The user separately confirmed typing works with their own keyboard.
|
|
|
|
### Vertical — NOT yet verified with the real IME
|
|
|
|
The vertical real-IME run was voided: the user's screen was locked (slock) so
|
|
the synthetic keystrokes never reached the browser (all-blue screenshots, no
|
|
composition events, text unchanged). **Rerun `vime-test.sh` when the screen is
|
|
unlocked.** Under CDP the vertical mechanics passed earlier (single
|
|
composition, style byte-identical, commit lands, caret advances down the
|
|
column), but CDP proves mechanics only.
|
|
|
|
## Remaining known bugs (the actual work left)
|
|
|
|
1. **Horizontal caret rect / hit test broken for CJK-heavy content → popup
|
|
anchors at the shape origin.** With content `日本語ab`:
|
|
- `text_editor_get_cursor_rect` always returns the LINE-START x (rect x ==
|
|
selrect.x) regardless of the real caret offset. In
|
|
`render-wasm/src/wasm/text_editor.rs` (`get_cursor_rect`, ~line 1103):
|
|
`get_rects_for_range(char_pos..char_pos, Tight, Tight)` comes back empty
|
|
and the fallback uses `get_glyph_position_at_coordinate((0.0,0.0))`
|
|
`.position as f32` — a glyph INDEX used as an X COORDINATE. That is a bug
|
|
regardless of the root cause of the empty rects.
|
|
- `text_editor_set_cursor_from_offset` maps EVERY (x,y) to offset 0 for the
|
|
same shape (verified with direct WASM calls bypassing all CLJS changes,
|
|
x swept 1..200, y swept 0..40; valign top). So click-to-place-caret is
|
|
broken on that content too.
|
|
- Both symptoms suggest the layout the hit-test paths read
|
|
(`text_content.layout.paragraphs`) differs from what is painted —
|
|
possibly laid out without the Japanese fallback fonts (zero-width
|
|
glyphs collapse every x to 0). The canvas paints correctly, and
|
|
`get_text_dimensions`/auto-width grow correctly, so some other layout is
|
|
fine. Compare how `layout.paragraphs` is (re)built after edits vs the
|
|
render path in `render/text.rs` / `render/text_editor.rs`.
|
|
- The vertical paths are fine (they recompute from
|
|
`text_vertical::layout_from_content` on demand).
|
|
2. **Vertical popup verification pending** (screen lock, above). After the
|
|
focus fix the mount-time WASM focus is established and the caret rect for
|
|
vertical returns correct values (measured: end-of-text caret
|
|
x=3728.6/y=1278/w=16.8/h=1 on the test shape), so the surface should now
|
|
get `writing-mode: vertical-rl` + caret position before the first
|
|
composition. Needs the on-screen run to confirm popup placement.
|
|
3. Possible mount race: `text-editor-focus` throws if the WASM shape is not
|
|
yet in state when the mount effect runs. Not reproduced conclusively, but
|
|
one entry attempt showed WASM focus false with no error until a manual call
|
|
at +300ms succeeded. If editor entry ever silently loses WASM focus again,
|
|
look here (make `focus-editor!` retry or make the throw a no-op+retry).
|
|
4. Compositions that start WITHOUT keydown (voice input, on-screen keyboards)
|
|
rely on the last idle position — now always correct by design (the surface
|
|
is always on the caret), which retires the old concern.
|
|
5. v2 editor untouched; macOS/Safari behavior unknown.
|
|
|
|
## Files touched (uncommitted, on `poc-japanese-text`)
|
|
|
|
- `frontend/src/app/main/ui/workspace/shapes/text/v3_editor.cljs` — the design
|
|
above (`update-ime-caret!` idle model, `schedule-ime-caret!` retries,
|
|
`composing-ref`, `focus-editor!`, `pointer-events`-aware pointer handlers,
|
|
no store sync in `on-composition-update`).
|
|
- `frontend/src/app/main/ui/workspace/shapes/text/v3_editor.scss` —
|
|
`pointer-events: none` on `.text-editor-container`.
|
|
- `frontend/src/app/render_wasm/text_editor.cljs` —
|
|
`text-editor-get-cursor-rect`.
|
|
- `render-wasm/src/wasm/text_editor.rs` — cursor rect independent of blink
|
|
phase.
|
|
|
|
Lint/format for the CLJS file not yet run this round (`clj-kondo`/`cljfmt`
|
|
live in the devenv container, not on the host PATH).
|
|
|
|
## Tooling — REAL-IME automated testing (new, important)
|
|
|
|
CDP (`Input.imeSetComposition`) bypasses the OS input method entirely: it
|
|
cannot reproduce IME aborts and cannot show the candidate popup (an OS window
|
|
fcitx draws outside the browser — Playwright screenshots never contain it).
|
|
The real pipeline is scriptable because the Playwright Chrome runs headed on
|
|
the user's X11 display (`DISPLAY=:0`):
|
|
|
|
- Switch engine: `fcitx5-remote -s mozc` … restore with
|
|
`fcitx5-remote -s keyboard-us-altgr-intl` (query with `-n`).
|
|
- Real keystrokes: `xdotool windowactivate --sync <WIN>` then
|
|
`xdotool type --delay 250 "nihongo"`, `xdotool key space`, `key Return`.
|
|
XTEST events go through fcitx/mozc exactly like hardware input.
|
|
- The Chrome window: find by exact title, e.g.
|
|
`xdotool search --name "Japanese text - Penpot"` (was id 60817432; do NOT
|
|
head -1 a loose pattern).
|
|
- Popup observation: full-screen `scrot -o file.png` + geometry via
|
|
`for w in $(xdotool search --class fcitx); do xdotool getwindowgeometry --shell $w; done`.
|
|
Geometry persists for unmapped windows — corroborate with the screenshot.
|
|
- Page-side instrumentation (before typing): composition-event log capturing
|
|
`node.getAttribute('style')` and the DOM caret anchor
|
|
(`getSelection().getRangeAt(0).getBoundingClientRect()`) per event, plus a
|
|
MutationObserver over the foreignObject subtree asserting zero
|
|
attribute/childList mutations mid-composition.
|
|
- Scripts saved in the session scratchpad as `ime-test.sh` (horizontal) and
|
|
`vime-test.sh` (vertical) — recreate from this description if gone.
|
|
|
|
**Safety checks before sending synthetic input:**
|
|
|
|
- Screen lock: `pgrep slock` (user uses slock) and
|
|
`DISPLAY=:0 xset q | grep Monitor` (Monitor off = user away). If locked, DO
|
|
NOT type — keystrokes land in the password prompt.
|
|
- The user's fcitx layout must be restored afterwards
|
|
(`keyboard-us-altgr-intl`).
|
|
|
|
**Other environment gotchas (all hit this session):**
|
|
|
|
- `requestAnimationFrame` does not fire while the Chrome window is occluded —
|
|
`schedule-ime-caret!` verification MUST be done with the window visible
|
|
(activate it first). Store-side checks are unaffected.
|
|
- After `location.reload()` the editor may not mount:
|
|
`(reset! app.render-wasm.api/page-transition? false)` then re-enter edition.
|
|
- Programmatic edition entry:
|
|
`select_shape(uuid)` → `zoom.fit_to_shapes([uuid])` (NOT the arity-0
|
|
`fit-to-shapes`; Shift+2 via CDP keyboard is unreliable) →
|
|
`start_editing_selected()`.
|
|
- Munged names used a lot: `app.render_wasm.text_editor.text_editor_get_cursor_rect`,
|
|
`..._has_focus_QMARK_`, `app.main.data.workspace.zoom.fit_to_shapes`,
|
|
`cljs.core.clj__GT_js`. Patching `app.render_wasm.text_editor.foo` does NOT
|
|
intercept calls made through `app.render_wasm.api.foo` (the api def captured
|
|
the original fn object) — patch the api var too.
|
|
- The test shapes contain residue text from aborted/committed test runs:
|
|
horizontal `0121fd5b-f9b4-8060-8008-586a95941bbe` now
|
|
`日本語ab日本語を知覚異常日本語…`, vertical
|
|
`f179b962-b099-8026-8008-5641a910fe2e`
|
|
`日本語をしますかアリアがとうごいます…`. Content is junk by design; keep using
|
|
them.
|
|
- File "Japanese text" (Drafts), page `showcase`,
|
|
`https://localhost:13449/#/workspace?team-id=9aa629b4-0900-8061-8008-3ac96936247d&file-id=f8554703-a493-807a-8008-454b071d651d&page-id=f8554703-a493-807a-8008-454b071d8665`.
|
|
|
|
## Suggested next steps (in order)
|
|
|
|
1. Rerun the vertical real-IME test with the screen unlocked
|
|
(`vime-test.sh`); verify the popup follows the caret down the column and
|
|
commits cleanly. Also re-verify horizontal once more after any changes.
|
|
2. Fix the Rust horizontal caret-rect/hit-test for CJK content (bug 1): find
|
|
why `layout.paragraphs` yields empty caret-range rects / zero-width
|
|
positions for `日本語ab` while painting is correct; fix the
|
|
index-as-coordinate fallback while there. Add a Rust regression test with
|
|
CJK content asserting `get_cursor_rect` x monotonically increases with the
|
|
caret offset and `set_cursor_from_offset` round-trips.
|
|
3. Re-verify click-to-place-caret + surface positioning end-to-end in both
|
|
flows, then run `clj-kondo`/`cljfmt` (devenv) on the edited CLJS.
|
|
4. Human confirmation from the user in both flows (type, convert, Esc-cancel,
|
|
two consecutive compositions, click into text mid-composition-free).
|
|
|
|
See auto-memory `v3-editor-repl-verification` for the distilled recipes and
|
|
`handoff-japanese-text.md` for build/test commands and branch state.
|