From e19be5efd458498ad142913aff34e472cc760a55 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Mon, 29 Apr 2024 23:34:22 +0800 Subject: [PATCH] Trim titles and values in basic settings (#9598) The `buildBasicSettingsDataset` call is used for: - `block_formats` - `font_family_formats` - `font_size_formats` The documented values for both `block_formats` and `font_family_formats` include a space between the `;` and the title, for example the default value in `block_formats` is: ``` Paragraph=p; Heading 1=h1; Heading 2=h2; Heading 3=h3; Heading 4=h4; Heading 5=h5; Heading 6=h6; ``` However the `split()` method in `SelectDatasets.ts` splits on the delimiter, leading to values such as ` Heading 1=h1` which are then passed into the `process()` method which just split them on `=` leading to a title of ` Heading 1` (with leading whitespace) which does not match any valid translation. This patch modifies the `split()` method to trim the title component. It also refactors the SelectDatasets module to move the configuration processing into a utility method, and allows it to be unit tested. Additionally any white-spare around the `=` or `;` leads to erroneous values, for example a value of: ``` Paragraph = p ; ``` Leads to: title: `Paragraph ` value: ` p ` And multiple contiguous spaces leads to additional values, for example: ``` 12pt 14pt 16pt 18pt ``` Leads to a set of values: - `12pt` - `` (empty value) - `14pt` - `16pt` - `18pt` --- .../tinymce-GH-9599-2024-04-30.yaml | 6 +++ .../test/ts/browser/FontSelectCustomTest.ts | 3 +- .../main/ts/ui/core/complex/SelectDatasets.ts | 42 +++-------------- .../main/ts/ui/core/complex/utils/Select.ts | 43 +++++++++++++++++ .../ui/core/complex/utils/SelectTest.ts | 47 +++++++++++++++++++ 5 files changed, 103 insertions(+), 38 deletions(-) create mode 100644 .changes/unreleased/tinymce-GH-9599-2024-04-30.yaml create mode 100644 modules/tinymce/src/themes/silver/main/ts/ui/core/complex/utils/Select.ts create mode 100644 modules/tinymce/src/themes/silver/test/ts/atomic/ui/core/complex/utils/SelectTest.ts diff --git a/.changes/unreleased/tinymce-GH-9599-2024-04-30.yaml b/.changes/unreleased/tinymce-GH-9599-2024-04-30.yaml new file mode 100644 index 00000000000..580601655b6 --- /dev/null +++ b/.changes/unreleased/tinymce-GH-9599-2024-04-30.yaml @@ -0,0 +1,6 @@ +project: tinymce +kind: Fixed +body: Select Dataset content should be trimmed and filtered before parsing +time: 2024-04-30T11:26:37.663244+08:00 +custom: + Issue: GH-9599 diff --git a/modules/tinymce/src/core/test/ts/browser/FontSelectCustomTest.ts b/modules/tinymce/src/core/test/ts/browser/FontSelectCustomTest.ts index f8a3e9252a3..50fc42d1a30 100644 --- a/modules/tinymce/src/core/test/ts/browser/FontSelectCustomTest.ts +++ b/modules/tinymce/src/core/test/ts/browser/FontSelectCustomTest.ts @@ -1,6 +1,5 @@ import { UiFinder } from '@ephox/agar'; import { describe, it } from '@ephox/bedrock-client'; -import { Strings } from '@ephox/katamari'; import { SugarBody, TextContent } from '@ephox/sugar'; import { TinyHooks, TinySelections } from '@ephox/wrap-mcagar'; import { assert } from 'chai'; @@ -22,7 +21,7 @@ describe('browser.tinymce.core.FontSelectCustomTest', () => { const assertSelectBoxDisplayValue = (title: string, expectedValue: string) => { const selectBox = UiFinder.findIn(SugarBody.body(), `*[data-mce-name="${title}"]`).getOrDie(); - const value = Strings.trim(TextContent.get(selectBox) ?? ''); + const value = TextContent.get(selectBox) ?? ''; assert.equal(value, expectedValue, 'Should be the expected display value'); }; diff --git a/modules/tinymce/src/themes/silver/main/ts/ui/core/complex/SelectDatasets.ts b/modules/tinymce/src/themes/silver/main/ts/ui/core/complex/SelectDatasets.ts index 01de005aa2a..def0f7699ee 100644 --- a/modules/tinymce/src/themes/silver/main/ts/ui/core/complex/SelectDatasets.ts +++ b/modules/tinymce/src/themes/silver/main/ts/ui/core/complex/SelectDatasets.ts @@ -1,14 +1,7 @@ -import { Arr } from '@ephox/katamari'; - import Editor from 'tinymce/core/api/Editor'; import { UiFactoryBackstageForStyleFormats } from '../../../backstage/StyleFormatsBackstage'; - -export interface BasicSelectItem { - readonly title: string; - readonly format: string; - readonly icon?: string; -} +import { BasicSelectItem, Delimiter, makeBasicSelectItems } from './utils/Select'; export interface BasicSelectDataset { readonly type: 'basic'; @@ -21,47 +14,24 @@ export interface AdvancedSelectDataset extends UiFactoryBackstageForStyleFormats export type SelectDataset = BasicSelectDataset | AdvancedSelectDataset; -const process = (rawFormats: string[]): BasicSelectItem[] => Arr.map(rawFormats, (item) => { - let title = item, format = item; - // Allow text=value block formats - const values = item.split('='); - if (values.length > 1) { - title = values[0]; - format = values[1]; - } - - return { title, format }; -}); - const buildBasicStaticDataset = (data: Array): BasicSelectDataset => ({ type: 'basic', data }); -export enum Delimiter { - SemiColon, - Space -} - -const split = (rawFormats: string, delimiter: Delimiter): string[] => { - if (delimiter === Delimiter.SemiColon) { - return rawFormats.replace(/;$/, '').split(';'); - } else { - return rawFormats.split(' '); - } -}; - const buildBasicSettingsDataset = (editor: Editor, settingName: 'block_formats' | 'font_family_formats' | 'font_size_formats', delimiter: Delimiter): BasicSelectDataset => { // eslint-disable-next-line @tinymce/no-direct-editor-options const rawFormats = editor.options.get(settingName); - const data = process(split(rawFormats, delimiter)); + const data = makeBasicSelectItems(rawFormats, delimiter); return { type: 'basic', - data + data, }; }; export { buildBasicSettingsDataset, - buildBasicStaticDataset + buildBasicStaticDataset, + Delimiter, + BasicSelectItem }; diff --git a/modules/tinymce/src/themes/silver/main/ts/ui/core/complex/utils/Select.ts b/modules/tinymce/src/themes/silver/main/ts/ui/core/complex/utils/Select.ts new file mode 100644 index 00000000000..b4c0157ddc5 --- /dev/null +++ b/modules/tinymce/src/themes/silver/main/ts/ui/core/complex/utils/Select.ts @@ -0,0 +1,43 @@ +import { Arr } from '@ephox/katamari'; + +import { Strings} from '@ephox/katamari'; + +export interface BasicSelectItem { + readonly title: string; + readonly format: string; + readonly icon?: string; +} + +export enum Delimiter { + SemiColon = ';', + Space = ' ' +} + +const process = (rawFormats: string[]): BasicSelectItem[] => Arr.map(rawFormats, (item) => { + let title = item, format = item; + // Allow text=value block formats + const values = Arr.filter(item.split('='), (value) => Strings.isNotEmpty(value)); + if (values.length > 1) { + title = values[0]; + format = values[1]; + } + + title = Strings.trim(title); + format = Strings.trim(format); + + return { title, format }; +}); + +const split = (rawFormats: string, delimiter: Delimiter): string[] => { + if (delimiter === Delimiter.SemiColon) { + return rawFormats.replace(/;$/, '').split(';'); + } else { + return rawFormats.split(' '); + } +}; + +const makeBasicSelectItems = (rawFormats: string, delimiter: Delimiter): BasicSelectItem[] => process(split(rawFormats, delimiter)); + +export { + makeBasicSelectItems +}; diff --git a/modules/tinymce/src/themes/silver/test/ts/atomic/ui/core/complex/utils/SelectTest.ts b/modules/tinymce/src/themes/silver/test/ts/atomic/ui/core/complex/utils/SelectTest.ts new file mode 100644 index 00000000000..8db6bc350bc --- /dev/null +++ b/modules/tinymce/src/themes/silver/test/ts/atomic/ui/core/complex/utils/SelectTest.ts @@ -0,0 +1,47 @@ +import { context, describe, it } from '@ephox/bedrock-client'; +import { assert } from 'chai'; + +import { Delimiter, makeBasicSelectItems } from 'tinymce/themes/silver/ui/core/complex/utils/Select'; + +describe('atomic.tinymce.themes.silver.ui.core.complex.utils.Select', () => { + context('makeBasicSelectItems', () => { + it('Works with a semicolon delimiter', () => { + const config = 'Paragraph=p; Heading 1=h1;Heading 2=h2; Heading 3 =h3;Pre =pre'; + const items = makeBasicSelectItems(config, Delimiter.SemiColon); + assert.isArray(items); + assert.lengthOf(items, 5); + assert.deepEqual(items, [ + { title: 'Paragraph', format: 'p' }, + { title: 'Heading 1', format: 'h1' }, + { title: 'Heading 2', format: 'h2' }, + { title: 'Heading 3', format: 'h3' }, + { title: 'Pre', format: 'pre' } + ]); + }); + it('Works with a space delimiter', () => { + const config = '8pt 10pt 12pt 14pt 18pt 24pt 36pt'; + const items = makeBasicSelectItems(config, Delimiter.Space); + assert.isArray(items); + assert.lengthOf(items, 7); + assert.deepEqual(items, [ + { title: '8pt', format: '8pt' }, + { title: '10pt', format: '10pt' }, + { title: '12pt', format: '12pt' }, + { title: '14pt', format: '14pt' }, + { title: '18pt', format: '18pt' }, + { title: '24pt', format: '24pt' }, + { title: '36pt', format: '36pt' } + ]); + }); + it('Works with a mix of text/value items', () => { + const config = 'Paragraph = p ; pre'; + const items = makeBasicSelectItems(config, Delimiter.SemiColon); + assert.isArray(items); + assert.lengthOf(items, 2); + assert.deepEqual(items, [ + { title: 'Paragraph', format: 'p' }, + { title: 'pre', format: 'pre' } + ]); + }); + }); +});