diff --git a/.claude/skills/resume-ckeditor/SKILL.md b/.claude/skills/resume-ckeditor/SKILL.md new file mode 100644 index 000000000000..18be2d51a584 --- /dev/null +++ b/.claude/skills/resume-ckeditor/SKILL.md @@ -0,0 +1,51 @@ +--- +name: resume-ckeditor +description: Resume the long-running "retire CKEditor / unify Undo" project (BL-6681). Use when the user says "/resume-ckeditor", "resume the ckeditor work", "continue retiring ckeditor", or picks that project back up after an interruption (sleep, token exhaustion, days off). +--- + +# Resume the CKEditor-retirement project (BL-6681) + +This is a deliberately long-running project spanning many sessions and many rebases against a +moving `master`. Its state lives in `docs/retire-ckeditor/`, not in any session's memory. + +## Do this, in order + +1. **Read the state.** + - `docs/retire-ckeditor/PROGRESS.md` — the live log, current phase, and next actions. Start here. + - `docs/retire-ckeditor/PLAN.md` — the staged plan (Stages 0–6). Authoritative for *what* to + do. §11 records what BL-6681 itself asks for, and what on it is already obsolete. + - `docs/retire-ckeditor/REVIEW-NOTES.md` — findings already verified and decisions already + made. **Do not re-litigate anything settled there.** If you think a settled point is wrong, + say so explicitly to the user rather than quietly changing course. + - `docs/retire-ckeditor/BEHAVIOR-INVENTORY.md` if it exists — the behaviours that must survive. + +2. **Orient in git.** `git status`, `git log --oneline -15`, and note the current branch. Because + the plan deliberately avoids long-lived branches, you may well be on `master` with nothing in + flight; that is the normal resting state between stages, not a sign something was lost. + +3. **Check for drift.** Other work lands on `master` continuously. Before continuing a stage, + confirm the files it touches still look the way the plan assumes — the plan cites specific + `file:line` locations, and those move. If a citation has gone stale, fix the plan text as part + of the work; a plan nobody trusts is worse than no plan. + +4. **Continue from the next unchecked item** in PROGRESS.md's "Next actions". Confirm with the + user which stage to work on if more than one is plausible. + +5. **Before ending the session** (or when you sense you are running low on context), update + `PROGRESS.md`: what you did, what you learned, the branch/PR, and a revised "Next actions". + Do this even if the work is half-finished — especially then. + +## Ground rules for this project + +- **New code goes in new files** (`bookEdit/undo/`, `bookEdit/textEditor/`). Edits to existing + files should be one-line dispatches wherever possible, and as late in the plan as possible. + This is the project's whole defence against rebase pain. +- **Don't keep a long-lived branch.** Each stage is designed to be its own small, green, + flag-inert PR onto `master`. +- **Deletion commits (Stage 5) are regenerated, never rebased.** If one conflicts, throw it away + and redo it mechanically. +- Build and test through the wrappers, never bare `dotnet`/`vite` — `build/agent-dotnet.sh` and + `build/agent-vite.sh` — because the developer usually has a Bloom running via `./go.sh`. See + `AGENTS.md`. Never run the full `pnpm build`. +- To see a change in the running Bloom, just edit the source (the Vite dev server pushes it in) + and observe via the `run-bloom` skill. No build. diff --git a/docs/retire-ckeditor/BEHAVIOR-INVENTORY.md b/docs/retire-ckeditor/BEHAVIOR-INVENTORY.md new file mode 100644 index 000000000000..3a77441197b4 --- /dev/null +++ b/docs/retire-ckeditor/BEHAVIOR-INVENTORY.md @@ -0,0 +1,268 @@ +# Behaviour inventory — what must still work after CKEditor is gone + +Stage 0 of [PLAN.md](PLAN.md). This is the acceptance criteria for the whole project. + +## What is and isn't listed here + +Listed: behaviours **at risk from this project** — implemented by CKEditor, or implemented in code +we will move or rewrite. Each row cites where it lives today and, where the code says so, the ticket +that caused it. + +Not listed: behaviours of `.bloom-editable` handling that CKEditor is not involved in and that we +are not moving. `BloomField.ManageField` is called from `SetupElements` regardless of CKEditor, so +its arrow-key, backspace and paragraph-maintenance behaviours (BL-786, BL-933, BL-952, BL-2274, +BL-7061, BL-16518 …) stay exactly where they are. They are not in scope and listing them would +dilute the rows that matter. + +**A ticket number in the `Ticket` column means the code cites it**, not that we have read the +ticket. Where the code's account of a ticket looks wrong, the row says so. + +## How to use it + +- `Verify` says how each row gets checked: **unit** (vitest), **live** (running Bloom over CDP via + the `run-bloom` skill), or **manual** (a human tester; feed these to the `add-test-ideas` skill). +- Rows marked **⚠ capture first** must have today's actual behaviour recorded in a + `PASTE-DROP-BASELINE.md` *before* any code changes, because the row is a guarantee whose failure is + silent and the config string alone doesn't tell us what really happens. **That file does not exist + yet** — producing it is an open Stage 0 item (see PROGRESS.md), and deliberately so: it has to be + captured against a running Bloom, not written from the config. +- Rows marked **✗ must NOT survive** are current behaviour we intend to *remove*. They are here so + nobody faithfully reimplements a workaround for a problem that no longer exists. + +--- + +## A. Text selection toolbar (CKEditor service 1) + +| # | Behaviour | Today | Ticket | Verify | +|---|---|---|---|---| +| A1 | Selecting text in a `.bloom-editable` shows a floating toolbar; deselecting hides it | `attachToCkEditor` `selectionCheck` | | live | +| A2 | Toolbar offers bold, italic, underline, superscript, text colour, remove-formatting, hyperlink — and **not** cut/copy/paste/undo/redo/anchor/strike/subscript/background-colour | `config.js` `removeButtons` | | manual | +| A3 | Toolbar is positioned above the text box, moved down if it would go off-screen | `attachToCkEditor` `selectionCheck` | | manual | +| A4 | Toolbar never appears for a field with `bloom-userCannotModifyStyles` (on the field or an ancestor up to `.marginBox`), but such a field is still editable and pasteable | `attachToCkEditor` `alwaysHideToolbar` | BL-14947 | live | +| A5 | No toolbar for a field whose computed `cursor` is `not-allowed` | `attachToCkEditor` early return, `bloomEditing.ts:1952` | | live | +| A6 | No flash of the toolbar during page load, nor when moving between two fields that both have selections | `hideAllCKEditors` body class | BL-12448 | manual | +| A7 | Colour-picker panel does not re-open by itself on every subsequent selection | `attachToCkEditor` hides `.cke_panel` | | manual | +| A8 | Toolbar button tooltips are localized | `localizeCkeditorTooltips` | | manual | +| A9 | Text colour choices come from Bloom's text palette | `getHexColorsForPalette(BloomPalette.Text)` | | manual | + +**Deliberate improvement:** the new toolbar should use Bloom's existing `colorPickerDialog` instead +of CKEditor's colour panel (§4.5), which supersedes A7 and A9 rather than reproducing them. + +## B. Formatting commands + +| # | Behaviour | Today | Ticket | Verify | +|---|---|---|---|---| +| B1 | Bold / italic / underline / superscript apply and un-apply over a selection, including one spanning several paragraphs | CKEditor `basicstyles`; ``/``/``/`` | | unit + live | +| B2 | Bold produces ``, italic ``, underline ``, superscript `` — the tags the talking-book code expects to see inside a sentence | `audioRecording.ts:3727-3730` | | unit | +| B3 | Text colour produces a **bare** `` with no class or id | `config.js` colorbutton | | unit | +| B4 | Ctrl+Space ("clear formatting") strips only `b,strong,i,em,u,sup,sub,font,span` | `config.js removeFormatTags`, `bloomEditing.ts:291-299` | | unit | +| B5 | Clear-formatting **preserves any `` carrying a class or id** — audio segments (`audio-sentence`, `bloom-highlightSegment`) and `bloom-linebreak` | `attachToCkEditor` `addRemoveFormatFilter` | | unit | +| B6 | Clear-formatting does **not** strip `class`/`style`/`align`/`lang` from elements it keeps (e.g. paragraphs merely spanned by the selection) | `config.js removeFormatAttributes = ""` | | unit | +| B7 | F6 wraps the selection in ``; F7 → `

`; F8 → `

`; Ctrl+Alt+0 → `

`; Ctrl+Alt+1 → `

`; Ctrl+Alt+2 → `

` | `AddEditKeyHandlers` | | live | +| B8 | Ctrl+R / Ctrl+L / Ctrl+E right/left/centre justify | `AddEditKeyHandlers` | | live | +| B9 | Shift+Enter inserts `` plus a ZWNJ if needed, and leaves the caret **after** it | `BloomField.InsertLineBreak` | BL-3009 | unit + live | +| B10 | The caret is never left *inside* a `bloom-linebreak` span | `BloomField.EnsureCaretNotInsideLineBreakSpan`, on selection change | | unit | +| B11 | An **empty** `span.bloom-linebreak` survives round-tripping and is not stripped as an empty element | `CKEDITOR.dtd.$removeEmpty.span = 0` | BL-3009 | unit | +| B12 | Typing a URL then a space/enter turns it into a live link | `autolink` plugin | BL-6845 | live | +| B13 | `.bloom-editable` divs get `spellcheck="false"` — no red squiggles | `config.disableNativeSpellChecker` | BL-12205 | live | +| B14 | Hyperlink button opens the link-target chooser and wraps the selection in ``; failure on a complex selection shows the `EditTab.HyperlinkPasteFailure` message | `BloomField` `setupHyperlink` command | | manual | + +**Decision (§10 q2):** B14's dialog and flow are kept exactly as-is. + +## C. Paste and drop filtering — the silent-failure guarantee + +**⚠ Every row in this section must be captured first** (§4.8). The permitted vocabulary today is +`p br em i strong sup u; b{font-weight}; a[!href]; span{font-variant,color}` +(`config.js:119-120`); everything else, attributes included, is dropped. + +| # | Behaviour | Today | Ticket | Verify | +|---|---|---|---|---| +| C1 | Pasting a **table** does not produce a table | `pasteFilter` | | ⚠ capture, then unit + live | +| C2 | Pasting nested **`
`s** does not produce divs — importantly, a div copied from another Bloom book must not arrive with its **id**, which would duplicate an id in this book | `pasteFilter` | BL-3899 | ⚠ capture, then unit + live | +| C3 | Pasting an **`