From 392978e820f16b2bbce2b67080a90d6ba7551ded Mon Sep 17 00:00:00 2001 From: John Thomson Date: Tue, 8 Sep 2026 16:23:37 -0500 Subject: [PATCH 1/4] Add e2e test for text formatting shortcuts (Notion test case 364) The new spec drives the Edit tab's CKEditor formatting toolbar and its shortcuts on a two-paragraph text box: bold, italic, underline, superscript and text color go on words through the toolbar buttons and Ctrl+B/I/U, singly and in layers; Ctrl+A then Ctrl+Space takes every formatting off and the top bar's Undo button puts it back; the Remove Formatting button clears only the selected text and undo restores it. helpers/textFormatting.ts is the new surface module: keyboard selection of a stretch of text, the toolbar buttons and shortcuts, the text color palette, and reading a box back as formatted runs. bookMaking.ts gains typeParagraphsInGroup and workspace.ts gains clickUndoButton, the UI route beside the existing production-path undo. Bloom production code: EditingControlButton in editTopBarControls.tsx gets a data-testid per command so the Undo button can be found in any UI language. AUTOMATION-DEBT.md records that Ctrl+Z is a WinForms accelerator no test can press, and a new entry for the text color palette, which Bloom's selection-check hiding closes under the click about one run in two; the helper clicks again as a person would. Co-Authored-By: Claude Fable 5.1 --- .../bookEdit/topbar/editTopBarControls.tsx | 3 + src/BloomE2E/AUTOMATION-DEBT.md | 24 + src/BloomE2E/helpers/bookMaking.ts | 31 ++ src/BloomE2E/helpers/textFormatting.ts | 462 ++++++++++++++++++ src/BloomE2E/helpers/workspace.ts | 26 +- .../tests/text-formatting-shortcuts.spec.ts | 196 ++++++++ 6 files changed, 741 insertions(+), 1 deletion(-) create mode 100644 src/BloomE2E/helpers/textFormatting.ts create mode 100644 src/BloomE2E/tests/text-formatting-shortcuts.spec.ts diff --git a/src/BloomBrowserUI/bookEdit/topbar/editTopBarControls.tsx b/src/BloomBrowserUI/bookEdit/topbar/editTopBarControls.tsx index 055973d19efc..a1725caf1560 100644 --- a/src/BloomBrowserUI/bookEdit/topbar/editTopBarControls.tsx +++ b/src/BloomBrowserUI/bookEdit/topbar/editTopBarControls.tsx @@ -374,6 +374,9 @@ export const EditingControlButton: React.FunctionComponent<{ { // Keep focus in the main editable browser; otherwise this button takes focus // first and copy/cut/paste/undo may run against the wrong context. diff --git a/src/BloomE2E/AUTOMATION-DEBT.md b/src/BloomE2E/AUTOMATION-DEBT.md index 0e059b897de4..708860a8d39f 100644 --- a/src/BloomE2E/AUTOMATION-DEBT.md +++ b/src/BloomE2E/AUTOMATION-DEBT.md @@ -76,6 +76,14 @@ Format dialog, so the test drives it in the Format dialog (`helpers/fontChooser. Settings route stays manual. `settings/setFontForLanguage` is not a way round it: like the other settings endpoints it only records a pending change on the open dialog. +seen again 2026-09-08 (Test Case ID 364, `text-formatting-shortcuts.spec.ts`): the manual test undoes +with the Edit tab's Undo button and with Ctrl+Z. The button is in the React top bar now, so the test +clicks it (`helpers/workspace.ts clickUndoButton`, by a test id added for it). Ctrl+Z is still a +WinForms accelerator that the shell handles before the browser sees it, so no test can press it; the +test calls `undo`, the production path with only the key press missing, for that step. Fix +direction: an `e2e/` hook that runs the shell's own accelerator handling for a named key, so a test +can say "press Ctrl+Z" and have the shell answer as it does for a person. + ## Native OS dialogs hang automation File pickers, the Image Toolbox, and video capture open native windows Playwright @@ -497,6 +505,22 @@ So: **to test the working tree, serve the dev server on 5173 and set `BLOOM_E2E_VITE_PORT=5173`.** Fix direction: emit the port into those two pug files the way the shell gets it, so `--vite-port` means what it says. (Found 2026-09-02.) +## The text color palette sometimes does not open on the first click + +The formatting toolbar's Text Color button drops down a CKEditor panel of swatches. `attachToCkEditor` +in `bookEdit/js/bloomEditing.ts` hides every `.cke_panel` on every CKEditor `selectionCheck`, to stop +the palette from popping up on its own each time the toolbar shows (its comment says nobody knows +why it does). CKEditor checks the selection a moment after each mouseup, on a timer, so when that +check lands after the click has opened the panel, the panel is hidden again while the button still +shows as "on": the click looks as if it did nothing, and the next click closes the panel CKEditor +thinks is open rather than showing it. Seen 2026-09-08 (Test Case ID 364, +`text-formatting-shortcuts.spec.ts`): the second color pick of a run failed this way about one run in +two, the first never did. `helpers/textFormatting.ts pickTextColorFromToolbar` clicks again until +the panel is showing, which is what a person does. Fix direction: find out why the panel reappears +on its own (CKEditor's floatpanel remembers `showBlockParams`, and Bloom's `display:none` bypasses +its `hide`, so its state and the DOM disagree from then on) and hide it through `panel.hide()` +instead, or only on `selectionChange` rather than on every check. A person can hit this too. + ## Canvas element toolbar buttons are anonymous The floating toolbar over a selected canvas element (`#canvas-element-context-controls`, diff --git a/src/BloomE2E/helpers/bookMaking.ts b/src/BloomE2E/helpers/bookMaking.ts index 33d24856fd75..8ff5a4c17145 100644 --- a/src/BloomE2E/helpers/bookMaking.ts +++ b/src/BloomE2E/helpers/bookMaking.ts @@ -597,6 +597,37 @@ export async function typeInGroup( await expect(box).toHaveText(text, { timeout: 15000 }); } +/** + * Type several paragraphs into one language's box of one translation group, the way a person does: + * the first as typeInGroup types it, then Enter and the next, and so on. The box ends up holding one + *

per paragraph, which is what a test of anything paragraph-shaped needs to start from. + * + * As with typeInGroup, nothing reaches the file until the book leaves this page — see goToPage. + */ +export async function typeParagraphsInGroup( + page: Page, + groupSelector: string, + languageTag: string, + paragraphs: string[], +): Promise { + if (paragraphs.length === 0) + throw new Error("typeParagraphsInGroup needs at least one paragraph."); + await typeInGroup(page, groupSelector, languageTag, paragraphs[0]); + // typeInGroup leaves the caret at the end of what it typed, so each Enter starts a new paragraph + // after the last one. Enter is a real key press because CKEditor makes the paragraph from it. + for (const paragraph of paragraphs.slice(1)) { + await page.keyboard.press("Enter"); + await page.keyboard.insertText(paragraph); + } + const box = editablePageFrame(page) + .locator(`${groupSelector} .bloom-editable[lang="${languageTag}"]`) + .first(); + await expect( + box.locator(":scope > p"), + `The "${languageTag}" box of "${groupSelector}" did not end up with one paragraph per string typed.`, + ).toHaveText(paragraphs, { timeout: 15000 }); +} + /** * The font one language's box of one translation group is shown in: the first family of its computed * font-family, without quotes, e.g. "Andika". This is how a test checks that a font chosen in the diff --git a/src/BloomE2E/helpers/textFormatting.ts b/src/BloomE2E/helpers/textFormatting.ts new file mode 100644 index 000000000000..1a4f0a605942 --- /dev/null +++ b/src/BloomE2E/helpers/textFormatting.ts @@ -0,0 +1,462 @@ +// The Edit tab's direct-formatting toolbar and its keyboard shortcuts. +// +// Every text box on a page is a CKEditor surface, and CKEditor floats a small toolbar above the box +// while text in it is selected (see attachToCkEditor in bookEdit/js/bloomEditing.ts, which shows +// and hides it on CKEditor's selectionCheck). Bloom's toolbar has Bold, Italic, Underline, +// Superscript, a text-color palette, Remove Formatting, and a hyperlink button. Ctrl+B, Ctrl+I and +// Ctrl+U are CKEditor's own shortcuts for the first three; Ctrl+Space is Bloom's "clear formatting", +// which runs the same removeFormat command as the button (AddEditKeyHandlers in bloomEditing.ts). +// +// What the toolbar writes is CKEditor's core styles: , , , , and a bare +// for text color. getFormattedRuns reads the box back in those terms, so a +// test asserts "this word is bold and underlined" rather than on markup. +// +// Undo is NOT here. The Edit tab's Undo button is a WinForms toolbar button and Ctrl+Z is a WinForms +// accelerator, so neither can be pressed from a test; helpers/workspace.ts `undo` runs the production +// undo path the shell calls for both. (AUTOMATION-DEBT.md: "WinForms surfaces are invisible to CDP".) + +import { expect, type Locator, type Page } from "@playwright/test"; +import { editablePageFrame, clickInGroup } from "./bookMaking"; +import { pressKey } from "./keys"; + +/** A command of the formatting toolbar. Text color is separate (pickTextColorFromToolbar). */ +export type FormatCommand = + | "bold" + | "italic" + | "underline" + | "superscript" + | "removeFormat"; + +/** The character formatting of one run of text, as the toolbar can apply it. */ +export interface IFormatting { + bold: boolean; + italic: boolean; + underline: boolean; + superscript: boolean; + /** The text color as "#rrggbb" lower case, or absent when the run has the style's color. */ + color?: string; +} + +/** A stretch of text within one paragraph of a box whose formatting is the same throughout. */ +export interface IFormattedRun { + /** Which paragraph of the box the run is in, counting from 0. */ + paragraph: number; + text: string; + formatting: IFormatting; +} + +/** No character formatting at all: what freshly typed text has. */ +export const PLAIN: IFormatting = { + bold: false, + italic: false, + underline: false, + superscript: false, +}; + +/** The class CKEditor gives each toolbar button's anchor, e.g. cke_button__bold. */ +const BUTTON_CLASS: Record = { + bold: "cke_button__bold", + italic: "cke_button__italic", + underline: "cke_button__underline", + superscript: "cke_button__superscript", + removeFormat: "cke_button__removeformat", + textColor: "cke_button__textcolor", +}; + +/** The keyboard shortcut for each command that has one. Superscript has none. */ +const SHORTCUT: Partial> = { + bold: "Control+b", + italic: "Control+i", + underline: "Control+u", + removeFormat: "Control+Space", +}; + +/** One language's box of one translation group on the page being shown. */ +function boxOf( + page: Page, + groupSelector: string, + languageTag: string, +): Locator { + return editablePageFrame(page) + .locator(`${groupSelector} .bloom-editable[lang="${languageTag}"]`) + .first(); +} + +/** + * Select `text`, the first place it occurs in one language's box of one translation group, the way + * a person does with the keyboard: click just before its first character, then Shift+ArrowRight + * once per character. The text may run across formatted and unformatted stretches. Returns once the + * browser reports exactly that text selected, which is also when CKEditor has shown the formatting + * toolbar for it. + * + * Throws when the text is not in the box, or when the selection came out as something else. + */ +export async function selectTextInGroup( + page: Page, + groupSelector: string, + languageTag: string, + text: string, +): Promise { + if (!text) throw new Error("selectTextInGroup needs some text to select."); + const box = boxOf(page, groupSelector, languageTag); + await box.waitFor({ state: "visible", timeout: 30000 }); + + // Where the first character of the text is, relative to the box, so the click below lands + // just before it. The text can start in one text node and end in another, so this searches + // the box's text as a whole and maps the hit back to its node. + const start = await box.evaluate((element, wanted) => { + const walker = document.createTreeWalker(element, NodeFilter.SHOW_TEXT); + const nodes: Text[] = []; + let whole = ""; + let node: Node | null; + while ((node = walker.nextNode())) { + nodes.push(node as Text); + whole += node.textContent ?? ""; + } + const index = whole.indexOf(wanted); + if (index < 0) return { found: false as const, whole }; + let passed = 0; + for (const textNode of nodes) { + const length = textNode.textContent?.length ?? 0; + if (index < passed + length) { + const range = document.createRange(); + range.setStart(textNode, index - passed); + range.setEnd(textNode, index - passed + 1); + const rect = range.getBoundingClientRect(); + const boxRect = element.getBoundingClientRect(); + return { + found: true as const, + x: rect.left - boxRect.left + Math.min(2, rect.width / 3), + y: rect.top - boxRect.top + rect.height / 2, + }; + } + passed += length; + } + return { found: false as const, whole }; + }, text); + if (!start.found) + throw new Error( + `The "${languageTag}" box of "${groupSelector}" does not contain "${text}". ` + + `Its text is: "${start.whole}".`, + ); + + await box.click({ position: { x: start.x, y: start.y } }); + await expect( + box, + `Clicking in the "${languageTag}" box of "${groupSelector}" did not give it the focus.`, + ).toBeFocused({ timeout: 15000 }); + for (let i = 0; i < text.length; i++) { + await pressKey(page, "Shift+ArrowRight"); + } + + const selected = await editablePageFrame(page).evaluate( + () => document.getSelection()?.toString() ?? "", + ); + if (selected !== text) + throw new Error( + `Meant to select "${text}" but the browser reports "${selected}" selected.`, + ); + return box; +} + +/** + * Select everything in one language's box of one translation group, the way a person does: click + * in it and press Ctrl+A. Returns the box. + */ +export async function selectAllInGroup( + page: Page, + groupSelector: string, + languageTag: string, +): Promise { + const box = await clickInGroup(page, groupSelector, languageTag); + await box.press("Control+a"); + // The selection's text has a line break between paragraphs and textContent has none, so the + // two are compared with the whitespace taken out. + const withoutSpaces = (text: string) => text.replace(/\s/g, ""); + const expected = withoutSpaces((await box.textContent()) ?? ""); + await expect + .poll( + async () => + withoutSpaces( + await editablePageFrame(page).evaluate( + () => document.getSelection()?.toString() ?? "", + ), + ), + { + timeout: 15000, + message: `Ctrl+A did not select the whole text of the "${languageTag}" box of "${groupSelector}".`, + }, + ) + .toBe(expected); + return box; +} + +/** + * The formatting toolbar's button for a command. The toolbar exists once per box and shows only + * while text in that box is selected, so this is the one that is visible right now. + */ +function formatButton( + page: Page, + command: FormatCommand | "textColor", +): Locator { + return editablePageFrame(page).locator( + `a.${BUTTON_CLASS[command]}:visible`, + ); +} + +/** + * Click a button of the formatting toolbar that floats above the box with selected text: Bold, + * Italic, Underline, Superscript, or Remove Formatting. Select the text first + * (selectTextInGroup); the toolbar is not there otherwise, and this says so. + * + * Returns as soon as the click is delivered. CKEditor applies the style at once, but assert on the + * result with expectFormatting rather than reading the box straight away. + */ +export async function clickFormatButton( + page: Page, + command: FormatCommand | "textColor", +): Promise { + const button = formatButton(page, command); + try { + await button.waitFor({ state: "visible", timeout: 15000 }); + } catch { + throw new Error( + `The formatting toolbar is not showing its ${command} button. The toolbar appears ` + + `only while text in a box is selected; select some first.`, + ); + } + await button.click(); +} + +/** + * Press the keyboard shortcut for a formatting command into the box that has the focus: Ctrl+B, + * Ctrl+I, Ctrl+U, or Ctrl+Space for Remove Formatting. Superscript has no shortcut, and asking for + * one throws. + */ +export async function pressFormatShortcut( + page: Page, + command: FormatCommand, +): Promise { + const key = SHORTCUT[command]; + if (!key) throw new Error(`There is no keyboard shortcut for ${command}.`); + await pressKey(page, key); +} + +/** + * Give the selected text a color from the toolbar's palette, the way a person does: click the text + * color button, then click the swatch for `hex` ("#rrggbb", any case) in the palette that drops + * down. The swatches are the collection's text palette (TextColorPalette in + * react_components/color-picking/bloomPalette.ts, unless the collection has customized it). + * + * Throws, listing the swatches on offer, when the palette has no such color. + */ +export async function pickTextColorFromToolbar( + page: Page, + hex: string, +): Promise { + const code = hex.replace(/^#/, "").toLowerCase(); + if (!/^[0-9a-f]{6}$/.test(code)) + throw new Error(`Expected a color like "#ff1616", not "${hex}".`); + + // CKEditor renders the palette inside an iframe of its own within a drop-down panel. The + // panel does not reliably stay open after the click: Bloom hides every CKEditor panel whenever + // CKEditor checks the selection (attachToCkEditor in bloomEditing.ts), and that check runs on a + // timer a moment after the click, so the panel can vanish right after it opened, or under the + // swatch click, leaving the button "on" over a hidden panel. A person clicks the button again, + // and so does this: the next click closes the panel CKEditor thinks is open, the one after + // opens it. (AUTOMATION-DEBT.md: "The text color palette sometimes does not open on the first + // click".) + const frame = editablePageFrame(page); + const panel = frame.locator(".cke_panel:visible"); + const swatches = panel.frameLocator("iframe").locator("a.cke_colorbox"); + const swatch = swatches.filter({ + has: panel + .frameLocator("iframe") + .locator(`span.cke_colorbox[style*="${code}" i]`), + }); + for (let attempt = 1; attempt <= 6; attempt++) { + await clickFormatButton(page, "textColor"); + const opened = await panel + .waitFor({ state: "visible", timeout: 2000 }) + .then(() => true) + .catch(() => false); + if (!opened) continue; + // The palette is showing. Click the swatch straight away, before the selection check can + // hide the panel; if the panel goes anyway, the click times out and the loop tries again. + try { + await swatch.click({ timeout: 2000 }); + await panel.waitFor({ state: "hidden", timeout: 15000 }); + return; + } catch { + if (!(await panel.isVisible())) continue; + // The panel is still showing, so the swatch itself is what is missing. + const offered = await panel + .frameLocator("iframe") + .locator("span.cke_colorbox") + .evaluateAll((spans) => + spans + .map((s) => s.getAttribute("style") ?? "") + .filter((style) => style.includes("background-color")) + .map((style) => + style.replace(/.*#/, "#").replace(/[;\s]/g, ""), + ), + ); + throw new Error( + `The text color palette has no swatch for #${code}. It offers: ${offered.join(", ")}.`, + ); + } + } + throw new Error( + "The text color palette never stayed open long enough to pick a color, though the Text " + + "Color button was clicked six times.", + ); +} + +/** + * Read back the character formatting of one language's box of one translation group, paragraph by + * paragraph, as runs of text that share one formatting. Adjacent text with the same formatting is + * one run, so a word that is bold throughout is in one run whether CKEditor wrote it as one + * or two. + */ +export async function getFormattedRuns( + page: Page, + groupSelector: string, + languageTag: string, +): Promise { + const box = boxOf(page, groupSelector, languageTag); + await box.waitFor({ state: "visible", timeout: 30000 }); + return box.evaluate((element) => { + // The browser reports an inline color as rgb(); the palette and the tests speak hex. + const toHex = (css: string): string => { + const match = /^rgba?\((\d+),\s*(\d+),\s*(\d+)/.exec(css); + if (!match) return css.toLowerCase(); + return ( + "#" + + [match[1], match[2], match[3]] + .map((n) => Number(n).toString(16).padStart(2, "0")) + .join("") + ); + }; + const same = (a: IFormatting, b: IFormatting) => + a.bold === b.bold && + a.italic === b.italic && + a.underline === b.underline && + a.superscript === b.superscript && + a.color === b.color; + + const paragraphs = Array.from(element.querySelectorAll(":scope > p")); + const blocks: Element[] = paragraphs.length ? paragraphs : [element]; + const runs: IFormattedRun[] = []; + blocks.forEach((block, paragraph) => { + const walker = document.createTreeWalker( + block, + NodeFilter.SHOW_TEXT, + ); + let node: Node | null; + while ((node = walker.nextNode())) { + const text = node.textContent ?? ""; + if (!text) continue; + const formatting: IFormatting = { + bold: false, + italic: false, + underline: false, + superscript: false, + }; + for ( + let ancestor = node.parentElement; + ancestor && ancestor !== block; + ancestor = ancestor.parentElement + ) { + const tag = ancestor.tagName; + if (tag === "STRONG" || tag === "B") formatting.bold = true; + else if (tag === "EM" || tag === "I") + formatting.italic = true; + else if (tag === "U") formatting.underline = true; + else if (tag === "SUP") formatting.superscript = true; + if ( + tag === "SPAN" && + (ancestor as HTMLElement).style.color && + formatting.color === undefined + ) + formatting.color = toHex( + (ancestor as HTMLElement).style.color, + ); + } + const last = runs[runs.length - 1]; + if ( + last && + last.paragraph === paragraph && + same(last.formatting, formatting) + ) + last.text += text; + else runs.push({ paragraph, text, formatting }); + } + }); + return runs; + }); +} + +/** The runs, one line each, for a failure message. */ +export function describeRuns(runs: IFormattedRun[]): string { + if (runs.length === 0) return "(the box has no text)"; + return runs + .map((run) => { + const on: string[] = ( + ["bold", "italic", "underline", "superscript"] as const + ).filter((k) => run.formatting[k]); + if (run.formatting.color) on.push(run.formatting.color); + return ` ¶${run.paragraph} "${run.text}" [${on.join(", ") || "plain"}]`; + }) + .join("\n"); +} + +/** The text of each paragraph of the box, with the formatting left out. */ +export function paragraphTextsOf(runs: IFormattedRun[]): string[] { + const texts: string[] = []; + for (const run of runs) { + texts[run.paragraph] = (texts[run.paragraph] ?? "") + run.text; + } + return texts; +} + +/** + * The formatting of `text` in the box: the formatting of the one run that contains it. When no + * single run does, because the text is not there or is formatted differently along its length, the + * answer is a description of the runs instead, so that an assertion on it fails naming what the box + * holds. + */ +export function formattingOf( + runs: IFormattedRun[], + text: string, +): IFormatting | string { + const run = runs.find((r) => r.text.includes(text)); + if (!run) + return `no single run holds "${text}"; the runs are:\n${describeRuns(runs)}`; + return run.formatting; +} + +/** + * Assert, polling, that `text` in the box has exactly this formatting. Polled because a toolbar + * click or a shortcut is applied a moment after the event is delivered. + */ +export async function expectFormatting( + page: Page, + groupSelector: string, + languageTag: string, + text: string, + expected: IFormatting, +): Promise { + await expect + .poll( + async () => + formattingOf( + await getFormattedRuns(page, groupSelector, languageTag), + text, + ), + { + timeout: 15000, + message: `"${text}" does not have the expected formatting.`, + }, + ) + .toEqual(expected); +} diff --git a/src/BloomE2E/helpers/workspace.ts b/src/BloomE2E/helpers/workspace.ts index c8af7d97a396..166d0c13b761 100644 --- a/src/BloomE2E/helpers/workspace.ts +++ b/src/BloomE2E/helpers/workspace.ts @@ -154,6 +154,29 @@ export async function canUndo(page: Page): Promise { return answer === "yes"; } +/** + * Click the Edit tab's Undo button in the top bar, the way a person undoes with the mouse. This is + * the UI route to undo; `undo` below is the route for a test whose subject is not the button. + * + * The button posts editView/topBarButtonClick, and the shell answers by running the page bundle's + * `topBarButtonClick({ command: "undo" })` in the Edit tab's browser (EditingViewApi.cs), which is + * a different route to undo from the shell's own Ctrl+Z handling that `undo` reproduces. Like + * `undo`, this returns as soon as the click is delivered; wait for the state you expect rather + * than reading the page straight after it. Throws when Bloom has nothing to undo, because the + * button is disabled then and a click on it would do nothing. + */ +export async function clickUndoButton(page: Page): Promise { + if (!(await canUndo(page))) + throw new Error( + "Bloom says there is nothing to undo, so the Undo button is disabled. " + + "The change you meant to undo may not have registered.", + ); + // The test id is set in bookEdit/topbar/editTopBarControls.tsx (EditingControlButton). + const button = page.getByTestId("edit-top-bar-undo-button"); + await button.waitFor({ state: "visible", timeout: 30000 }); + await button.click(); +} + /** * Undo the last change. This returns as soon as the front end has been told to undo; what the undo * changes lands asynchronously, so wait for the state you expect (a text, a count, a class) @@ -164,7 +187,8 @@ export async function canUndo(page: Page): Promise { * `workspaceBundle.handleUndo()`, which is exactly what this calls. So this is the production undo * path with only the key press missing, and it covers CKEditor undo and the canvas element * manager's undo alike, because handleUndo is the code that chooses between them. - * (AUTOMATION-DEBT.md: "WinForms surfaces cannot be driven".) + * (AUTOMATION-DEBT.md: "WinForms surfaces cannot be driven".) A test whose subject is the Undo + * BUTTON clicks it with clickUndoButton instead. */ export async function undo(page: Page): Promise { if (!(await canUndo(page))) diff --git a/src/BloomE2E/tests/text-formatting-shortcuts.spec.ts b/src/BloomE2E/tests/text-formatting-shortcuts.spec.ts new file mode 100644 index 000000000000..189c4f766f95 --- /dev/null +++ b/src/BloomE2E/tests/text-formatting-shortcuts.spec.ts @@ -0,0 +1,196 @@ +// Character formatting in a text box: bold, italic, underline, superscript and text color, put on +// words through the formatting toolbar and through Ctrl+B, Ctrl+I and Ctrl+U; taken off again with +// Ctrl+Space and the Remove Formatting button; and put back by undo. Automates the manual test +// "Text Formatting Shortcuts" (Test Case ID 364). +// +// The tests are serial: each starts from the state the one before it left behind, and the second +// one builds the formatted text that the later ones take away and restore. +// +// The manual test undoes twice: once with the top bar's Undo button, which the test clicks for +// real, and once with Ctrl+Z, which is a WinForms accelerator the shell handles before the browser +// sees it, so no test can press it. For that step the test calls helpers/workspace.ts `undo`, the +// production undo path with only the key press missing (AUTOMATION-DEBT.md: "WinForms surfaces +// are invisible to CDP"). + +import type { Page } from "@playwright/test"; +import { expect, test } from "../fixtures/bloomTest"; +import { + addPage, + getContentPages, + goToPage, + makeBookFromTemplate, + typeParagraphsInGroup, +} from "../helpers/bookMaking"; +import { + clickFormatButton, + expectFormatting, + getFormattedRuns, + paragraphTextsOf, + pickTextColorFromToolbar, + pressFormatShortcut, + selectAllInGroup, + selectTextInGroup, + PLAIN, + type IFormatting, +} from "../helpers/textFormatting"; +import { clickUndoButton, undo } from "../helpers/workspace"; + +test.use({ + collectionSpec: { name: "text-formatting-shortcuts", languages: ["en"] }, +}); + +test.describe.configure({ mode: "serial" }); + +/** The one text box on the page this file builds: the box under the picture. */ +const TEXT_BOX = ".bloom-translationGroup"; +const LANGUAGE = "en"; + +/** Two paragraphs, every word different, so that each word names one place in the text. */ +const PARAGRAPHS = [ + "The quick brown fox jumps over the lazy dog.", + "Pack my box with five dozen liquor jugs.", +]; + +/** A color from Bloom's text palette (TextColorPalette in bloomPalette.ts). */ +const RED = "#ff1616"; + +/** + * How each formatted word gets its formatting, and what it should have afterwards. Between them the + * words use every toolbar button and every shortcut, singly and in layers, in both paragraphs. + */ +const FORMATTED_WORDS: { + word: string; + apply: (page: Page) => Promise; + expected: IFormatting; +}[] = [ + { + word: "quick", + apply: (page) => clickFormatButton(page, "bold"), + expected: { ...PLAIN, bold: true }, + }, + { + word: "brown", + apply: (page) => pressFormatShortcut(page, "italic"), + expected: { ...PLAIN, italic: true }, + }, + { + word: "fox", + apply: (page) => clickFormatButton(page, "underline"), + expected: { ...PLAIN, underline: true }, + }, + { + word: "jumps", + apply: (page) => clickFormatButton(page, "superscript"), + expected: { ...PLAIN, superscript: true }, + }, + { + word: "lazy", + apply: (page) => pickTextColorFromToolbar(page, RED), + expected: { ...PLAIN, color: RED }, + }, + { + word: "Pack", + apply: async (page) => { + await pressFormatShortcut(page, "bold"); + await clickFormatButton(page, "underline"); + }, + expected: { ...PLAIN, bold: true, underline: true }, + }, + { + word: "dozen", + apply: async (page) => { + await clickFormatButton(page, "italic"); + await pressFormatShortcut(page, "bold"); + await pressFormatShortcut(page, "underline"); + }, + expected: { ...PLAIN, bold: true, italic: true, underline: true }, + }, + { + word: "liquor", + apply: async (page) => { + await pickTextColorFromToolbar(page, RED); + await clickFormatButton(page, "superscript"); + }, + expected: { ...PLAIN, superscript: true, color: RED }, + }, +]; + +/** Words that are never formatted, one per paragraph, to show that formatting stays where it was put. */ +const PLAIN_WORDS = ["over", "with"]; + +/** Assert that every word has the formatting the table above gives it, and the plain words none. */ +const expectAllFormattingInPlace = async (page: Page) => { + for (const { word, expected } of FORMATTED_WORDS) + await expectFormatting(page, TEXT_BOX, LANGUAGE, word, expected); + for (const word of PLAIN_WORDS) + await expectFormatting(page, TEXT_BOX, LANGUAGE, word, PLAIN); +}; + +/** Assert that formatting has changed nothing about the words themselves. */ +const expectTextUnchanged = async (page: Page) => { + expect( + paragraphTextsOf(await getFormattedRuns(page, TEXT_BOX, LANGUAGE)), + ).toEqual(PARAGRAPHS); +}; + +test.describe("Text formatting shortcuts", () => { + test("builds a book with a text box holding two paragraphs", async ({ + page, + }) => { + test.setTimeout(300000); + await makeBookFromTemplate(page, "Basic Book"); + await addPage(page, "Basic Text & Image"); + const [textPage] = await getContentPages(page); + await goToPage(page, textPage.id); + await typeParagraphsInGroup(page, TEXT_BOX, LANGUAGE, PARAGRAPHS); + + // Sanity check the state the rest of the file rests on: two paragraphs, no formatting. + const runs = await getFormattedRuns(page, TEXT_BOX, LANGUAGE); + expect(paragraphTextsOf(runs)).toEqual(PARAGRAPHS); + expect(runs.map((run) => run.formatting)).toEqual([PLAIN, PLAIN]); + }); + + test("the toolbar buttons and Ctrl+B/I/U format the selected words, singly and in layers [Test Case ID 364]", async ({ + page, + }) => { + for (const { word, apply } of FORMATTED_WORDS) { + await selectTextInGroup(page, TEXT_BOX, LANGUAGE, word); + await apply(page); + } + await expectAllFormattingInPlace(page); + await expectTextUnchanged(page); + }); + + test("Ctrl+A then Ctrl+Space removes all the formatting, and the Undo button puts it all back [Test Case ID 364]", async ({ + page, + }) => { + await selectAllInGroup(page, TEXT_BOX, LANGUAGE); + await pressFormatShortcut(page, "removeFormat"); + for (const { word } of FORMATTED_WORDS) + await expectFormatting(page, TEXT_BOX, LANGUAGE, word, PLAIN); + await expectTextUnchanged(page); + + await clickUndoButton(page); + await expectAllFormattingInPlace(page); + await expectTextUnchanged(page); + }); + + test("the Remove Formatting button clears only the selected text, and undo (Ctrl+Z) puts it back [Test Case ID 364]", async ({ + page, + }) => { + // Two formatted words and the space between them: a selection that crosses formatting. + await selectTextInGroup(page, TEXT_BOX, LANGUAGE, "brown fox"); + await clickFormatButton(page, "removeFormat"); + await expectFormatting(page, TEXT_BOX, LANGUAGE, "brown fox", PLAIN); + for (const { word, expected } of FORMATTED_WORDS.filter( + (f) => f.word !== "brown" && f.word !== "fox", + )) + await expectFormatting(page, TEXT_BOX, LANGUAGE, word, expected); + await expectTextUnchanged(page); + + // The manual step is Ctrl+Z; see the note at the top of this file. + await undo(page); + await expectAllFormattingInPlace(page); + await expectTextUnchanged(page); + }); +}); From 663018c97caea011d9f2bafb28c4a5db421adeee Mon Sep 17 00:00:00 2001 From: John Thomson Date: Tue, 8 Sep 2026 16:24:42 -0500 Subject: [PATCH 2/4] Correct the textFormatting helper's note on where undo lives The Undo button is a React top-bar button that clickUndoButton clicks; only Ctrl+Z is the WinForms accelerator a test cannot press. Co-Authored-By: Claude Fable 5.1 --- src/BloomE2E/helpers/textFormatting.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/BloomE2E/helpers/textFormatting.ts b/src/BloomE2E/helpers/textFormatting.ts index 1a4f0a605942..63c8c3365151 100644 --- a/src/BloomE2E/helpers/textFormatting.ts +++ b/src/BloomE2E/helpers/textFormatting.ts @@ -11,9 +11,10 @@ // for text color. getFormattedRuns reads the box back in those terms, so a // test asserts "this word is bold and underlined" rather than on markup. // -// Undo is NOT here. The Edit tab's Undo button is a WinForms toolbar button and Ctrl+Z is a WinForms -// accelerator, so neither can be pressed from a test; helpers/workspace.ts `undo` runs the production -// undo path the shell calls for both. (AUTOMATION-DEBT.md: "WinForms surfaces are invisible to CDP".) +// Undo is NOT here: it belongs to the top bar, so helpers/workspace.ts has it. `clickUndoButton` +// clicks the Undo button; `undo` runs the production undo path for the Ctrl+Z step, because Ctrl+Z +// is a WinForms accelerator no test can press (AUTOMATION-DEBT.md: "WinForms surfaces are +// invisible to CDP"). import { expect, type Locator, type Page } from "@playwright/test"; import { editablePageFrame, clickInGroup } from "./bookMaking"; From 4144d98dc0d16df4ea215f114e9bb07ae5fe66f2 Mon Sep 17 00:00:00 2001 From: John Thomson Date: Tue, 8 Sep 2026 18:51:11 -0500 Subject: [PATCH 3/4] Address Devin review: grapheme-aware selection, helpers that wait for their result selectTextInGroup now presses Shift+ArrowRight once per grapheme (Intl.Segmenter) rather than per UTF-16 code unit, so an emoji or a combining sequence is one caret step as the browser counts it. clickFormatButton and pressFormatShortcut wait for the command to land: a style command until its toolbar button toggles, Remove Formatting until the selection is plain. pickTextColorFromToolbar waits out CKEditor's selection-check throttle after the palette opens before clicking a swatch, because a click into a panel that closes at that moment landed on the text and moved the selection; it now fails if the selection moved. AUTOMATION-DEBT.md records that. Co-Authored-By: Claude Fable 5.1 --- src/BloomE2E/AUTOMATION-DEBT.md | 8 +- src/BloomE2E/helpers/textFormatting.ts | 147 +++++++++++++++++++++---- 2 files changed, 134 insertions(+), 21 deletions(-) diff --git a/src/BloomE2E/AUTOMATION-DEBT.md b/src/BloomE2E/AUTOMATION-DEBT.md index 708860a8d39f..0100674cbe35 100644 --- a/src/BloomE2E/AUTOMATION-DEBT.md +++ b/src/BloomE2E/AUTOMATION-DEBT.md @@ -515,8 +515,12 @@ check lands after the click has opened the panel, the panel is hidden again whil shows as "on": the click looks as if it did nothing, and the next click closes the panel CKEditor thinks is open rather than showing it. Seen 2026-09-08 (Test Case ID 364, `text-formatting-shortcuts.spec.ts`): the second color pick of a run failed this way about one run in -two, the first never did. `helpers/textFormatting.ts pickTextColorFromToolbar` clicks again until -the panel is showing, which is what a person does. Fix direction: find out why the panel reappears +two, the first never did. Worse, a swatch click delivered into a panel that closed at that moment +landed on the text beneath it and moved the selection, so the color went on half a word. +`helpers/textFormatting.ts pickTextColorFromToolbar` therefore waits out CKEditor's 200ms +selection-check throttle after the panel opens, clicks a swatch only in a panel that is still +showing, clicks the button again otherwise (which is what a person does), and fails if the +selection moved. Fix direction: find out why the panel reappears on its own (CKEditor's floatpanel remembers `showBlockParams`, and Bloom's `display:none` bypasses its `hide`, so its state and the DOM disagree from then on) and hide it through `panel.hide()` instead, or only on `selectionChange` rather than on every check. A person can hit this too. diff --git a/src/BloomE2E/helpers/textFormatting.ts b/src/BloomE2E/helpers/textFormatting.ts index 63c8c3365151..cf2c80361b26 100644 --- a/src/BloomE2E/helpers/textFormatting.ts +++ b/src/BloomE2E/helpers/textFormatting.ts @@ -146,7 +146,14 @@ export async function selectTextInGroup( box, `Clicking in the "${languageTag}" box of "${groupSelector}" did not give it the focus.`, ).toBeFocused({ timeout: 15000 }); - for (let i = 0; i < text.length; i++) { + // One press per character as the caret counts them: an emoji, or a letter with its combining + // accents, is one caret step but several UTF-16 code units, so text.length would overrun. + const graphemes = [ + ...new Intl.Segmenter(undefined, { granularity: "grapheme" }).segment( + text, + ), + ].length; + for (let i = 0; i < graphemes; i++) { await pressKey(page, "Shift+ArrowRight"); } @@ -206,17 +213,13 @@ function formatButton( } /** - * Click a button of the formatting toolbar that floats above the box with selected text: Bold, - * Italic, Underline, Superscript, or Remove Formatting. Select the text first - * (selectTextInGroup); the toolbar is not there otherwise, and this says so. - * - * Returns as soon as the click is delivered. CKEditor applies the style at once, but assert on the - * result with expectFormatting rather than reading the box straight away. + * The formatting toolbar's button for a command, waited for. Throws, saying why, when the toolbar + * is not showing, which is what happens when no text is selected. */ -export async function clickFormatButton( +async function visibleFormatButton( page: Page, command: FormatCommand | "textColor", -): Promise { +): Promise { const button = formatButton(page, command); try { await button.waitFor({ state: "visible", timeout: 15000 }); @@ -226,13 +229,96 @@ export async function clickFormatButton( `only while text in a box is selected; select some first.`, ); } + return button; +} + +/** + * Whether the toolbar shows a style button as "on", which CKEditor does when the selected text + * already has that style. Only the four style buttons have such a state. + */ +async function isFormatButtonOn( + page: Page, + command: FormatCommand, +): Promise { + const classes = + (await formatButton(page, command).getAttribute("class")) ?? ""; + return classes.split(/\s+/).includes("cke_button_on"); +} + +/** + * Whether the current selection has none of the formatting the toolbar can put on text: nothing + * formatted inside it, and no formatting element around it either. + */ +async function isSelectionPlain(page: Page): Promise { + return editablePageFrame(page).evaluate(() => { + const selection = document.getSelection(); + if (!selection || selection.rangeCount === 0) return false; + const range = selection.getRangeAt(0); + const formatted = "strong,b,em,i,u,sup,span[style]"; + if (range.cloneContents().querySelector(formatted)) return false; + const around = + range.commonAncestorContainer instanceof Element + ? range.commonAncestorContainer + : range.commonAncestorContainer.parentElement; + return !around?.closest(formatted); + }); +} + +/** + * Wait for a formatting command to have taken effect on the selection. A style command toggles + * its toolbar button, so this waits for the button to leave the state it was in before; Remove + * Formatting has no button state, so this waits for the selection to be plain. + */ +async function waitForFormatCommand( + page: Page, + command: FormatCommand, + buttonWasOn: boolean, +): Promise { + if (command === "removeFormat") { + await expect + .poll(() => isSelectionPlain(page), { + timeout: 15000, + message: + "Remove Formatting left formatting on the selected text.", + }) + .toBe(true); + return; + } + await expect + .poll(() => isFormatButtonOn(page, command), { + timeout: 15000, + message: `The ${command} button never changed state, so the command did not land.`, + }) + .toBe(!buttonWasOn); +} + +/** + * Click a button of the formatting toolbar that floats above the box with selected text: Bold, + * Italic, Underline, Superscript, or Remove Formatting. Select the text first + * (selectTextInGroup); the toolbar is not there otherwise, and this says so. + * + * Returns once the command has taken effect on the selection (see waitForFormatCommand). The + * Text Color button only opens the palette; pickTextColorFromToolbar drives that. + */ +export async function clickFormatButton( + page: Page, + command: FormatCommand | "textColor", +): Promise { + const button = await visibleFormatButton(page, command); + if (command === "textColor") { + await button.click(); + return; + } + const wasOn = await isFormatButtonOn(page, command); await button.click(); + await waitForFormatCommand(page, command, wasOn); } /** * Press the keyboard shortcut for a formatting command into the box that has the focus: Ctrl+B, * Ctrl+I, Ctrl+U, or Ctrl+Space for Remove Formatting. Superscript has no shortcut, and asking for - * one throws. + * one throws. Returns once the command has taken effect on the selection, read from the same + * toolbar the buttons live on, so the text must be selected here too. */ export async function pressFormatShortcut( page: Page, @@ -240,7 +326,10 @@ export async function pressFormatShortcut( ): Promise { const key = SHORTCUT[command]; if (!key) throw new Error(`There is no keyboard shortcut for ${command}.`); + await visibleFormatButton(page, command); + const wasOn = await isFormatButtonOn(page, command); await pressKey(page, key); + await waitForFormatCommand(page, command, wasOn); } /** @@ -275,6 +364,11 @@ export async function pickTextColorFromToolbar( .frameLocator("iframe") .locator(`span.cke_colorbox[style*="${code}" i]`), }); + const selectedText = () => + editablePageFrame(page).evaluate( + () => document.getSelection()?.toString() ?? "", + ); + const selectedBefore = await selectedText(); for (let attempt = 1; attempt <= 6; attempt++) { await clickFormatButton(page, "textColor"); const opened = await panel @@ -282,15 +376,14 @@ export async function pickTextColorFromToolbar( .then(() => true) .catch(() => false); if (!opened) continue; - // The palette is showing. Click the swatch straight away, before the selection check can - // hide the panel; if the panel goes anyway, the click times out and the loop tries again. - try { - await swatch.click({ timeout: 2000 }); - await panel.waitFor({ state: "hidden", timeout: 15000 }); - return; - } catch { - if (!(await panel.isVisible())) continue; - // The panel is still showing, so the swatch itself is what is missing. + // CKEditor checks the selection at most 200ms after the last mouse or key event (its + // checkSelectionChange throttle), and that is the check in which Bloom may hide the panel + // that has just opened. A swatch click delivered into a panel that closes at that moment + // lands on the text beneath it and moves the selection, which is worse than a retry. So + // wait out that one check, and click only a panel that is still showing afterwards. + await page.waitForTimeout(kSelectionCheckMs); + if (!(await panel.isVisible())) continue; + if ((await swatch.count()) !== 1) { const offered = await panel .frameLocator("iframe") .locator("span.cke_colorbox") @@ -306,6 +399,15 @@ export async function pickTextColorFromToolbar( `The text color palette has no swatch for #${code}. It offers: ${offered.join(", ")}.`, ); } + await swatch.click(); + await panel.waitFor({ state: "hidden", timeout: 15000 }); + const selectedAfter = await selectedText(); + if (selectedAfter !== selectedBefore) + throw new Error( + `Picking a text color changed the selection from "${selectedBefore}" to ` + + `"${selectedAfter}", so the click did not land on the palette.`, + ); + return; } throw new Error( "The text color palette never stayed open long enough to pick a color, though the Text " + @@ -313,6 +415,13 @@ export async function pickTextColorFromToolbar( ); } +/** + * How long CKEditor can take to run its selection check after a mouse or key event: it throttles + * checkSelectionChange to one per 200ms. pickTextColorFromToolbar waits this long after opening + * the palette, because the check that follows the click is the one in which Bloom may close it. + */ +const kSelectionCheckMs = 250; + /** * Read back the character formatting of one language's box of one translation group, paragraph by * paragraph, as runs of text that share one formatting. Adjacent text with the same formatting is From b62e37ba6db6acbd93c9e5f2b22726d26b14bb4c Mon Sep 17 00:00:00 2001 From: John Thomson Date: Thu, 10 Sep 2026 13:21:45 -0500 Subject: [PATCH 4/4] Press Ctrl+Z for real in the text formatting test The shell lets Ctrl+Z through to the page, where CKEditor's undo plugin handles it, so the test presses the key (workspace.ts pressUndoKey) instead of calling the front end's undo dispatcher. The claims that Ctrl+Z was a WinForms accelerator are gone from keys.ts, the helper and AUTOMATION-DEBT.md. Co-Authored-By: Claude Fable 5.1 --- src/BloomE2E/AUTOMATION-DEBT.md | 8 ------ src/BloomE2E/helpers/keys.ts | 6 ++-- src/BloomE2E/helpers/textFormatting.ts | 6 ++-- src/BloomE2E/helpers/workspace.ts | 28 ++++++++++++------- .../tests/text-formatting-shortcuts.spec.ts | 11 +++----- 5 files changed, 27 insertions(+), 32 deletions(-) diff --git a/src/BloomE2E/AUTOMATION-DEBT.md b/src/BloomE2E/AUTOMATION-DEBT.md index 0100674cbe35..3504eb58d580 100644 --- a/src/BloomE2E/AUTOMATION-DEBT.md +++ b/src/BloomE2E/AUTOMATION-DEBT.md @@ -76,14 +76,6 @@ Format dialog, so the test drives it in the Format dialog (`helpers/fontChooser. Settings route stays manual. `settings/setFontForLanguage` is not a way round it: like the other settings endpoints it only records a pending change on the open dialog. -seen again 2026-09-08 (Test Case ID 364, `text-formatting-shortcuts.spec.ts`): the manual test undoes -with the Edit tab's Undo button and with Ctrl+Z. The button is in the React top bar now, so the test -clicks it (`helpers/workspace.ts clickUndoButton`, by a test id added for it). Ctrl+Z is still a -WinForms accelerator that the shell handles before the browser sees it, so no test can press it; the -test calls `undo`, the production path with only the key press missing, for that step. Fix -direction: an `e2e/` hook that runs the shell's own accelerator handling for a named key, so a test -can say "press Ctrl+Z" and have the shell answer as it does for a person. - ## Native OS dialogs hang automation File pickers, the Image Toolbox, and video capture open native windows Playwright diff --git a/src/BloomE2E/helpers/keys.ts b/src/BloomE2E/helpers/keys.ts index c8cd287c19d5..4161626cd22d 100644 --- a/src/BloomE2E/helpers/keys.ts +++ b/src/BloomE2E/helpers/keys.ts @@ -11,9 +11,9 @@ // text box raises no key events".) // // Every press goes through Playwright's keyboard, which sends the same CDP raw key events the -// browser would build from a physical key. What it cannot do is send a key that Bloom's WinForms -// shell claims as an accelerator: Ctrl+Z never reaches the page at all, which is why undo has its -// own helper in workspace.ts rather than a press here. +// browser would build from a physical key. That includes Ctrl+Z: the shell lets it through to the +// page, where CKEditor's undo plugin handles it (workspace.ts `pressUndoKey` is that press, beside +// the other undo routes). import { expect, type Locator, type Page } from "@playwright/test"; diff --git a/src/BloomE2E/helpers/textFormatting.ts b/src/BloomE2E/helpers/textFormatting.ts index cf2c80361b26..5104ccda1a4d 100644 --- a/src/BloomE2E/helpers/textFormatting.ts +++ b/src/BloomE2E/helpers/textFormatting.ts @@ -11,10 +11,8 @@ // for text color. getFormattedRuns reads the box back in those terms, so a // test asserts "this word is bold and underlined" rather than on markup. // -// Undo is NOT here: it belongs to the top bar, so helpers/workspace.ts has it. `clickUndoButton` -// clicks the Undo button; `undo` runs the production undo path for the Ctrl+Z step, because Ctrl+Z -// is a WinForms accelerator no test can press (AUTOMATION-DEBT.md: "WinForms surfaces are -// invisible to CDP"). +// Undo is NOT here: it belongs to the workspace, so helpers/workspace.ts has it. `clickUndoButton` +// clicks the top bar's Undo button, `pressUndoKey` presses Ctrl+Z, and `undo` is the setup route. import { expect, type Locator, type Page } from "@playwright/test"; import { editablePageFrame, clickInGroup } from "./bookMaking"; diff --git a/src/BloomE2E/helpers/workspace.ts b/src/BloomE2E/helpers/workspace.ts index 166d0c13b761..8b65ad092c42 100644 --- a/src/BloomE2E/helpers/workspace.ts +++ b/src/BloomE2E/helpers/workspace.ts @@ -178,17 +178,25 @@ export async function clickUndoButton(page: Page): Promise { } /** - * Undo the last change. This returns as soon as the front end has been told to undo; what the undo - * changes lands asynchronously, so wait for the state you expect (a text, a count, a class) - * rather than reading the page straight after this. + * Press Ctrl+Z, the way a person undoes from the keyboard, into whatever has the focus. In a text + * box the key goes to CKEditor's own undo plugin; the shell does not claim it (Shell.ProcessCmdKey + * only raises an event and lets the key through, and the C# UndoCommand's implementer is empty). + * Like the other undo routes this returns as soon as the key is delivered; wait for the state you + * expect rather than reading the page straight after it. + */ +export async function pressUndoKey(page: Page): Promise { + await page.keyboard.press("Control+z"); +} + +/** + * Undo the last change through the front end's own undo dispatcher, `workspaceBundle.handleUndo()`, + * which is the code the Undo button ends in and which chooses between CKEditor undo, origami undo + * and the canvas element manager's undo. This is the SETUP route to an undo; a test whose subject + * is undo itself clicks the button (clickUndoButton) or presses the key (pressUndoKey). * - * Ctrl+Z in the Edit tab is a WinForms accelerator: the key press never reaches the browser, so a - * test cannot send it. What the shell does when the key is pressed is call the front end's - * `workspaceBundle.handleUndo()`, which is exactly what this calls. So this is the production undo - * path with only the key press missing, and it covers CKEditor undo and the canvas element - * manager's undo alike, because handleUndo is the code that chooses between them. - * (AUTOMATION-DEBT.md: "WinForms surfaces cannot be driven".) A test whose subject is the Undo - * BUTTON clicks it with clickUndoButton instead. + * Returns as soon as the front end has been told to undo; what the undo changes lands + * asynchronously, so wait for the state you expect (a text, a count, a class) rather than reading + * the page straight after this. */ export async function undo(page: Page): Promise { if (!(await canUndo(page))) diff --git a/src/BloomE2E/tests/text-formatting-shortcuts.spec.ts b/src/BloomE2E/tests/text-formatting-shortcuts.spec.ts index 189c4f766f95..352767ba9a46 100644 --- a/src/BloomE2E/tests/text-formatting-shortcuts.spec.ts +++ b/src/BloomE2E/tests/text-formatting-shortcuts.spec.ts @@ -7,10 +7,8 @@ // one builds the formatted text that the later ones take away and restore. // // The manual test undoes twice: once with the top bar's Undo button, which the test clicks for -// real, and once with Ctrl+Z, which is a WinForms accelerator the shell handles before the browser -// sees it, so no test can press it. For that step the test calls helpers/workspace.ts `undo`, the -// production undo path with only the key press missing (AUTOMATION-DEBT.md: "WinForms surfaces -// are invisible to CDP"). +// real, and once with Ctrl+Z, which the test presses for real; CKEditor's own undo plugin handles +// the key. import type { Page } from "@playwright/test"; import { expect, test } from "../fixtures/bloomTest"; @@ -33,7 +31,7 @@ import { PLAIN, type IFormatting, } from "../helpers/textFormatting"; -import { clickUndoButton, undo } from "../helpers/workspace"; +import { clickUndoButton, pressUndoKey } from "../helpers/workspace"; test.use({ collectionSpec: { name: "text-formatting-shortcuts", languages: ["en"] }, @@ -188,8 +186,7 @@ test.describe("Text formatting shortcuts", () => { await expectFormatting(page, TEXT_BOX, LANGUAGE, word, expected); await expectTextUnchanged(page); - // The manual step is Ctrl+Z; see the note at the top of this file. - await undo(page); + await pressUndoKey(page); await expectAllFormattingInPlace(page); await expectTextUnchanged(page); });