From 13cc49e42232597b6e6816a5164b6dd57fc8055b Mon Sep 17 00:00:00 2001 From: Ethan James Date: Mon, 6 Jul 2026 14:24:08 -0700 Subject: [PATCH 1/3] fix(#3912): reflect formatting button state when moving cursor via bullet getCommandState treated the outer editable
wrapper (returned by selection.html() for a collapsed caret) as text, which cleared every command match and made the Bold/Italic/Underline/Strikethrough buttons report no formatting. Skip non-formatting tags instead, respecting quoted attribute values (e.g. placeholder="One") that may contain '>'. Adds a data-active hook to ToolbarButton, an e2e regression test, and unit tests for the editable-div-wrapped cases. --- src/components/ToolbarButton.tsx | 1 + src/e2e/puppeteer/__tests__/format.ts | 46 +++++++++++++++++++++ src/util/__tests__/getCommandState.ts | 57 +++++++++++++++++++++++++++ src/util/getCommandState.ts | 11 ++++++ 4 files changed, 115 insertions(+) diff --git a/src/components/ToolbarButton.tsx b/src/components/ToolbarButton.tsx index b5188cc7e98..4c6d5cd0e59 100644 --- a/src/components/ToolbarButton.tsx +++ b/src/components/ToolbarButton.tsx @@ -206,6 +206,7 @@ const ToolbarButton: FC = ({ {...longPress.props} aria-label={command.label} data-testid='toolbar-icon' + data-active={isButtonActive} ref={dndRef(node => dragSource(dropTarget(node)))} key={commandId} title={`${command.label}${(command.overlay?.keyboard ?? command.keyboard) ? ` (${formatKeyboardShortcut((command.overlay?.keyboard ?? command.keyboard)!)})` : ''}${buttonError ? '\nError: ' + buttonError : ''}`} diff --git a/src/e2e/puppeteer/__tests__/format.ts b/src/e2e/puppeteer/__tests__/format.ts index f2fe151f658..f8648a75977 100644 --- a/src/e2e/puppeteer/__tests__/format.ts +++ b/src/e2e/puppeteer/__tests__/format.ts @@ -55,3 +55,49 @@ it('Apply text color to an uppercase formatting tag', async () => { const result = await getEditingText() expect(result).toBe('HELLO WORLD') }) + +/** Returns whether the Bold toolbar button is rendered in its active state. */ +const isBoldButtonActive = () => + page.evaluate( + () => + document.querySelector('[data-testid="toolbar-icon"][aria-label="Bold"]')?.getAttribute('data-active') === 'true', + ) + +// Regression test for #3912: the Bold/Italic/Underline/Strikethrough buttons flickered back to their inactive +// state when the cursor was moved to a fully-formatted thought by tapping its bullet (Chrome/Android). Moving the +// cursor with a collapsed caret should reflect the whole-thought formatting, not a stale empty selection. +it('Bold button stays active when the cursor is moved to a fully-bold thought via its bullet (#3912)', async () => { + const importText = ` + - One + - Two` + + await paste(importText) + + // format the whole first thought as bold + await clickThought('One') + await click('[data-testid="toolbar-icon"][aria-label="Bold"]') + await waitForEditable('One') + + // move the cursor to the plain thought: the Bold button should be inactive + await clickThought('Two') + expect(await isBoldButtonActive()).toBe(false) + + // move the cursor back to the bold thought by tapping its bullet + const editableOne = await waitForEditable('One') + const bulletNode = await page.evaluateHandle(editableNode => { + if (!editableNode) throw new Error('Node handle does not contain a valid Element') + const thoughtContainer = editableNode.closest('[aria-label="thought-container"]') + if (!thoughtContainer) throw new Error('Thought container not found') + const bullet = thoughtContainer.querySelector('[aria-label="bullet"]') + if (!bullet) throw new Error('Bullet not found in thought container') + return bullet + }, editableOne) + // @ts-expect-error - https://github.com/puppeteer/puppeteer/issues/8852 + await bulletNode.asElement()?.click() + + // wait for the caret to settle at the start of the bold thought so the command state has updated + await page.waitForFunction(() => (window.getSelection()?.focusOffset ?? -1) === 0) + + // the Bold button should reflect the thought's bold formatting rather than flickering back to inactive + expect(await isBoldButtonActive()).toBe(true) +}) diff --git a/src/util/__tests__/getCommandState.ts b/src/util/__tests__/getCommandState.ts index 6b91351f485..7843f1abcc9 100644 --- a/src/util/__tests__/getCommandState.ts +++ b/src/util/__tests__/getCommandState.ts @@ -212,3 +212,60 @@ it('nested background colors should use the inner one', () => { backColor: 'rgb(0, 0, 255)', }) }) + +// selection.html() returns the outer editable
wrapper when the caret is collapsed on the editable element +// itself (e.g. when the cursor is moved to a thought by tapping its bullet). The wrapper must not clear the command +// state (#3912). +it('bold thought wrapped in the editable div', () => { + expect(getCommandState('
text
')).toStrictEqual({ + bold: true, + italic: false, + underline: false, + strikethrough: false, + code: false, + foreColor: undefined, + backColor: undefined, + }) +}) + +it('bold thought wrapped in the editable div with empty text', () => { + expect(getCommandState('
')).toStrictEqual({ + bold: true, + italic: false, + underline: false, + strikethrough: false, + code: false, + foreColor: undefined, + backColor: undefined, + }) +}) + +it('partially bold thought wrapped in the editable div', () => { + expect(getCommandState('
one two
')).toStrictEqual({ + bold: false, + italic: false, + underline: false, + strikethrough: false, + code: false, + foreColor: undefined, + backColor: undefined, + }) +}) + +// The editable div carries a placeholder attribute whose value contains raw HTML with '>' characters. Skipping +// the wrapper tag must respect quoted attribute values so the inner formatting is still parsed (#3912). +it('bold thought wrapped in the editable div with an html placeholder attribute', () => { + expect( + getCommandState( + '
One
', + ), + ).toStrictEqual({ + bold: true, + italic: false, + underline: false, + strikethrough: false, + code: false, + foreColor: undefined, + backColor: undefined, + }) +}) diff --git a/src/util/getCommandState.ts b/src/util/getCommandState.ts index 74da8968ea0..d593bc99bbd 100644 --- a/src/util/getCommandState.ts +++ b/src/util/getCommandState.ts @@ -115,6 +115,17 @@ const getCommandState = (value: string): CommandState => { if (foundTag) { continue } + // Skip any non-formatting tag (e.g. the outer editable
wrapper or a tag) rather than treating it + // as text. Absorbing the tag's characters as text would incorrectly clear the command matches, so that a + // fully-bold thought wrapped in its editable div (as returned by selection.html() for a collapsed caret) would + // report no formatting (#3912). Formatting tags are already handled above, so anything reaching here is a + // non-formatting tag that should not affect the command state. Quoted attribute values are consumed wholesale + // because they may themselves contain '>' (e.g. the editable div's placeholder="One" attribute). + const nonFormattingTag = value.match(/^<\/?[a-z](?:[^>"']|"[^"]*"|'[^']*')*>/i) + if (nonFormattingTag) { + value = value.substring(nonFormattingTag[0].length) + continue + } // Handle all of the subsequent non-tag characters const nonTagCharacters = value.match(/^[^<]+/) // Default to parsing a single `<` character if it has been determined to be a non-tag character From 7d7ce8b132938c6b68d43ac19b04b41b5f8505bb Mon Sep 17 00:00:00 2001 From: Ethan James Date: Mon, 6 Jul 2026 14:55:21 -0700 Subject: [PATCH 2/3] docs(test): explain why clickBullet helper is not reused for bold thoughts --- src/e2e/puppeteer/__tests__/format.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/e2e/puppeteer/__tests__/format.ts b/src/e2e/puppeteer/__tests__/format.ts index f8648a75977..757562a3a94 100644 --- a/src/e2e/puppeteer/__tests__/format.ts +++ b/src/e2e/puppeteer/__tests__/format.ts @@ -82,7 +82,11 @@ it('Bold button stays active when the cursor is moved to a fully-bold thought vi await clickThought('Two') expect(await isBoldButtonActive()).toBe(false) - // move the cursor back to the bold thought by tapping its bullet + // move the cursor back to the bold thought by tapping its bullet. + // NOTE: the clickBullet helper is not reused here because it locates the thought via getEditable, whose XPath + // matches on direct text nodes (contains(text(), …)). A fully-bold thought's text is wrapped in , so the + // editable div has no direct text node and getEditable finds nothing. waitForEditable matches on innerHTML, so + // the bullet is looked up from the One editable handle instead. const editableOne = await waitForEditable('One') const bulletNode = await page.evaluateHandle(editableNode => { if (!editableNode) throw new Error('Node handle does not contain a valid Element') From ec8eacb1c7a6cd8f9f4f25df13bc1cdc6acbad83 Mon Sep 17 00:00:00 2001 From: Ethan James Date: Tue, 7 Jul 2026 15:11:43 -0700 Subject: [PATCH 3/3] refactor(#3912): fix at the source in selection.html() instead of getCommandState Return the editable's inner HTML rather than its outer wrapper element when the caret is collapsed on the editable element (e.g. cursor moved via bullet tap), so selection.html() no longer emits the wrapper
and its placeholder attribute (which contains raw HTML). This also benefits the other html() callers (copy/cut/formatWithTag/Note). Reverts the getCommandState tag-skipping change, which is no longer needed. Adds a jsdom unit test for the html() collapsed-caret case. --- src/device/__tests__/selection.ts | 27 ++++++++++++- src/device/selection.ts | 6 ++- src/util/__tests__/getCommandState.ts | 57 --------------------------- src/util/getCommandState.ts | 11 ------ 4 files changed, 31 insertions(+), 70 deletions(-) diff --git a/src/device/__tests__/selection.ts b/src/device/__tests__/selection.ts index ad3c98c0c77..13d211658bf 100644 --- a/src/device/__tests__/selection.ts +++ b/src/device/__tests__/selection.ts @@ -1,5 +1,5 @@ import getTextContentFromHTML from '../../device/getTextContentFromHTML' -import { offsetFromClosestParent } from '../selection' +import { html, offsetFromClosestParent } from '../selection' /** Create dummy div with given html value. */ const createDummyDiv = (htmlValue: string) => { @@ -80,3 +80,28 @@ describe('offsetFromClosestParent', () => { }) }) }) + +describe('html', () => { + // When the caret is collapsed on the editable element itself (e.g. when the cursor is moved to a thought by + // tapping its bullet), html() must return the editable's contents, not its outer wrapper element with attributes + // such as placeholder="One" (#3912). + it('returns the editable inner html rather than the wrapper element for a collapsed caret on the editable', () => { + const editable = document.createElement('div') + editable.setAttribute('contenteditable', 'true') + editable.setAttribute('data-editable', 'true') + editable.setAttribute('placeholder', 'One') + editable.innerHTML = 'One' + document.body.appendChild(editable) + + const range = document.createRange() + range.setStart(editable, 0) + range.collapse(true) + const selection = window.getSelection()! + selection.removeAllRanges() + selection.addRange(range) + + expect(html()).toBe('') + + document.body.removeChild(editable) + }) +}) diff --git a/src/device/selection.ts b/src/device/selection.ts index 9ba8a3adfb5..3b466bc576d 100644 --- a/src/device/selection.ts +++ b/src/device/selection.ts @@ -468,7 +468,11 @@ export const html = () => { // Check if the node is an Element using the instanceof operator if (node instanceof Element) { - containerHtml = node.outerHTML + // When the caret is collapsed on the editable element itself (e.g. when the cursor is moved to a thought + // by tapping its bullet), return the editable's inner HTML rather than its outerHTML, so that the wrapper + // element and its attributes (such as placeholder="…", whose value contains raw HTML) are excluded + // from the selection html (#3912). + containerHtml = node.getAttribute('contenteditable') === 'true' ? node.innerHTML : node.outerHTML } else if (node instanceof CharacterData) { while (node.parentElement?.tagName !== 'DIV') { node = node.parentElement! diff --git a/src/util/__tests__/getCommandState.ts b/src/util/__tests__/getCommandState.ts index 7843f1abcc9..6b91351f485 100644 --- a/src/util/__tests__/getCommandState.ts +++ b/src/util/__tests__/getCommandState.ts @@ -212,60 +212,3 @@ it('nested background colors should use the inner one', () => { backColor: 'rgb(0, 0, 255)', }) }) - -// selection.html() returns the outer editable
wrapper when the caret is collapsed on the editable element -// itself (e.g. when the cursor is moved to a thought by tapping its bullet). The wrapper must not clear the command -// state (#3912). -it('bold thought wrapped in the editable div', () => { - expect(getCommandState('
text
')).toStrictEqual({ - bold: true, - italic: false, - underline: false, - strikethrough: false, - code: false, - foreColor: undefined, - backColor: undefined, - }) -}) - -it('bold thought wrapped in the editable div with empty text', () => { - expect(getCommandState('
')).toStrictEqual({ - bold: true, - italic: false, - underline: false, - strikethrough: false, - code: false, - foreColor: undefined, - backColor: undefined, - }) -}) - -it('partially bold thought wrapped in the editable div', () => { - expect(getCommandState('
one two
')).toStrictEqual({ - bold: false, - italic: false, - underline: false, - strikethrough: false, - code: false, - foreColor: undefined, - backColor: undefined, - }) -}) - -// The editable div carries a placeholder attribute whose value contains raw HTML with '>' characters. Skipping -// the wrapper tag must respect quoted attribute values so the inner formatting is still parsed (#3912). -it('bold thought wrapped in the editable div with an html placeholder attribute', () => { - expect( - getCommandState( - '
One
', - ), - ).toStrictEqual({ - bold: true, - italic: false, - underline: false, - strikethrough: false, - code: false, - foreColor: undefined, - backColor: undefined, - }) -}) diff --git a/src/util/getCommandState.ts b/src/util/getCommandState.ts index d593bc99bbd..74da8968ea0 100644 --- a/src/util/getCommandState.ts +++ b/src/util/getCommandState.ts @@ -115,17 +115,6 @@ const getCommandState = (value: string): CommandState => { if (foundTag) { continue } - // Skip any non-formatting tag (e.g. the outer editable
wrapper or a tag) rather than treating it - // as text. Absorbing the tag's characters as text would incorrectly clear the command matches, so that a - // fully-bold thought wrapped in its editable div (as returned by selection.html() for a collapsed caret) would - // report no formatting (#3912). Formatting tags are already handled above, so anything reaching here is a - // non-formatting tag that should not affect the command state. Quoted attribute values are consumed wholesale - // because they may themselves contain '>' (e.g. the editable div's placeholder="One" attribute). - const nonFormattingTag = value.match(/^<\/?[a-z](?:[^>"']|"[^"]*"|'[^']*')*>/i) - if (nonFormattingTag) { - value = value.substring(nonFormattingTag[0].length) - continue - } // Handle all of the subsequent non-tag characters const nonTagCharacters = value.match(/^[^<]+/) // Default to parsing a single `<` character if it has been determined to be a non-tag character