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/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/e2e/puppeteer/__tests__/format.ts b/src/e2e/puppeteer/__tests__/format.ts index f2fe151f658..757562a3a94 100644 --- a/src/e2e/puppeteer/__tests__/format.ts +++ b/src/e2e/puppeteer/__tests__/format.ts @@ -55,3 +55,53 @@ 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. + // 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') + 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) +})