diff --git a/.changelog/20260429140000_ck_20026_title_custom_roots.md b/.changelog/20260429140000_ck_20026_title_custom_roots.md new file mode 100644 index 00000000000..124eea18d2b --- /dev/null +++ b/.changelog/20260429140000_ck_20026_title_custom_roots.md @@ -0,0 +1,9 @@ +--- +type: Fix +scope: + - ckeditor5-heading +closes: + - https://github.com/ckeditor/ckeditor5/issues/20026 +--- + +The Title feature now correctly handles editor configurations where some or all roots use a custom `modelElement`. Roots that do not accept the `title` element are silently skipped at runtime, and the feature logs a single warning when no root supports the title structure. diff --git a/.changelog/20260429140100_ck_20026_ghs_hgroup.md b/.changelog/20260429140100_ck_20026_ghs_hgroup.md new file mode 100644 index 00000000000..e6e4883d847 --- /dev/null +++ b/.changelog/20260429140100_ck_20026_ghs_hgroup.md @@ -0,0 +1,9 @@ +--- +type: Fix +scope: + - ckeditor5-html-support +closes: + - https://github.com/ckeditor/ckeditor5/issues/20026 +--- + +The General HTML Support schema for the `hgroup` element no longer hardcodes `$root` as its allowed parent, so the element is correctly registered in editor configurations using a custom root `modelElement`. diff --git a/.changelog/20260429140200_ck_20026_html_comments.md b/.changelog/20260429140200_ck_20026_html_comments.md new file mode 100644 index 00000000000..06acdd5a228 --- /dev/null +++ b/.changelog/20260429140200_ck_20026_html_comments.md @@ -0,0 +1,9 @@ +--- +type: Fix +scope: + - ckeditor5-html-support +closes: + - https://github.com/ckeditor/ckeditor5/issues/20026 +--- + +The HTML comments feature no longer assumes the root model element is `$root`. Comments are now preserved in editor configurations using a custom root `modelElement`. diff --git a/eslint.config.mjs b/eslint.config.mjs index 804fa2404ca..eebc172f650 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -95,6 +95,11 @@ export default defineConfig( [ 'ckeditor5-rules/validate-module-tag': 'error', 'ckeditor5-rules/no-default-export': 'error', 'ckeditor5-rules/allow-svg-imports-only-in-icons-package': 'error', + 'ckeditor5-rules/no-literal-dollar-root': [ 'error', { + allowedPackages: [ 'ckeditor5-engine', 'ckeditor5-core' ], + allowedCalls: [ 'is' ] + } ], + 'ckeditor5-rules/require-explicit-data-context': 'error', 'ckeditor5-rules/ckeditor-plugin-flags': [ 'error', { requiredFlags: [ { name: 'isOfficialPlugin', diff --git a/package.json b/package.json index 039c28a013a..4545b9196b9 100644 --- a/package.json +++ b/package.json @@ -62,9 +62,9 @@ "date-fns": "^4.0.0", "esbuild": "^0.25.0", "eslint": "^9.34.0", - "eslint-config-ckeditor5": "^15.0.0", + "eslint-config-ckeditor5": "^15.1.0", "eslint-formatter-stylish": "^8.40.0", - "eslint-plugin-ckeditor5-rules": "^15.0.0", + "eslint-plugin-ckeditor5-rules": "^15.1.0", "estree-walker": "^3.0.3", "fs-extra": "^11.0.0", "glob": "^13.0.0", diff --git a/packages/ckeditor5-core/src/editor/editor.ts b/packages/ckeditor5-core/src/editor/editor.ts index c745b49c403..5663f773405 100644 --- a/packages/ckeditor5-core/src/editor/editor.ts +++ b/packages/ckeditor5-core/src/editor/editor.ts @@ -953,12 +953,18 @@ export abstract class Editor extends /* #__PURE__ */ ObservableMixin() { * Registers a given string as a root attribute key. Registered root attributes are added to * the {@link module:engine/model/schema~ModelSchema schema}. * - * Note: Attributes passed in the configuration for multi-root editors + * **Note:** Attributes passed in the configuration for multi-root editors * ({@link module:core/editor/editorconfig~EditorConfig#roots `config.roots..modelAttributes`}) or * single-root editors ({@link module:core/editor/editorconfig~EditorConfig#root `config.root.modelAttributes`}) * are automatically registered when the editor is initialized. However, registering the same attribute twice * does not have any negative impact, so it is recommended to use this method in any feature that uses * root attributes. + * + * **Note:** Registered attributes are attached only to the generic `$root` schema element. A custom root + * {@link module:core/editor/editorconfig~RootConfig#modelElement `modelElement`} must opt into the `$root` + * attribute chain via `allowAttributesOf: '$root'` to inherit these attributes. + * See the {@glink framework/deep-dive/schema#custom-root-elements Custom root elements} section of the + * {@glink framework/deep-dive/schema Schema deep-dive} guide for more details. */ public registerRootAttribute( key: string ): void { if ( this._registeredRootsAttributesKeys.has( key ) ) { diff --git a/packages/ckeditor5-editor-multi-root/src/multirooteditor.ts b/packages/ckeditor5-editor-multi-root/src/multirooteditor.ts index 52096989a6c..05ff8308e2d 100644 --- a/packages/ckeditor5-editor-multi-root/src/multirooteditor.ts +++ b/packages/ckeditor5-editor-multi-root/src/multirooteditor.ts @@ -115,6 +115,7 @@ export class MultiRootEditor extends Editor { normalizeRootsConfig( sourceElementsOrData, this.config, false ); normalizeRootsAttributesConfig( this.config ); + normalizeRootEditableOptionsConfig( this.config ); if ( this.config.get( 'lazyRoots' ) ) { /** @@ -175,30 +176,12 @@ export class MultiRootEditor extends Editor { } } - registerAndInitializeRootConfigAttributes( this ); + // Register `$rootEditableOptions` unconditionally, so it is always returned by `getRootAttributes()` (e.g. for RH). + // The value is set via `config.roots..modelAttributes.$rootEditableOptions` (see `normalizeRootEditableOptionsConfig`), + // which also makes it round-trip through RTC's initial-data path. + this.registerRootAttribute( '$rootEditableOptions' ); - // Registering `$rootEditableOptions` attribute to make it available in the editor model. - // This allows to store editable options for each root in the model, and make them available on other RTC clients. - // We do not use `registerRootAttribute()` method here, as this attribute is used internally - // and should not be returned by `getRootsAttributes()` method. - this.editing.model.schema.extend( '$root', { allowAttributes: '$rootEditableOptions' } ); - - this.data.on( 'init', () => { - this.model.enqueueChange( { isUndoable: false }, writer => { - for ( const [ rootName, rootConfig ] of rootsConfig ) { - const root = this.model.document.getRoot( rootName )!; - - // Set editable config for consistency with `addRoot()` method. This will allow features - // to use the same configuration for both initially loaded and dynamically added roots. - const rootEditableOptions: RootEditableOptions = { - ...rootConfig.placeholder && { placeholder: rootConfig.placeholder }, - ...rootConfig.label && { label: rootConfig.label } - }; - - writer.setAttribute( '$rootEditableOptions', rootEditableOptions, root ); - } - } ); - } ); + registerAndInitializeRootConfigAttributes( this ); const options = { shouldToolbarGroupWhenFull: !this.config.get( 'toolbar.shouldNotGroupWhenFull' ), @@ -447,7 +430,8 @@ export class MultiRootEditor extends Editor { public addRoot( rootName: string, options: AddRootOptions & AddRootRootConfig = {} ): void { const initialData: string = options.initialData || options.data || ''; - const modelAttributes: EditorRootAttributes = options.modelAttributes || options.attributes || {}; + const modelAttributes: EditorRootAttributes = { ...options.modelAttributes || options.attributes }; + // eslint-disable-next-line ckeditor5-rules/no-literal-dollar-root -- public API default for `addRoot()` const modelElement: string = options.modelElement || options.elementName || '$root'; if ( !this.model.schema.isLimit( modelElement ) ) { @@ -474,6 +458,9 @@ export class MultiRootEditor extends Editor { logWarning( 'multi-root-editor-add-root-element-option-ignored' ); } + // Storing editable options as a root attribute to make them available on other RTC clients. + ensureRootEditableOptions( modelAttributes, options ); + const _addRoot = ( writer: ModelWriter ) => { const root = writer.addRoot( rootName, modelElement ); @@ -485,14 +472,6 @@ export class MultiRootEditor extends Editor { this.registerRootAttribute( key ); writer.setAttribute( key, modelAttributes[ key ], root ); } - - // Storing editable options as a root attribute to make them available on other RTC clients. - const rootEditableOptions: RootEditableOptions = { - ...options.placeholder && { placeholder: options.placeholder }, - ...options.label && { label: options.label } - }; - - writer.setAttribute( '$rootEditableOptions', rootEditableOptions, root ); }; if ( options.isUndoable ) { @@ -1238,6 +1217,53 @@ function normalizeRootsAttributesConfig( config: Config ): void { } } +/** + * Normalize `placeholder` and `label` from `config.roots.` into the `$rootEditableOptions` root model attribute, + * stored under `config.roots..modelAttributes`. This way the attribute is registered, set on initial data load + * and shipped through RTC initial-data path together with the rest of `modelAttributes`. + * + * This is also required by the revision history feature: on editor load, RH compares the latest revision data against + * `initialData` and `modelAttributes` passed to the editor and logs a warning if they do not match. Because `$rootEditableOptions` + * ends up in the revision data, it must also be present in `modelAttributes` (even as an empty object when no options + * are configured), otherwise the comparison reports a spurious mismatch. + */ +function normalizeRootEditableOptionsConfig( config: Config ): void { + const rootsConfig = config.get( 'roots' )!; + + for ( const [ rootName, rootConfig ] of Object.entries( rootsConfig ) ) { + const modelAttributes: EditorRootAttributes = { ...rootConfig.modelAttributes }; + + if ( !ensureRootEditableOptions( modelAttributes, rootConfig ) ) { + continue; + } + + config.set( `roots.${ rootName }.modelAttributes`, modelAttributes ); + } +} + +/** + * Mutates the given `modelAttributes` map by adding the `$rootEditableOptions` entry derived from `placeholder` and `label`. + * If `$rootEditableOptions` is already present, the map is left untouched. + * + * Returns `true` when the map was modified, `false` otherwise — useful to skip downstream work (e.g. `config.set`) + * when nothing changed. + */ +function ensureRootEditableOptions( + modelAttributes: EditorRootAttributes, + { placeholder, label }: RootEditableOptions +): boolean { + if ( '$rootEditableOptions' in modelAttributes ) { + return false; + } + + modelAttributes.$rootEditableOptions = { + ...placeholder && { placeholder }, + ...label && { label } + } satisfies RootEditableOptions; + + return true; +} + function isElement( value: any ): value is Element { return _isElement( value ); } diff --git a/packages/ckeditor5-editor-multi-root/tests/multirooteditor.js b/packages/ckeditor5-editor-multi-root/tests/multirooteditor.js index 96c07b66acf..ba5cda87afa 100644 --- a/packages/ckeditor5-editor-multi-root/tests/multirooteditor.js +++ b/packages/ckeditor5-editor-multi-root/tests/multirooteditor.js @@ -1288,6 +1288,28 @@ describe( 'MultiRootEditor', () => { } ); } ); + it( 'should not overwrite $rootEditableOptions explicitly passed in attributes', () => { + const explicitOptions = { placeholder: 'From attributes', label: 'From attributes' }; + + editor.addRoot( 'bar', { + attributes: { $rootEditableOptions: explicitOptions }, + placeholder: 'From options', + label: 'From options' + } ); + + const root = editor.model.document.getRoot( 'bar' ); + + expect( root.getAttribute( '$rootEditableOptions' ) ).to.deep.equal( explicitOptions ); + } ); + + it( 'should not mutate the attributes object passed by the caller', () => { + const attributes = { order: 10 }; + + editor.addRoot( 'bar', { attributes, placeholder: 'Type here...' } ); + + expect( attributes ).to.deep.equal( { order: 10 } ); + } ); + it( 'should prefer initialData over data', () => { editor.addRoot( 'bar', { initialData: '

New.

', data: '

Old.

' } ); @@ -1439,7 +1461,7 @@ describe( 'MultiRootEditor', () => { expect( root._isLoaded ).to.be.true; expect( editor.getData( { rootName: 'foo' } ) ).to.equal( '

Foo

' ); - expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { order: 100 } ); + expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { order: 100, $rootEditableOptions: {} } ); expect( editor.registerRootAttribute.calledWithExactly( 'order' ) ); } ); @@ -1449,7 +1471,7 @@ describe( 'MultiRootEditor', () => { expect( root._isLoaded ).to.be.true; expect( editor.getData( { rootName: 'foo' } ) ).to.equal( '' ); - expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( {} ); + expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { $rootEditableOptions: {} } ); } ); it( 'should log a warning and not do anything when a root is loaded for the second time', () => { @@ -1464,7 +1486,7 @@ describe( 'MultiRootEditor', () => { expect( root._isLoaded ).to.be.true; expect( editor.getData( { rootName: 'foo' } ) ).to.equal( '

Foo

' ); - expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { order: 100 } ); + expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { order: 100, $rootEditableOptions: {} } ); expect( spy.notCalled ).to.be.true; } ); @@ -1953,8 +1975,8 @@ describe( 'MultiRootEditor', () => { } ); expect( editor.getRootsAttributes() ).to.deep.equal( { - foo: { order: 10, isLocked: null }, - bar: { order: null, isLocked: false } + foo: { order: 10, isLocked: null, $rootEditableOptions: {} }, + bar: { order: null, isLocked: false, $rootEditableOptions: {} } } ); expect( editor.editing.model.schema.checkAttribute( '$root', 'order' ) ).to.be.true; @@ -2003,12 +2025,14 @@ describe( 'MultiRootEditor', () => { expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { isLocked: true, - order: 30 + order: 30, + $rootEditableOptions: {} } ); expect( editor.getRootAttributes( 'bar' ) ).to.deep.equal( { isLocked: true, - order: 20 + order: 20, + $rootEditableOptions: {} } ); await editor.destroy(); @@ -2034,12 +2058,14 @@ describe( 'MultiRootEditor', () => { expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { isLocked: null, - order: 10 + order: 10, + $rootEditableOptions: {} } ); expect( editor.getRootAttributes( 'bar' ) ).to.deep.equal( { isLocked: true, - order: null + order: null, + $rootEditableOptions: {} } ); await editor.destroy(); @@ -2064,12 +2090,14 @@ describe( 'MultiRootEditor', () => { expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { isLocked: true, - order: 10 + order: 10, + $rootEditableOptions: {} } ); expect( editor.getRootAttributes( 'bar' ) ).to.deep.equal( { isLocked: false, - order: 20 + order: 20, + $rootEditableOptions: {} } ); await editor.destroy(); @@ -2093,18 +2121,20 @@ describe( 'MultiRootEditor', () => { expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { isLocked: true, - order: 10 + order: 10, + $rootEditableOptions: {} } ); expect( editor.getRootAttributes( 'bar' ) ).to.deep.equal( { isLocked: false, - order: null + order: null, + $rootEditableOptions: {} } ); await editor.destroy(); } ); - it( 'should not include $rootEditableOptions', async () => { + it( 'should include $rootEditableOptions when placeholder or label are configured', async () => { editor = await MultiRootEditor.create( { foo: '' }, { roots: { foo: { @@ -2115,7 +2145,10 @@ describe( 'MultiRootEditor', () => { } } ); - expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { order: 10 } ); + expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { + order: 10, + $rootEditableOptions: { placeholder: 'Type here...', label: 'My label' } + } ); await editor.destroy(); } ); @@ -2133,12 +2166,14 @@ describe( 'MultiRootEditor', () => { expect( editor.getRootAttributes( 'foo' ) ).to.deep.equal( { isLocked: null, - order: 30 + order: 30, + $rootEditableOptions: {} } ); expect( editor.getRootAttributes( 'bar' ) ).to.deep.equal( { isLocked: true, - order: null + order: null, + $rootEditableOptions: {} } ); await editor.destroy(); @@ -2168,11 +2203,13 @@ describe( 'MultiRootEditor', () => { expect( editor.getRootsAttributes() ).to.deep.equal( { bar: { isLocked: true, - order: 20 + order: 20, + $rootEditableOptions: {} }, foo: { isLocked: true, - order: 30 + order: 30, + $rootEditableOptions: {} } } ); @@ -2207,11 +2244,13 @@ describe( 'MultiRootEditor', () => { expect( editor.getRootsAttributes( 'foo' ) ).to.deep.equal( { foo: { isLocked: null, - order: 10 + order: 10, + $rootEditableOptions: {} }, bar: { isLocked: true, - order: null + order: null, + $rootEditableOptions: {} } } ); @@ -2239,22 +2278,25 @@ describe( 'MultiRootEditor', () => { expect( editor.getRootsAttributes() ).to.deep.equal( { abc: { isLocked: null, - order: 30 + order: 30, + $rootEditableOptions: {} }, foo: { isLocked: true, - order: 10 + order: 10, + $rootEditableOptions: {} }, xxx: { isLocked: false, - order: 40 + order: 40, + $rootEditableOptions: {} } } ); await editor.destroy(); } ); - it( 'should not include $rootEditableOptions', async () => { + it( 'should include $rootEditableOptions when placeholder or label are configured', async () => { editor = await MultiRootEditor.create( { foo: '', bar: '' }, { roots: { foo: { @@ -2269,8 +2311,36 @@ describe( 'MultiRootEditor', () => { } ); expect( editor.getRootsAttributes() ).to.deep.equal( { - foo: { order: 10 }, - bar: { order: 20 } + foo: { + order: 10, + $rootEditableOptions: { placeholder: 'Foo placeholder', label: 'Foo label' } + }, + bar: { + order: 20, + $rootEditableOptions: {} + } + } ); + + await editor.destroy(); + } ); + + it( 'should not overwrite $rootEditableOptions explicitly set via modelAttributes', async () => { + const explicitOptions = { placeholder: 'From modelAttributes', label: 'From modelAttributes' }; + + editor = await MultiRootEditor.create( { foo: '' }, { + roots: { + foo: { + modelAttributes: { $rootEditableOptions: explicitOptions }, + placeholder: 'From config', + label: 'From config' + } + } + } ); + + expect( editor.getRootsAttributes() ).to.deep.equal( { + foo: { + $rootEditableOptions: explicitOptions + } } ); await editor.destroy(); diff --git a/packages/ckeditor5-engine/docs/framework/deep-dive/schema.md b/packages/ckeditor5-engine/docs/framework/deep-dive/schema.md index a8bd608e6cf..90b113a63e9 100644 --- a/packages/ckeditor5-engine/docs/framework/deep-dive/schema.md +++ b/packages/ckeditor5-engine/docs/framework/deep-dive/schema.md @@ -765,6 +765,37 @@ Which, in turn, has these [semantics](#defining-additional-semantics): ``` +### Custom root elements + +The generic `$root` / `$container` / `$block` chain described above is keyed on the element name `$root`. By default, a root is created as `<$root>`, so every rule of the form `allowIn: '$root'` or `allowAttributesOf: '$root'` applies to it automatically. + +You can change the element name used for a root via the {@link module:core/editor/editorconfig~RootConfig#modelElement `config.root.modelElement`} (or `config.roots..modelElement` for the {@link module:editor-multi-root/multirooteditor~MultiRootEditor multi-root editor}) option. When you do, the created root is `` instead of `<$root>`, and **it does not automatically inherit the `$root` chain**. Features that define their elements as `allowIn: '$root'`, `allowContentOf: '$root'`, or `allowAttributesOf: '$root'` will not apply to the custom root unless you opt in. + +Declare the custom root in the schema and pick which parts of the `$root` chain you want: + +```js +// Inherit everything $root provides - allowed children, attributes, and is* flags. +schema.register( 'myRoot', { + inheritAllFrom: '$root', + allowChildren: [ '$container', '$block' ] +} ); + +// Or opt in selectively - e.g. only inherit attributes (the `$inlineRoot` pattern). +schema.register( 'myInlineRoot', { + allowContentOf: '$block', + allowAttributesOf: '$root', + isLimit: true +} ); +``` + +Key rules to remember for custom roots: + +* **Block / container content.** If the root should accept the generic block chain, declare `allowChildren: [ '$container', '$block' ]` (or `inheritAllFrom: '$root'`). Otherwise `$block`-based elements like `` and `$container`-based elements like `
` will not be allowed inside it. +* **Root attributes.** Attributes registered via {@link module:core/editor/editor~Editor#registerRootAttribute `editor.registerRootAttribute()`} are attached only to the `$root` schema element. Custom roots must opt into this chain via `allowAttributesOf: '$root'` to receive them. +* **Data conversion context.** When calling {@link module:engine/controller/datacontroller~DataController#parse `editor.data.parse()`} or {@link module:engine/controller/datacontroller~DataController#toModel `editor.data.toModel()`} against a custom root, pass the target root element (or its configured model element name) as the `context` argument. The default `'$root'` only matches the generic root and can produce wrong conversion results. + +The engine ships with one ready-made custom root definition - `$inlineRoot` - for roots that should only contain inline content. You can use it out of the box via `config.root.modelElement: '$inlineRoot'` without adding your own schema registration. + ## Defining advanced rules using callbacks The base {@link module:engine/model/schema~ModelSchemaItemDefinition declarative `SchemaItemDefinition` API} is by its nature limited, and some custom rules might not be possible to be implemented this way. diff --git a/packages/ckeditor5-engine/src/controller/datacontroller.ts b/packages/ckeditor5-engine/src/controller/datacontroller.ts index d98efce848c..fe32c9787c7 100644 --- a/packages/ckeditor5-engine/src/controller/datacontroller.ts +++ b/packages/ckeditor5-engine/src/controller/datacontroller.ts @@ -458,6 +458,13 @@ export class DataController extends /* #__PURE__ */ EmitterMixin() { * Returns the data parsed by the {@link #processor data processor} and then converted by upcast converters * attached to the {@link #upcastDispatcher}. * + * **Note:** The default `context` value is `'$root'`, which only matches the generic root. When the editor uses a + * custom root {@link module:core/editor/editorconfig~RootConfig#modelElement `modelElement`}, pass the target + * {@link module:engine/model/rootelement~ModelRootElement root element} (or its configured model element name) + * explicitly, otherwise the conversion result may be wrong. + * See the {@glink framework/deep-dive/schema#custom-root-elements Custom root elements} section of the + * {@glink framework/deep-dive/schema Schema deep-dive} guide for more details. + * * @see #set * @param data Data to parse. * @param context Base context in which the view will be converted to the model. @@ -480,6 +487,13 @@ export class DataController extends /* #__PURE__ */ EmitterMixin() { * When marker elements were converted during the conversion process, it will be set as a document fragment's * {@link module:engine/model/documentfragment~ModelDocumentFragment#markers static markers map}. * + * **Note:** The default `context` value is `'$root'`, which only matches the generic root. When the editor uses a + * custom root {@link module:core/editor/editorconfig~RootConfig#modelElement `modelElement`}, pass the target + * {@link module:engine/model/rootelement~ModelRootElement root element} (or its configured model element name) + * explicitly, otherwise the conversion result may be wrong. + * See the {@glink framework/deep-dive/schema#custom-root-elements Custom root elements} section of the + * {@glink framework/deep-dive/schema Schema deep-dive} guide for more details. + * * @fires toModel * @param viewElementOrFragment The element or document fragment whose content will be converted. * @param context Base context in which the view will be converted to the model. diff --git a/packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts b/packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts index 4c1a26f4fc3..ccd42dc2353 100644 --- a/packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts +++ b/packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts @@ -184,6 +184,13 @@ export class UpcastDispatcher extends /* #__PURE__ */ EmitterMixin() { /** * Starts the conversion process. The entry point for the conversion. * + * **Note:** The default `context` value is `[ '$root' ]`, which only matches the generic root. When the editor uses + * a custom root {@link module:core/editor/editorconfig~RootConfig#modelElement `modelElement`}, pass the target + * {@link module:engine/model/rootelement~ModelRootElement root element} (or its configured model element name) + * explicitly, otherwise the conversion result may be wrong. + * See the {@glink framework/deep-dive/schema#custom-root-elements Custom root elements} section of the + * {@glink framework/deep-dive/schema Schema deep-dive} guide for more details. + * * @fires element * @fires text * @fires documentFragment diff --git a/packages/ckeditor5-engine/src/dev-utils/model.ts b/packages/ckeditor5-engine/src/dev-utils/model.ts index 1c049202ee3..d383a739f40 100644 --- a/packages/ckeditor5-engine/src/dev-utils/model.ts +++ b/packages/ckeditor5-engine/src/dev-utils/model.ts @@ -356,6 +356,13 @@ export function _stringifyModel( * <$text attribute="value">Text data * ``` * + * **Note:** The default `options.context` value is `'$root'`, which only matches the generic root. When the editor + * uses a custom root {@link module:core/editor/editorconfig~RootConfig#modelElement `modelElement`}, pass the target + * {@link module:engine/model/rootelement~ModelRootElement root element} (or its configured model element name) + * explicitly, otherwise the conversion result may be wrong. + * See the {@glink framework/deep-dive/schema#custom-root-elements Custom root elements} section of the + * {@glink framework/deep-dive/schema Schema deep-dive} guide for more details. + * * @param data HTML-like string to be parsed. * @param schema A schema instance used by converters for element validation. * @param options Additional configuration. diff --git a/packages/ckeditor5-engine/src/model/document.ts b/packages/ckeditor5-engine/src/model/document.ts index 9e1aa483454..02adc27763e 100644 --- a/packages/ckeditor5-engine/src/model/document.ts +++ b/packages/ckeditor5-engine/src/model/document.ts @@ -227,6 +227,12 @@ export class ModelDocument extends /* #__PURE__ */ EmitterMixin() { * **Note:** do not use this method after the editor has been initialized! If you want to dynamically add a root, use * {@link module:engine/model/writer~ModelWriter#addRoot `model.Writer#addRoot`} instead. * + * **Note:** The default `elementName` value is `'$root'`. When the editor uses a custom root + * {@link module:core/editor/editorconfig~RootConfig#modelElement `modelElement`}, pass the configured model element + * name explicitly so the created root matches the schema the features expect. + * See the {@glink framework/deep-dive/schema#custom-root-elements Custom root elements} section of the + * {@glink framework/deep-dive/schema Schema deep-dive} guide for more details. + * * @param elementName The element name. Defaults to `'$root'` which also has some basic schema defined * (e.g. `$block` elements are allowed inside the `$root`). Make sure to define a proper schema if you use a different name. * @param rootName A unique root name. diff --git a/packages/ckeditor5-engine/src/model/writer.ts b/packages/ckeditor5-engine/src/model/writer.ts index b1177aa8c76..ee1b234a86f 100644 --- a/packages/ckeditor5-engine/src/model/writer.ts +++ b/packages/ckeditor5-engine/src/model/writer.ts @@ -1335,6 +1335,12 @@ export class ModelWriter { * * Throws an error, if trying to add a root that is already added and attached. * + * **Note:** The default `elementName` value is `'$root'`. When the editor uses a custom root + * {@link module:core/editor/editorconfig~RootConfig#modelElement `modelElement`}, pass the configured model element + * name explicitly so the added root matches the schema the features expect. + * See the {@glink framework/deep-dive/schema#custom-root-elements Custom root elements} section of the + * {@glink framework/deep-dive/schema Schema deep-dive} guide for more details. + * * @param rootName Name of the added root. * @param elementName The element name. Defaults to `'$root'` which also has some basic schema defined * (e.g. `$block` elements are allowed inside the `$root`). Make sure to define a proper schema if you use a different name. diff --git a/packages/ckeditor5-heading/src/title.ts b/packages/ckeditor5-heading/src/title.ts index fb4556da3da..ecce5c1c1ce 100644 --- a/packages/ckeditor5-heading/src/title.ts +++ b/packages/ckeditor5-heading/src/title.ts @@ -9,7 +9,7 @@ import { Plugin, type Editor, type ElementApi } from '@ckeditor/ckeditor5-core'; import { Paragraph } from '@ckeditor/ckeditor5-paragraph'; -import { first, type GetCallback } from '@ckeditor/ckeditor5-utils'; +import { first, logWarning, type GetCallback } from '@ckeditor/ckeditor5-utils'; import { ViewDowncastWriter, enableViewPlaceholder, @@ -83,6 +83,14 @@ export class Title extends Plugin { // // // See: https://github.com/ckeditor/ckeditor5/issues/2005. + // + // Title is scoped to roots whose `modelElement` is the generic `$root`. Custom root + // `modelElement` names (including `$inlineRoot`) are intentionally not supported: + // the title structure (`title` + `title-content` + paragraph body placeholder) relies on + // the root accepting `$block` content, which is not guaranteed for custom or inline roots. + // Runtime codepaths below additionally guard on `schema.checkChild( root, 'title' )` so + // the plugin gracefully no-ops on roots where the schema does not allow the title element. + // eslint-disable-next-line ckeditor5-rules/no-literal-dollar-root -- registered only on the default `$root` by design model.schema.register( 'title', { isBlock: true, allowIn: '$root' } ); model.schema.register( 'title-content', { isBlock: true, allowIn: 'title', allowAttributes: [ 'alignment' ] } ); model.schema.extend( '$text', { allowIn: 'title-content' } ); @@ -131,6 +139,37 @@ export class Title extends Plugin { // Attach Tab handling. this._attachTabPressHandling(); + + this._warnIfNoSupportedRoot(); + } + + /** + * Logs a single warning when none of the editor's roots can host the title structure. The Title feature + * only operates on roots whose `modelElement` is the default `$root`; roots configured with a custom + * `modelElement` are silently skipped at runtime. If no root supports the structure, the plugin is + * effectively a no-op and the integrator likely wants to know. + */ + private _warnIfNoSupportedRoot(): void { + const model = this.editor.model; + + for ( const root of model.document.getRoots() ) { + if ( model.schema.checkChild( root, 'title' ) ) { + return; + } + } + + /** + * The Title feature was loaded, but none of the editor's roots supports the `title` element. The feature + * only operates on roots whose `modelElement` is the default `$root`; roots configured with a custom + * `modelElement` (including `$inlineRoot`) are silently skipped, so `getTitle()` / `getBody()` fall back + * to the regular data getter and no title structure is ever inserted. + * + * To use the Title feature, ensure at least one root uses the default `$root` model element. Otherwise, + * remove the Title plugin from this editor's plugin list. + * + * @error title-no-supported-root + */ + logWarning( 'title-no-supported-root' ); } /** @@ -148,7 +187,13 @@ export class Title extends Plugin { public getTitle( options: Record = {} ): string { const rootName = options.rootName ? options.rootName as string : undefined; const titleElement = this._getTitleElement( rootName ); - const titleContentElement = titleElement!.getChild( 0 ) as ModelElement; + + // Root does not support the title structure (custom/inline root) — nothing to stringify. + if ( !titleElement ) { + return ''; + } + + const titleContentElement = titleElement.getChild( 0 ) as ModelElement; return this.editor.data.stringify( titleContentElement, options ); } @@ -170,6 +215,20 @@ export class Title extends Plugin { const model = editor.model; const rootName = options.rootName ? options.rootName as string : undefined; const root = editor.model.document.getRoot( rootName )!; + + // Root does not support the title structure (custom/inline root) — the whole root is the body. + // Delegate to the regular data getter so mixed-root callers receive useful content. + if ( !model.schema.checkChild( root, 'title' ) ) { + return data.get( { ...options, rootName: root.rootName } ); + } + + // Root is empty / missing the expected title element (e.g. detached root or transient state) — no body to stringify. + const firstChild = root.getChild( 0 ); + + if ( !firstChild || !firstChild.is( 'element', 'title' ) ) { + return ''; + } + const view = editor.editing.view; const viewWriter = new ViewDowncastWriter( view.document ); @@ -177,7 +236,7 @@ export class Title extends Plugin { const viewDocumentFragment = viewWriter.createDocumentFragment(); // Find all markers that intersects with body. - const bodyStartPosition = model.createPositionAfter( root.getChild( 0 )! ); + const bodyStartPosition = model.createPositionAfter( firstChild ); const bodyRange = model.createRange( bodyStartPosition, model.createPositionAt( root, 'end' ) ); const markers = new Map(); @@ -206,7 +265,13 @@ export class Title extends Plugin { * Returns the `title` element when it is in the document. Returns `undefined` otherwise. */ private _getTitleElement( rootName?: string ): ModelElement | undefined { - const root = this.editor.model.document.getRoot( rootName )!; + const model = this.editor.model; + const root = model.document.getRoot( rootName )!; + + // Root does not support the title structure (custom/inline root). + if ( !model.schema.checkChild( root, 'title' ) ) { + return; + } for ( const child of root.getChildren() as IterableIterator ) { if ( isTitle( child ) ) { @@ -256,6 +321,11 @@ export class Title extends Plugin { const model = this.editor.model; for ( const modelRoot of this.editor.model.document.getRoots() ) { + // Skip roots that do not support the title structure (custom/inline root). + if ( !model.schema.checkChild( modelRoot, 'title' ) ) { + continue; + } + const titleElements = Array.from( modelRoot.getChildren() as IterableIterator ).filter( isTitle ); const firstTitleElement = titleElements[ 0 ]; const firstRootChild = modelRoot.getChild( 0 ) as ModelElement; @@ -305,12 +375,15 @@ export class Title extends Plugin { * when it is needed for the placeholder purposes. */ private _fixBodyElement( writer: ModelWriter ) { + const schema = this.editor.model.schema; let changed = false; for ( const rootName of this.editor.model.document.getRootNames() ) { const modelRoot = this.editor.model.document.getRoot( rootName )!; - if ( modelRoot.childCount < 2 ) { + // Only insert the paragraph body placeholder when the root actually accepts `paragraph`. + // Custom/inline roots that do not accept `$block` are intentionally skipped. + if ( modelRoot.childCount < 2 && schema.checkChild( modelRoot, 'paragraph' ) ) { const placeholder = writer.createElement( 'paragraph' ); writer.insert( placeholder, modelRoot, 1 ); @@ -332,7 +405,12 @@ export class Title extends Plugin { for ( const rootName of this.editor.model.document.getRootNames() ) { const root = this.editor.model.document.getRoot( rootName )!; - const placeholder = this._bodyPlaceholder.get( rootName )!; + const placeholder = this._bodyPlaceholder.get( rootName ); + + // Roots that do not support the title structure never had a body placeholder created. + if ( !placeholder ) { + continue; + } if ( shouldRemoveLastParagraph( placeholder, root ) ) { this._bodyPlaceholder.delete( rootName ); @@ -383,7 +461,16 @@ export class Title extends Plugin { continue; } - // If `viewRoot` is not empty, then we can expect at least two elements in it. + // Skip roots whose schema does not support the title structure (custom/inline root). + // Their view root won't have the expected title+body layout. + // A title-allowed root always has a paragraph body placeholder created by `_fixBodyElement`, + // so the second view child is guaranteed to exist once this guard passes. + const modelRoot = editor.editing.mapper.toModelElement( viewRoot )!; + + if ( !editor.model.schema.checkChild( modelRoot, 'title' ) ) { + continue; + } + const body = viewRoot!.getChild( 1 ) as ViewElement; const oldBody = bodyViewElements.get( viewRoot.rootName ); @@ -455,6 +542,11 @@ export class Title extends Plugin { const selectionPosition = selection.getFirstPosition()!; const root = editor.model.document.getRoot( selectionPosition.root.rootName! )!; + // Root does not support the title structure (custom/inline root) — no title to jump to. + if ( !model.schema.checkChild( root, 'title' ) ) { + return; + } + const title = root.getChild( 0 ) as ModelElement; const body = root.getChild( 1 ); @@ -471,6 +563,9 @@ export class Title extends Plugin { /** * A view-to-model converter for the h1 that appears at the beginning of the document (a title element). * + * Matches only the synthetic upcast parent named `$root` (the default generic root element). Title is not supported + * for roots whose `modelElement` is customized, so this converter intentionally does not fire on them. + * * @see module:engine/conversion/upcastdispatcher~UpcastDispatcher#event:element * @param evt An object containing information about the fired event. * @param data An object containing conversion input, a placeholder for conversion output and possibly other values. @@ -480,6 +575,9 @@ function dataViewModelH1Insertion( evt: unknown, data: UpcastConversionData { // Does not include title and body. expect( barModelRoot.isEmpty ).to.be.true; } ); + + it( 'should return an empty string from getTitle() for a detached root', () => { + multiRoot.detachRoot( 'bar' ); + + expect( multiRoot.plugins.get( Title ).getTitle( { rootName: 'bar' } ) ).to.equal( '' ); + } ); + + it( 'should return an empty string from getBody() for a detached root (first-child guard)', () => { + multiRoot.detachRoot( 'bar' ); + + const barRoot = multiRoot.model.document.getRoot( 'bar' ); + + // Detached root is empty but schema still reports `title` as allowed, so the + // first-child check is what prevents the NPE here. + expect( barRoot.isEmpty ).to.equal( true ); + expect( multiRoot.model.schema.checkChild( barRoot, 'title' ) ).to.equal( true ); + + expect( multiRoot.plugins.get( Title ).getBody( { rootName: 'bar' } ) ).to.equal( '' ); + } ); +} ); + +describe( 'Title integration with a mixed $root / $inlineRoot multi root editor', () => { + let multiRoot, titlePlugin, mainRoot, inlineRoot; + + beforeEach( async () => { + multiRoot = await MultiRootEditor.create( {}, { + plugins: [ Paragraph, Heading, Enter, Title ], + roots: { + main: { + modelElement: '$root', + initialData: '

MainTitle

Main body

' + }, + inline: { + modelElement: '$inlineRoot', + initialData: 'Inline content' + } + } + } ); + titlePlugin = multiRoot.plugins.get( Title ); + mainRoot = multiRoot.model.document.getRoot( 'main' ); + inlineRoot = multiRoot.model.document.getRoot( 'inline' ); + } ); + + afterEach( async () => { + await multiRoot.destroy(); + } ); + + it( 'should allow title only in the $root root, not in the $inlineRoot root', () => { + const schema = multiRoot.model.schema; + + expect( schema.checkChild( mainRoot, 'title' ) ).to.equal( true ); + expect( schema.checkChild( inlineRoot, 'title' ) ).to.equal( false ); + } ); + + it( 'should create the title + body structure in the $root root', () => { + expect( mainRoot.getChild( 0 ).is( 'element', 'title' ) ).to.equal( true ); + expect( mainRoot.getChild( 1 ).is( 'element', 'paragraph' ) ).to.equal( true ); + } ); + + it( 'should not insert a title element into the $inlineRoot root', () => { + const hasTitle = Array.from( inlineRoot.getChildren() ) + .some( child => child.is( 'element' ) && child.name === 'title' ); + + expect( hasTitle ).to.equal( false ); + } ); + + it( 'should not insert a paragraph body placeholder into the $inlineRoot root', () => { + const hasParagraph = Array.from( inlineRoot.getChildren() ) + .some( child => child.is( 'element' ) && child.name === 'paragraph' ); + + expect( hasParagraph ).to.equal( false ); + } ); + + it( 'should return title value for the $root root and an empty string for the $inlineRoot root', () => { + expect( titlePlugin.getTitle( { rootName: 'main' } ) ).to.equal( 'MainTitle' ); + expect( titlePlugin.getTitle( { rootName: 'inline' } ) ).to.equal( '' ); + } ); + + it( 'should return body value for the $root root and fall back to full data for the $inlineRoot root', () => { + expect( titlePlugin.getBody( { rootName: 'main' } ) ).to.equal( '

Main body

' ); + // No title structure on the inline root — the whole root is the body. + expect( titlePlugin.getBody( { rootName: 'inline' } ) ).to.equal( 'Inline content' ); + } ); + + it( 'should round-trip data on the $inlineRoot root without being touched by Title', () => { + expect( multiRoot.getData( { rootName: 'inline' } ) ).to.equal( 'Inline content' ); + } ); + + it( 'should not throw when the view post-fixer runs after a change in the $inlineRoot root', () => { + expect( () => { + multiRoot.model.change( writer => { + writer.insertText( '!', writer.createPositionAt( inlineRoot, 'end' ) ); + } ); + } ).not.to.throw(); + + expect( multiRoot.getData( { rootName: 'inline' } ) ).to.equal( 'Inline content!' ); + } ); + + it( 'should not throw when the view post-fixer runs after a change in the $root root', () => { + expect( () => { + multiRoot.model.change( writer => { + writer.insertText( '!', writer.createPositionAt( mainRoot.getChild( 1 ), 'end' ) ); + } ); + } ).not.to.throw(); + + expect( titlePlugin.getBody( { rootName: 'main' } ) ).to.equal( '

Main body!

' ); + } ); } ); diff --git a/packages/ckeditor5-heading/tests/title.js b/packages/ckeditor5-heading/tests/title.js index f9ce92c1553..35650acc183 100644 --- a/packages/ckeditor5-heading/tests/title.js +++ b/packages/ckeditor5-heading/tests/title.js @@ -369,6 +369,25 @@ describe( 'Title', () => { '' ); } ); + + it( 'should keep the body placeholder paragraph once it has typed content', () => { + // On an empty editor the post-fixer creates a `` placeholder and remembers it. + // Typing into it gives the placeholder `childCount > 0`, which must short-circuit + // `shouldRemoveLastParagraph` so the paragraph is kept. + const root = model.document.getRoot(); + const placeholderParagraph = root.getChild( 1 ); + + expect( placeholderParagraph.name ).to.equal( 'paragraph' ); + expect( placeholderParagraph.childCount ).to.equal( 0 ); + + model.change( writer => { + writer.insertText( 'x', writer.createPositionAt( placeholderParagraph, 0 ) ); + } ); + + // The placeholder is still there with the typed content. + expect( root.getChild( 1 ) ).to.equal( placeholderParagraph ); + expect( placeholderParagraph.childCount ).to.equal( 1 ); + } ); } ); describe( 'getTitle()', () => { @@ -944,8 +963,153 @@ describe( 'Title', () => { ); } ); } ); + + describe( 'with an $inlineRoot modelElement', () => { + let inlineElement, inlineEditor, inlineModel, inlineRoot, titlePlugin, warnStub; + + beforeEach( async () => { + // Title logs a single `title-no-supported-root` warning when no root accepts the title element; + // silence it here so the CI watchdog for unexpected console output does not fail the suite. + warnStub = sinon.stub( console, 'warn' ); + + inlineElement = document.createElement( 'div' ); + document.body.appendChild( inlineElement ); + + inlineEditor = await ClassicTestEditor.create( inlineElement, { + plugins: [ Paragraph, Title, Heading, BlockQuote, Clipboard, Image, ImageUpload, Enter, Undo ], + root: { modelElement: '$inlineRoot' } + } ); + inlineModel = inlineEditor.model; + inlineRoot = inlineModel.document.getRoot(); + titlePlugin = inlineEditor.plugins.get( Title ); + } ); + + afterEach( async () => { + await inlineEditor.destroy(); + inlineElement.remove(); + warnStub.restore(); + } ); + + it( 'should not allow title as a child of $inlineRoot', () => { + expect( inlineModel.schema.checkChild( inlineRoot, 'title' ) ).to.equal( false ); + } ); + + it( 'should not allow paragraph as a child of $inlineRoot', () => { + // Sanity check behind the `_fixBodyElement` schema guard. + expect( inlineModel.schema.checkChild( inlineRoot, 'paragraph' ) ).to.equal( false ); + } ); + + it( 'should not insert a title element into $inlineRoot on load (model post-fixer no-op)', () => { + inlineEditor.setData( 'Foo' ); + + const hasTitle = Array.from( inlineRoot.getChildren() ) + .some( child => child.is( 'element' ) && child.name === 'title' ); + + expect( hasTitle ).to.equal( false ); + } ); + + it( 'should not insert a paragraph body placeholder into $inlineRoot', () => { + inlineEditor.setData( 'Foo' ); + + const hasParagraph = Array.from( inlineRoot.getChildren() ) + .some( child => child.is( 'element' ) && child.name === 'paragraph' ); + + expect( hasParagraph ).to.equal( false ); + } ); + + it( 'should return an empty string from getTitle() for $inlineRoot', () => { + inlineEditor.setData( 'Foo' ); + + expect( titlePlugin.getTitle() ).to.equal( '' ); + } ); + + it( 'should fall back to the full root data from getBody() for $inlineRoot', () => { + inlineEditor.setData( 'Foo' ); + + // No title structure exists, so the whole root IS the body. + expect( titlePlugin.getBody() ).to.equal( 'Foo' ); + } ); + + it( 'should not upcast

to title when the target root is $inlineRoot', () => { + inlineEditor.setData( '

Foo

' ); + + const hasTitle = Array.from( inlineRoot.getChildren() ) + .some( child => child.is( 'element' ) && child.name === 'title' ); + + expect( hasTitle ).to.equal( false ); + } ); + + it( 'should no-op on Shift+Tab when the root is $inlineRoot', () => { + inlineEditor.setData( 'Foo' ); + + inlineModel.change( writer => { + writer.setSelection( inlineRoot, 0 ); + } ); + + const eventData = getEventData( keyCodes.tab, { shiftKey: true } ); + + inlineEditor.keystrokes.press( eventData ); + + sinon.assert.notCalled( eventData.preventDefault ); + sinon.assert.notCalled( eventData.stopPropagation ); + } ); + + it( 'should not throw when the view post-fixer runs after a model change on $inlineRoot', () => { + inlineEditor.setData( 'Foo' ); + + expect( () => { + inlineModel.change( writer => { + writer.insertText( ' bar', writer.createPositionAt( inlineRoot, 'end' ) ); + } ); + } ).not.to.throw(); + + expect( inlineEditor.getData() ).to.equal( 'Foo bar' ); + } ); + } ); + + describe( '_warnIfNoSupportedRoot()', () => { + const WARNING_ID = 'title-no-supported-root'; + + let warnStub, warnEditorElement, warnEditor; + + beforeEach( () => { + warnStub = sinon.stub( console, 'warn' ); + warnEditorElement = document.createElement( 'div' ); + document.body.appendChild( warnEditorElement ); + } ); + + afterEach( async () => { + if ( warnEditor ) { + await warnEditor.destroy(); + warnEditor = null; + } + warnEditorElement.remove(); + warnStub.restore(); + } ); + + it( 'should not warn when at least one root supports the title element', async () => { + warnEditor = await ClassicTestEditor.create( warnEditorElement, { + plugins: [ Paragraph, Title, Heading ] + } ); + + expect( countWarnings( warnStub, WARNING_ID ) ).to.equal( 0 ); + } ); + + it( 'should warn exactly once when no root supports the title element', async () => { + warnEditor = await ClassicTestEditor.create( warnEditorElement, { + plugins: [ Paragraph, Title, Heading ], + root: { modelElement: '$inlineRoot' } + } ); + + expect( countWarnings( warnStub, WARNING_ID ) ).to.equal( 1 ); + } ); + } ); } ); +function countWarnings( warnStub, id ) { + return warnStub.getCalls().filter( call => String( call.args[ 0 ] ).includes( id ) ).length; +} + function getEventData( keyCode, { shiftKey = false } = {} ) { return { keyCode, diff --git a/packages/ckeditor5-html-support/src/htmlcomment.ts b/packages/ckeditor5-html-support/src/htmlcomment.ts index 20c013bcf66..d59da635b6d 100644 --- a/packages/ckeditor5-html-support/src/htmlcomment.ts +++ b/packages/ckeditor5-html-support/src/htmlcomment.ts @@ -40,9 +40,15 @@ export class HtmlComment extends Plugin { editor.data.processor.skipComments = false; - // Allow storing comment's content as the $root attribute with the name `$comment:`. + // Declare the `$comment` attribute on `$root`. Custom roots that opt into the `$root` + // attribute chain via `allowAttributesOf: '$root'` get comment support automatically. + editor.model.schema.extend( '$root', { allowAttributes: '$comment' } ); + + // Allow storing comment's content as a root attribute with the name `$comment:`. + // Gate the per-comment attribute on the root already allowing the generic `$comment` attribute + // so the rule works for any root name. editor.model.schema.addAttributeCheck( ( context, attributeName ) => { - if ( context.endsWith( '$root' ) && attributeName.startsWith( '$comment' ) ) { + if ( attributeName.startsWith( '$comment:' ) && editor.model.schema.checkAttribute( context, '$comment' ) ) { return true; } } ); diff --git a/packages/ckeditor5-html-support/src/schemadefinitions.ts b/packages/ckeditor5-html-support/src/schemadefinitions.ts index d0de5b2fa8d..46eff414eca 100644 --- a/packages/ckeditor5-html-support/src/schemadefinitions.ts +++ b/packages/ckeditor5-html-support/src/schemadefinitions.ts @@ -358,7 +358,7 @@ export const defaultConfig = { model: 'htmlHgroup', view: 'hgroup', modelSchema: { - allowIn: [ '$root', '$container' ], + allowWhere: '$container', allowChildren: [ 'paragraph', 'htmlP', diff --git a/packages/ckeditor5-html-support/tests/htmlcomment.js b/packages/ckeditor5-html-support/tests/htmlcomment.js index 123ce70e9e7..6b470a23ed5 100644 --- a/packages/ckeditor5-html-support/tests/htmlcomment.js +++ b/packages/ckeditor5-html-support/tests/htmlcomment.js @@ -55,6 +55,75 @@ describe( 'HtmlComment', () => { expect( editor.getData() ).to.equal( '

Foo

' ); } ); } ); + + it( 'should declare the $comment attribute on $root so custom roots inheriting its attributes pick it up', () => { + model.schema.register( 'myRoot', { + inheritAllFrom: '$root' + } ); + + expect( model.schema.checkAttribute( [ 'myRoot' ], '$comment' ) ).to.be.true; + expect( model.schema.checkAttribute( [ 'myRoot' ], '$comment:abc123' ) ).to.be.true; + } ); + + it( 'should allow the per-comment attribute on a custom root that only opts into $root attributes', () => { + model.schema.register( 'myAttrRoot', { + allowAttributesOf: '$root' + } ); + + expect( model.schema.checkAttribute( [ 'myAttrRoot' ], '$comment' ) ).to.be.true; + expect( model.schema.checkAttribute( [ 'myAttrRoot' ], '$comment:abc123' ) ).to.be.true; + } ); + + it( 'should not allow the $comment attribute on a root that does not opt into $root attributes', () => { + model.schema.register( 'isolatedRoot', { + isLimit: true + } ); + + expect( model.schema.checkAttribute( [ 'isolatedRoot' ], '$comment' ) ).to.be.false; + expect( model.schema.checkAttribute( [ 'isolatedRoot' ], '$comment:abc123' ) ).to.be.false; + } ); + } ); + + describe( '$inlineRoot editor', () => { + let inlineEditor, inlineRoot; + + beforeEach( async () => { + inlineEditor = await VirtualTestEditor.create( { + plugins: [ HtmlComment ], + root: { modelElement: '$inlineRoot' } + } ); + + inlineRoot = inlineEditor.model.document.getRoot(); + } ); + + afterEach( () => { + return inlineEditor.destroy(); + } ); + + it( 'should preserve HTML comments around inline text through a setData/getData round trip', () => { + inlineEditor.setData( 'FooBar' ); + + expect( inlineEditor.getData() ).to.equal( 'FooBar' ); + } ); + + it( 'should store each HTML comment content as a $comment: attribute on the $inlineRoot', () => { + inlineEditor.setData( 'Foo' ); + + const commentAttributes = [ ...inlineRoot.getAttributeKeys() ] + .filter( attr => attr.startsWith( '$comment:' ) ) + .map( attr => inlineRoot.getAttribute( attr ) ); + + expect( commentAttributes ).to.have.members( [ ' alpha ', ' beta ' ] ); + } ); + + it( 'should create a $comment marker for each HTML comment upcast inside the $inlineRoot', () => { + inlineEditor.setData( 'FooBar' ); + + const commentMarkers = [ ...inlineEditor.model.markers ].filter( marker => marker.name.startsWith( '$comment:' ) ); + + expect( commentMarkers ).to.have.length( 1 ); + expect( commentMarkers[ 0 ].getStart().root ).to.equal( inlineRoot ); + } ); } ); describe( 'upcast conversion', () => { diff --git a/packages/ckeditor5-html-support/tests/integrations/heading.js b/packages/ckeditor5-html-support/tests/integrations/heading.js index 658dbe72542..b8b1c1ae09a 100644 --- a/packages/ckeditor5-html-support/tests/integrations/heading.js +++ b/packages/ckeditor5-html-support/tests/integrations/heading.js @@ -4,8 +4,11 @@ */ import { ClassicTestEditor } from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor.js'; -import { _getViewData } from '@ckeditor/ckeditor5-engine'; +import { Plugin } from '@ckeditor/ckeditor5-core'; +import { _getModelData, _getViewData } from '@ckeditor/ckeditor5-engine'; import { HeadingEditing } from '@ckeditor/ckeditor5-heading'; +import { Paragraph } from '@ckeditor/ckeditor5-paragraph'; +import { BlockQuote } from '@ckeditor/ckeditor5-block-quote'; import { GeneralHtmlSupport } from '../../src/generalhtmlsupport.js'; import { getModelDataWithAttributes } from '../_utils/utils.js'; import { HeadingElementSupport } from '../../src/integrations/heading.js'; @@ -89,7 +92,7 @@ describe( 'HeadingElementSupport', () => { model: 'htmlHgroup', view: 'hgroup', modelSchema: { - allowIn: [ '$root', '$container' ], + allowWhere: '$container', allowChildren: [ 'paragraph', 'htmlP', @@ -521,7 +524,7 @@ describe( 'HeadingElementSupport', () => { model: 'htmlHgroup', view: 'hgroup', modelSchema: { - allowIn: [ '$root', '$container' ], + allowWhere: '$container', allowChildren: [ 'paragraph', 'htmlP', @@ -943,4 +946,131 @@ describe( 'HeadingElementSupport', () => { ); } ); } ); + + describe( 'htmlHgroup placement (allowWhere: $container)', () => { + async function createEditorWithPlugins( plugins, config = {} ) { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + editor = await ClassicTestEditor.create( editorElement, { + plugins: [ Paragraph, HeadingEditing, GeneralHtmlSupport, ...plugins ], + ...config + } ); + + editor.plugins.get( 'DataFilter' ).loadAllowedConfig( [ { + name: /^(hgroup|h1|h2|h3|h4|h5|h6)$/, + attributes: true + } ] ); + + model = editor.model; + } + + it( 'should upcast hgroup placed directly in $root to htmlHgroup', async () => { + await createEditorWithPlugins( [] ); + + editor.setData( '

Sub

Title

' ); + + expect( _getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + + 'Sub' + + 'Title' + + '' + ); + } ); + + it( 'should downcast htmlHgroup back to hgroup when placed in $root', async () => { + await createEditorWithPlugins( [] ); + + const html = '

Sub

Title

'; + + editor.setData( html ); + + expect( editor.getData() ).to.equal( html ); + } ); + + it( 'should allow hgroup nested inside a $container-based element (blockQuote)', async () => { + await createEditorWithPlugins( [ BlockQuote ] ); + + editor.setData( + '
' + + '

Sub

Title

' + + '

Foo

' + + '
' + ); + + expect( _getModelData( model, { withoutSelection: true } ) ).to.equal( + '
' + + '' + + 'Sub' + + 'Title' + + '' + + 'Foo' + + '
' + ); + } ); + + it( 'should round-trip hgroup nested inside a blockQuote through setData/getData', async () => { + await createEditorWithPlugins( [ BlockQuote ] ); + + const html = + '
' + + '

Sub

Title

' + + '

Foo

' + + '
'; + + editor.setData( html ); + + expect( editor.getData() ).to.equal( html ); + } ); + + it( 'should allow hgroup in a custom root that opts into the $container chain', async () => { + class CustomRootSchema extends Plugin { + init() { + this.editor.model.schema.register( 'myRoot', { + inheritAllFrom: '$root' + } ); + + // Opt the custom root into the generic container chain so anything declaring + // `allowWhere: '$container'` (htmlHgroup, block-level GHS wrappers, etc.) can + // land inside it. + this.editor.model.schema.extend( '$container', { allowIn: 'myRoot' } ); + this.editor.model.schema.extend( '$block', { allowIn: 'myRoot' } ); + } + } + + await createEditorWithPlugins( [ CustomRootSchema ], { + root: { modelElement: 'myRoot' } + } ); + + editor.setData( '

Sub

Title

' ); + + // `setData` triggers DataFilter's deferred element registration, so the schema check is valid afterwards. + expect( model.schema.checkChild( [ 'myRoot' ], 'htmlHgroup' ) ).to.be.true; + + expect( _getModelData( model, { withoutSelection: true } ) ).to.equal( + '' + + 'Sub' + + 'Title' + + '' + ); + + expect( editor.getData() ).to.equal( '

Sub

Title

' ); + } ); + + it( 'should not allow hgroup in a custom root that does not opt into the $container chain', async () => { + class IsolatedRootSchema extends Plugin { + init() { + this.editor.model.schema.register( 'isolatedRoot', { + isLimit: true + } ); + } + } + + await createEditorWithPlugins( [ IsolatedRootSchema ], { + root: { modelElement: 'isolatedRoot' } + } ); + + expect( model.schema.checkChild( [ 'isolatedRoot' ], 'htmlHgroup' ) ).to.be.false; + } ); + } ); } ); diff --git a/packages/ckeditor5-watchdog/tests/editorwatchdog.js b/packages/ckeditor5-watchdog/tests/editorwatchdog.js index 3dd59b4b4bc..e31b2715044 100644 --- a/packages/ckeditor5-watchdog/tests/editorwatchdog.js +++ b/packages/ckeditor5-watchdog/tests/editorwatchdog.js @@ -1723,8 +1723,8 @@ describe( 'EditorWatchdog', () => { } ); expect( watchdog.editor.getRootsAttributes() ).to.deep.equal( { - header: { order: 1 }, - content: { order: 2 } + header: { order: 1, $rootEditableOptions: {} }, + content: { order: 2, $rootEditableOptions: {} } } ); await watchdog.destroy(); @@ -1775,8 +1775,8 @@ describe( 'EditorWatchdog', () => { } ); expect( watchdog.editor.getRootsAttributes() ).to.deep.equal( { - header: { order: 1 }, - new: { order: 3 } + header: { order: 1, $rootEditableOptions: {} }, + new: { order: 3, $rootEditableOptions: {} } } ); await watchdog.destroy(); @@ -1920,8 +1920,8 @@ describe( 'EditorWatchdog', () => { } ); expect( watchdog.editor.getRootsAttributes() ).to.deep.equal( { - header: { order: 1 }, - content: { order: 2 } + header: { order: 1, $rootEditableOptions: {} }, + content: { order: 2, $rootEditableOptions: {} } } ); } ); @@ -1947,8 +1947,8 @@ describe( 'EditorWatchdog', () => { } ); expect( watchdog.editor.getRootsAttributes() ).to.deep.equal( { - header: { order: 1 }, - new: { order: 3 } + header: { order: 1, $rootEditableOptions: {} }, + new: { order: 3, $rootEditableOptions: {} } } ); } ); @@ -1974,9 +1974,9 @@ describe( 'EditorWatchdog', () => { } ); expect( watchdog.editor.getRootsAttributes() ).to.deep.equal( { - header: { order: 1 }, - content: { order: 2 }, - lazyTwo: { order: 5 } + header: { order: 1, $rootEditableOptions: {} }, + content: { order: 2, $rootEditableOptions: {} }, + lazyTwo: { order: 5, $rootEditableOptions: {} } } ); } ); } ); @@ -2081,8 +2081,8 @@ describe( 'EditorWatchdog', () => { } ); expect( watchdog.editor.getRootsAttributes() ).to.deep.equal( { - header: { order: 1 }, - new: { order: 3 } + header: { order: 1, $rootEditableOptions: {} }, + new: { order: 3, $rootEditableOptions: {} } } ); } ); @@ -2108,9 +2108,9 @@ describe( 'EditorWatchdog', () => { } ); expect( watchdog.editor.getRootsAttributes() ).to.deep.equal( { - header: { order: 1 }, - content: { order: 2 }, - lazyTwo: { order: 5 } + header: { order: 1, $rootEditableOptions: {} }, + content: { order: 2, $rootEditableOptions: {} }, + lazyTwo: { order: 5, $rootEditableOptions: {} } } ); } ); } ); @@ -2148,8 +2148,8 @@ describe( 'EditorWatchdog', () => { } ); expect( watchdog.editor.getRootsAttributes() ).to.deep.equal( { - header: { order: 1 }, - content: { order: 2 } + header: { order: 1, $rootEditableOptions: { placeholder: 'Type in header' } }, + content: { order: 2, $rootEditableOptions: { placeholder: 'Type in content' } } } ); const editables = watchdog.editor.ui.view.editables; @@ -2188,8 +2188,8 @@ describe( 'EditorWatchdog', () => { } ); expect( watchdog.editor.getRootsAttributes() ).to.deep.equal( { - header: { order: 1 }, - content: { order: 2 } + header: { order: 1, $rootEditableOptions: { placeholder: 'Type in some content' } }, + content: { order: 2, $rootEditableOptions: { placeholder: 'Type in some content' } } } ); const editables = watchdog.editor.ui.view.editables; diff --git a/packages/ckeditor5-watchdog/tests/manual/watchdog-data.js b/packages/ckeditor5-watchdog/tests/manual/watchdog-data.js index 35d3b447a84..79cf67ad04f 100644 --- a/packages/ckeditor5-watchdog/tests/manual/watchdog-data.js +++ b/packages/ckeditor5-watchdog/tests/manual/watchdog-data.js @@ -65,12 +65,7 @@ function createWatchdog( editorElement, stateElement, name ) { const watchdog = new EditorWatchdog( ClassicEditor ); watchdog.setCreator( config => { - return ClassicEditor.create( { - ...config, - root: { - initialData: editorElement.innerHTML - } - } ).then( editor => { + return ClassicEditor.create( config ).then( editor => { console.log( `${ name } editor created (from creator).` ); editorElement.innerHTML = ''; @@ -87,7 +82,12 @@ function createWatchdog( editorElement, stateElement, name ) { return editor.destroy(); } ); - watchdog.create( editorConfig ); + watchdog.create( { + ...editorConfig, + root: { + initialData: editorElement.innerHTML + } + } ); watchdog.on( 'error', () => { console.log( `${ name } editor crashed!` ); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f38c543960c..9fcc72ee4ae 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -110,14 +110,14 @@ importers: specifier: ^9.34.0 version: 9.36.0(jiti@2.6.1) eslint-config-ckeditor5: - specifier: ^15.0.0 - version: 15.0.0(eslint@9.36.0(jiti@2.6.1))(typescript@5.5.4) + specifier: ^15.1.0 + version: 15.1.0(eslint@9.36.0(jiti@2.6.1))(typescript@5.5.4) eslint-formatter-stylish: specifier: ^8.40.0 version: 8.40.0 eslint-plugin-ckeditor5-rules: - specifier: ^15.0.0 - version: 15.0.0 + specifier: ^15.1.0 + version: 15.1.0 estree-walker: specifier: ^3.0.3 version: 3.0.3 @@ -8032,8 +8032,8 @@ packages: engines: {node: '>=6.0'} hasBin: true - eslint-config-ckeditor5@15.0.0: - resolution: {integrity: sha512-XpemPdkTE96OIRIBNbbVovqw/KejKheCHL3tlil0vz8mDdVp/TO7Cj81GEvHz2jdoR4SuY1GCTFUp8FzMRhdHg==} + eslint-config-ckeditor5@15.1.0: + resolution: {integrity: sha512-5Ac2JYj6QsvUngH6d+AzFxIX3VECTuwDmXV5InY9I8riNsSdc2msOOry9pov2yTEUc+CaoJA5Uy5ggkDvdMYpw==} engines: {node: '>=24.11.0'} peerDependencies: eslint: ^9.0.0 @@ -8043,8 +8043,8 @@ packages: resolution: {integrity: sha512-blbD5ZSQnjNEUaG38VCO4WG9nfDQWE8/IOmt8DFRHXUIfZikaIXmsQTdWNFk0/e0j7RgIVRza86MpsJ+aHgFLg==} engines: {node: ^12.22.0 || ^14.17.0 || >=16.0.0} - eslint-plugin-ckeditor5-rules@15.0.0: - resolution: {integrity: sha512-naZRNFCpiEhanWKaCqqgxB9hlx+K8RmKATkVwxt4z7nPQnPjg5Yh2UYAoBtCBXhG9iiHclR35T4b694o0KynTw==} + eslint-plugin-ckeditor5-rules@15.1.0: + resolution: {integrity: sha512-qD0dNIMGVX0I5ntUGX0XSGsq2P6z8qGHHRIcA5eScZDxzCZ/Me2mpjLDd1WPU/1C0gmeuzjfFKWpd3xbB3WGUg==} engines: {node: '>=24.11.0'} eslint-plugin-mocha@11.2.0: @@ -16708,13 +16708,13 @@ snapshots: optionalDependencies: source-map: 0.6.1 - eslint-config-ckeditor5@15.0.0(eslint@9.36.0(jiti@2.6.1))(typescript@5.5.4): + eslint-config-ckeditor5@15.1.0(eslint@9.36.0(jiti@2.6.1))(typescript@5.5.4): dependencies: '@eslint/js': 9.39.4 '@eslint/markdown': 6.6.0 '@stylistic/eslint-plugin': 4.4.1(eslint@9.36.0(jiti@2.6.1))(typescript@5.5.4) eslint: 9.36.0(jiti@2.6.1) - eslint-plugin-ckeditor5-rules: 15.0.0 + eslint-plugin-ckeditor5-rules: 15.1.0 eslint-plugin-mocha: 11.2.0(eslint@9.36.0(jiti@2.6.1)) globals: 16.5.0 typescript: 5.5.4 @@ -16728,7 +16728,7 @@ snapshots: strip-ansi: 6.0.1 text-table: 0.2.0 - eslint-plugin-ckeditor5-rules@15.0.0: + eslint-plugin-ckeditor5-rules@15.1.0: dependencies: '@es-joy/jsdoccomment': 0.50.2 enhanced-resolve: 5.20.1