diff --git a/core/src/components/select/select.tsx b/core/src/components/select/select.tsx index c870291ec4c..7c3f7f7477f 100644 --- a/core/src/components/select/select.tsx +++ b/core/src/components/select/select.tsx @@ -15,7 +15,7 @@ import { printIonWarning } from '@utils/logging'; import { actionSheetController, alertController, popoverController, modalController } from '@utils/overlays'; import type { OverlaySelect } from '@utils/overlays-interface'; import { isRTL } from '@utils/rtl'; -import { reflectPropertiesToAttributes, sanitizeDOMTree } from '@utils/sanitization'; +import { blockedTags, reflectPropertiesToAttributes, sanitizeDOMTree } from '@utils/sanitization'; import { createSlotMutationController } from '@utils/slot-mutation-controller'; import type { SlotMutationController } from '@utils/slot-mutation-controller'; import { createColorClasses, hostContext } from '@utils/theme'; @@ -1481,33 +1481,14 @@ const textForValue = ( } /** - * When custom HTML is enabled, extract only the default slot content. - * This ensures aria-label and other text-only contexts read only - * the relevant option text. + * Every text-only context reads only the default slot, so the start + * and end slots stay out of the `aria-label` and the overlay labels. + * Both config paths derive that text through the same helper, so they + * cannot disagree about what an option's text is. `null` marks an + * option with no text, which is dropped from the joined text of a + * `multiple` select rather than joined in as an empty entry. */ - if (customHTMLEnabled) { - const content = getOptionContent(selectOpt); - - if (typeof content === 'string') { - return content; - } - - /** - * Elements were found in the default slot, extract and concatenate - * their text content while trimming whitespace. - */ - if (content) { - const texts = Array.from(content.childNodes) - .map((n) => n.textContent?.trim()) - .filter((t) => t); - return texts.join(' ') || null; - } - - // Empty option - return null; - } - - return getDefaultSlotPlainText(selectOpt); + return getDefaultSlotPlainText(selectOpt) || null; }; /** @@ -1570,9 +1551,13 @@ const getOptionContent = ( return null; } - // Return plain text if no elements are found + /** + * Return plain text if no elements are found. This reads the option the + * same way the non-custom-HTML path does, so the two do not disagree + * about what an option's text is. + */ if (!slotName && nodes.every((n) => n.nodeType === Node.TEXT_NODE)) { - return nodes.map((n) => n.textContent?.trim()).join(' ') || null; + return getDefaultSlotPlainText(option) || null; } /** @@ -1636,22 +1621,47 @@ const getOptionDefaultSlot = (option: HTMLIonSelectOptionElement): Node[] | null return defaultSlotNodes; }; +/** + * Concatenates the text a node renders, skipping the subtrees of tags + * whose contents the browser never paints (`script`, `style`, and the + * rest of `blockedTags`). `textContent` includes those, so reading it + * directly would put stylesheet or script source into the select text + * and the `aria-label`. + * + * @param node - The node to read text from. + * @returns The node's rendered text. + */ +const getRenderedTextContent = (node: Node): string => { + if (node.nodeType === Node.TEXT_NODE) { + return node.textContent ?? ''; + } + + if (node.nodeType !== Node.ELEMENT_NODE) { + return ''; + } + + if (blockedTags.includes((node as Element).tagName.toLowerCase())) { + return ''; + } + + return Array.from(node.childNodes) + .map((child) => getRenderedTextContent(child)) + .join(''); +}; + /** * Extracts plain text from only the default slot of an option, - * excluding content assigned to named slots (start/end). + * excluding content assigned to named slots (start/end). Text is + * concatenated with no separator and collapsible whitespace is + * collapsed, approximating how the browser renders the option. + * NBSP is not collapsible, so it is preserved. + * + * @param option - The `ion-select-option` element to read text from. + * @returns The option's default slot text. */ const getDefaultSlotPlainText = (option: HTMLIonSelectOptionElement): string => { - const texts = Array.from(option.childNodes) - .filter((node) => { - if (node.nodeType === Node.ELEMENT_NODE) { - return !(node as HTMLElement).hasAttribute('slot'); - } - return node.nodeType === Node.TEXT_NODE; - }) - .filter((node) => node.nodeType === Node.TEXT_NODE) - .map((n) => n.textContent?.trim()) - .filter((t) => t); - return texts.join(' '); + const text = (getOptionDefaultSlot(option) ?? []).map((node) => getRenderedTextContent(node)).join(''); + return text.replace(/[ \t\n\r\f]+/g, ' ').replace(/^[ \t\n\r\f]+|[ \t\n\r\f]+$/g, ''); }; /** diff --git a/core/src/components/select/test/rich-content-option/select.e2e.ts b/core/src/components/select/test/rich-content-option/select.e2e.ts index 3acf2d806ac..fa29d7e49e5 100644 --- a/core/src/components/select/test/rich-content-option/select.e2e.ts +++ b/core/src/components/select/test/rich-content-option/select.e2e.ts @@ -430,7 +430,7 @@ configs({ modes: ['md'] }).forEach(({ title, config }) => { */ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { test.describe(title('select: rich content options'), () => { - test('it should only render text nodes when `innerHTMLTemplatesEnabled` is disabled', async ({ page }) => { + test('should not render markup when `innerHTMLTemplatesEnabled` is disabled', async ({ page }) => { await page.setContent( ` @@ -466,6 +466,12 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { await expect(endContainer).toHaveCount(0); await expect(span).toHaveCount(0); + /** + * The span is not rendered, but the text it wrapped still reads as + * text, so the option is not silently emptied out. + */ + await expect(firstOption).toContainText('Full Content This is a span element'); + // Click on the first option await firstOption.click(); @@ -479,6 +485,12 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { const selectTextSpan = selectText.locator('.span-style'); await expect(selectTextSpan).toHaveCount(0); + + /** + * Only the default slot is read, so the text of the `start` and `end` + * slots stays out of the selected text. + */ + await expect(selectText).toHaveText('Full Content This is a span element'); }); }); }); diff --git a/core/src/components/select/test/select.spec.tsx b/core/src/components/select/test/select.spec.tsx index ad7c0d3d050..f214e289501 100644 --- a/core/src/components/select/test/select.spec.tsx +++ b/core/src/components/select/test/select.spec.tsx @@ -1,5 +1,6 @@ import { h } from '@stencil/core'; import { newSpecPage } from '@stencil/core/testing'; +import { alertController } from '@utils/overlays'; import { config } from '../../../global/config'; import { SelectOption } from '../../select-option/select-option'; @@ -160,6 +161,259 @@ describe('ion-select: required', () => { }); }); +describe('ion-select: option plain text', () => { + it('should not insert a space between adjacent text nodes in an option', async () => { + const page = await newSpecPage({ + components: [Select, SelectOption], + html: ``, + }); + + const select = page.body.querySelector('ion-select')!; + + appendAdjacentTextNodes(select.querySelector('ion-select-option')!); + + select.value = 'star'; + await page.waitForChanges(); + + expect(select.shadowRoot!.querySelector('.select-text')!.textContent).toBe('★Star'); + expect(select.shadowRoot!.querySelector('button')!.getAttribute('aria-label')).toBe('★Star'); + }); + + it('should read option text that is wrapped in an element', async () => { + const page = await newSpecPage({ + components: [Select, SelectOption], + html: `A Star`, + }); + + const select = page.body.querySelector('ion-select')!; + await page.waitForChanges(); + + expect(select.shadowRoot!.querySelector('.select-text')!.textContent).toBe('A Star'); + expect(select.shadowRoot!.querySelector('button')!.getAttribute('aria-label')).toBe('A Star'); + }); + + it('should read option text when the whole option content is wrapped in an element', async () => { + const page = await newSpecPage({ + components: [Select, SelectOption], + html: `Star`, + }); + + const select = page.body.querySelector('ion-select')!; + await page.waitForChanges(); + + /** + * An option with no text node of its own, such as one whose label comes + * from an i18n component, would otherwise render as an empty select with + * an empty accessible name. + */ + expect(select.shadowRoot!.querySelector('.select-text')!.textContent).toBe('Star'); + expect(select.shadowRoot!.querySelector('button')!.getAttribute('aria-label')).toBe('Star'); + }); + + it('should ignore content assigned to the start and end slots', async () => { + const page = await newSpecPage({ + components: [Select, SelectOption], + html: `LeadStarTrail`, + }); + + const select = page.body.querySelector('ion-select')!; + await page.waitForChanges(); + + expect(select.shadowRoot!.querySelector('.select-text')!.textContent).toBe('Star'); + expect(select.shadowRoot!.querySelector('button')!.getAttribute('aria-label')).toBe('Star'); + }); + + it('should not read text the browser never paints', async () => { + const page = await newSpecPage({ + components: [Select, SelectOption], + html: `Star`, + }); + + const select = page.body.querySelector('ion-select')!; + await page.waitForChanges(); + + /** + * `textContent` includes the source of tags the browser does not render, + * and those tags are the same ones the sanitizer strips from the + * custom HTML path, so both paths have to agree to ignore them. + */ + expect(select.shadowRoot!.querySelector('.select-text')!.textContent).toBe('Star'); + expect(select.shadowRoot!.querySelector('button')!.getAttribute('aria-label')).toBe('Star'); + }); + + it('should collapse whitespace from the source markup around option text', async () => { + const page = await newSpecPage({ + components: [Select, SelectOption], + html: ` + + + Star Option + + + `, + }); + + const select = page.body.querySelector('ion-select')!; + await page.waitForChanges(); + + expect(select.shadowRoot!.querySelector('.select-text')!.textContent).toBe('Star Option'); + }); + + it('should preserve a non-breaking space that indents option text', async () => { + const page = await newSpecPage({ + components: [Select, SelectOption], + html: `  Star Option`, + }); + + const select = page.body.querySelector('ion-select')!; + await page.waitForChanges(); + + /** + * NBSP is not collapsible, so an option indented with ` ` to fake a + * hierarchy keeps its indentation. Trimming has to leave it alone too, + * which rules out `String.prototype.trim`. + */ + expect(select.shadowRoot!.querySelector('.select-text')!.textContent).toBe('\u00a0\u00a0Star Option'); + expect(select.shadowRoot!.querySelector('button')!.getAttribute('aria-label')).toBe('\u00a0\u00a0Star Option'); + }); +}); + +/** + * Frameworks render `{icon}{label}` as two sibling text nodes with no + * whitespace between them. The nodes have to be built here rather than in + * markup, because a parser collapses adjacent text into a single node. + */ +const appendAdjacentTextNodes = (option: Element) => { + option.append(document.createTextNode('★'), document.createTextNode('Star')); +}; + +/** + * The overlay interfaces build their labels from the same helper that produces + * the displayed text, so they need the same coverage. `ion-alert` is not + * defined in a spec page, so the created overlay is stubbed and the options + * passed to the controller are asserted instead. + */ +const stubAlertController = () => + jest.spyOn(alertController, 'create').mockImplementation(async () => { + const overlay = document.createElement('div') as any; + overlay.present = () => Promise.resolve(); + // Never resolves, so the select keeps treating the overlay as open. + overlay.onDidDismiss = () => new Promise(() => {}); + return overlay; + }); + +describe('ion-select: overlay option labels', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('should label alert inputs with the text the option renders', async () => { + const createAlert = stubAlertController(); + + const page = await newSpecPage({ + components: [Select, SelectOption], + html: ` + + + Star + + `, + }); + + const select = page.body.querySelector('ion-select')!; + + appendAdjacentTextNodes(select.querySelector('ion-select-option[value="adjacent"]')!); + + await page.waitForChanges(); + + await select.open(); + + expect(createAlert).toHaveBeenCalledTimes(1); + const { inputs } = createAlert.mock.calls[0][0]; + expect(inputs!.map((input) => input.label)).toEqual(['★Star', 'Star']); + }); +}); + +describe('ion-select: option plain text with custom HTML enabled', () => { + /** + * With `innerHTMLTemplatesEnabled` on, the option is read through + * `getOptionContent` instead. An option that holds only text still has to + * produce the same text as the default path. + */ + beforeEach(() => { + config.reset({ innerHTMLTemplatesEnabled: true }); + }); + + afterEach(() => { + config.reset({}); + jest.restoreAllMocks(); + }); + + it('should not insert a space between adjacent text nodes in an option', async () => { + const page = await newSpecPage({ + components: [Select, SelectOption], + html: ``, + }); + + const select = page.body.querySelector('ion-select')!; + appendAdjacentTextNodes(select.querySelector('ion-select-option')!); + + select.value = 'star'; + await page.waitForChanges(); + + expect(select.shadowRoot!.querySelector('.select-text')!.innerHTML).toBe('★Star'); + expect(select.shadowRoot!.querySelector('button')!.getAttribute('aria-label')).toBe('★Star'); + }); + + it('should not insert a space between adjacent text nodes in an option that also holds an element', async () => { + const page = await newSpecPage({ + components: [Select, SelectOption], + html: ``, + }); + + const select = page.body.querySelector('ion-select')!; + const option = select.querySelector('ion-select-option')!; + appendAdjacentTextNodes(option); + + const badge = document.createElement('ion-badge'); + badge.textContent = 'NEW'; + option.append(badge); + + select.value = 'star'; + await page.waitForChanges(); + + /** + * An element in the default slot reads the option through a different + * branch than an option that holds only text. The text nodes render as + * one span, so the `aria-label` has to keep them together too. The + * visible separation from the badge comes from `--select-text-gap` + * rather than from a space in the text. + */ + expect(select.shadowRoot!.querySelector('.select-text')!.innerHTML).toBe( + '★StarNEW' + ); + expect(select.shadowRoot!.querySelector('button')!.getAttribute('aria-label')).toBe('★StarNEW'); + }); + + it('should label alert inputs with the text the option renders', async () => { + const createAlert = stubAlertController(); + + const page = await newSpecPage({ + components: [Select, SelectOption], + html: ``, + }); + + const select = page.body.querySelector('ion-select')!; + appendAdjacentTextNodes(select.querySelector('ion-select-option')!); + await page.waitForChanges(); + + await select.open(); + + const { inputs } = createAlert.mock.calls[0][0]; + expect(inputs!.map((input) => input.label)).toEqual(['★Star']); + }); +}); + describe('ion-select: option content property reflection', () => { beforeEach(() => { // Cloning rich option content into the select text only happens when