diff --git a/.changelog/20260402080927_ck_10517_preserve_formatting_delete_content.md b/.changelog/20260402080927_ck_10517_preserve_formatting_delete_content.md new file mode 100644 index 00000000000..8a98e1f2930 --- /dev/null +++ b/.changelog/20260402080927_ck_10517_preserve_formatting_delete_content.md @@ -0,0 +1,10 @@ +--- +type: Fix +scope: + - ckeditor5-engine +closes: + - 10517 + - 19777 +--- + +Should preserve formatting (for example, bold or italic) after deleting content that empties a block so that typing continues with the same formatting. diff --git a/packages/ckeditor5-clipboard/tests/pasteplaintext.js b/packages/ckeditor5-clipboard/tests/pasteplaintext.js index 653190c472a..d6bf51f1661 100644 --- a/packages/ckeditor5-clipboard/tests/pasteplaintext.js +++ b/packages/ckeditor5-clipboard/tests/pasteplaintext.js @@ -243,7 +243,7 @@ describe( 'PastePlainText', () => { expect( _getModelData( model ) ).to.equal( '<$text test="true">Linked foo[].' ); } ); - it( 'should not preserve non formatting attribute if it was fully selected', () => { + it( 'should not preserve non formatting attribute if it was fully selected in a single paragraph', () => { _setModelData( model, '<$text test="true">[Linked text.]' ); viewDocument.fire( 'clipboardInput', { @@ -258,6 +258,24 @@ describe( 'PastePlainText', () => { expect( _getModelData( model ) ).to.equal( 'foo[]' ); } ); + it( 'should not preserve non formatting attribute if the entire content was fully selected across multiple paragraphs', () => { + _setModelData( + model, + '<$text test="true">[Linked text.<$text test="true">Other text.]' + ); + + viewDocument.fire( 'clipboardInput', { + dataTransfer: createDataTransfer( { + 'text/html': 'foo', + 'text/plain': 'foo' + } ), + stopPropagation() {}, + preventDefault() {} + } ); + + expect( _getModelData( model ) ).to.equal( 'foo[]' ); + } ); + it( 'should not treat a pasted object as a plain text', () => { model.schema.register( 'obj', { allowWhere: '$block', diff --git a/packages/ckeditor5-engine/src/model/utils/deletecontent.ts b/packages/ckeditor5-engine/src/model/utils/deletecontent.ts index 484365c7815..8b80011ebf2 100644 --- a/packages/ckeditor5-engine/src/model/utils/deletecontent.ts +++ b/packages/ckeditor5-engine/src/model/utils/deletecontent.ts @@ -98,6 +98,16 @@ export function deleteContent( } const schema = model.schema; + const documentSelection = model.document.selection; + + // Only restore selection attributes when the provided selection targets the same range as the document + // selection. We compare ranges rather than instances because CKEditor may pass a transient copy of the + // document selection (same range, but a different object without stored attributes). When the ranges + // differ, the caller is operating on a synthetic selection elsewhere in the document and we must not + // touch the document selection attributes. + const selectionIsDocumentSelection = !!documentSelection.getFirstRange()?.isEqual( selRange ); + const selectionAttributes = Array.from( documentSelection.getAttributes() ); + const selectionParentWasEmpty = !!documentSelection.getFirstRange()?.start.parent.isEmpty; model.change( writer => { // 1. Replace the entire content with paragraph. @@ -163,6 +173,10 @@ export function deleteContent( insertParagraph( writer, startPosition, selection, attributesForAutoparagraph ); } + if ( selectionIsDocumentSelection ) { + restoreSelectionAttributesOnEmptyParent( writer, selectionAttributes, selectionParentWasEmpty ); + } + startPosition.detach(); endPosition.detach(); } ); @@ -627,3 +641,49 @@ function collapseSelectionAt( selection.setTo( positionOrRange ); } } + +/** + * Restores the document selection attributes after a deletion that leaves the selection in an empty parent block. + * This preserves the pre-delete formatting (e.g. bold, italic) so that subsequent typing continues in the same style. + * + * Attributes are only restored when: + * - There were attributes on the selection before the deletion. + * - The deletion left the document selection's parent block empty. + * - The parent block was **not** already empty before the deletion — this ensures that attributes are not + * re-applied when `deleteContent()` was called on a completely unrelated block. + */ +function restoreSelectionAttributesOnEmptyParent( + writer: ModelWriter, + selectionAttributes: Array<[ string, unknown ]>, + selectionParentWasEmpty: boolean +) { + if ( !selectionAttributes.length ) { + return; + } + + const documentSelection = writer.model.document.selection; + + const selectionParent = documentSelection.anchor!.parent as ModelElement; + + if ( !selectionParent.isEmpty ) { + return; + } + + // Preserve attributes only when the delete operation leaves the live selection in an empty parent + // that was not empty before the change. This avoids reasserting attributes on unrelated empty blocks + // when deleteContent() operates on a synthetic selection somewhere else in the document. + if ( selectionParentWasEmpty ) { + return; + } + + // Setting document selection attributes here also persists them as `selection:*` + // on the empty parent, so future typing keeps the pre-delete formatting. + for ( const [ key, value ] of selectionAttributes ) { + if ( + writer.model.schema.getAttributeProperties( key ).isFormatting && + writer.model.schema.checkAttributeInSelection( documentSelection, key ) + ) { + writer.setSelectionAttribute( key, value ); + } + } +} diff --git a/packages/ckeditor5-engine/tests/model/utils/deletecontent.js b/packages/ckeditor5-engine/tests/model/utils/deletecontent.js index dcd07e72ad7..24b83989bc0 100644 --- a/packages/ckeditor5-engine/tests/model/utils/deletecontent.js +++ b/packages/ckeditor5-engine/tests/model/utils/deletecontent.js @@ -8,6 +8,7 @@ import { ModelPosition } from '../../../src/model/position.js'; import { ModelRange } from '../../../src/model/range.js'; import { ModelSelection } from '../../../src/model/selection.js'; import { ModelElement } from '../../../src/model/element.js'; +import { ModelWriter } from '../../../src/model/writer.js'; import { deleteContent } from '../../../src/model/utils/deletecontent.js'; import { _setModelData, _getModelData } from '../../../src/dev-utils/model.js'; import { _stringifyView } from '../../../src/dev-utils/view.js'; @@ -15,6 +16,10 @@ import { _stringifyView } from '../../../src/dev-utils/view.js'; describe( 'DataController utils', () => { let model, doc; + afterEach( () => { + sinon.restore(); + } ); + describe( 'deleteContent', () => { it( 'should use parent batch', () => { model = new Model(); @@ -147,6 +152,9 @@ describe( 'DataController utils', () => { allowIn: '$root', allowAttributes: [ 'bold', 'italic' ] } ); + + schema.setAttributeProperties( 'bold', { isFormatting: true } ); + schema.setAttributeProperties( 'italic', { isFormatting: true } ); } ); it( 'deletes characters (first half has attrs)', () => { @@ -167,15 +175,19 @@ describe( 'DataController utils', () => { expect( doc.selection.getAttribute( 'bold' ) ).to.undefined; } ); - it( 'clears selection attrs when emptied content', () => { + it( 'preserves selection attrs when emptied content', () => { _setModelData( model, - 'x[<$text bold="true">foo]y' + 'x<$text bold="true">[foo]y' ); deleteContent( model, doc.selection ); - expect( _getModelData( model ) ).to.equal( 'x[]y' ); - expect( doc.selection.getAttribute( 'bold' ) ).to.undefined; + expect( _getModelData( model ) ).to.equal( + 'x' + + '<$text bold="true">[]' + + 'y' + ); + expect( doc.selection.getAttribute( 'bold' ) ).to.equal( true ); } ); it( 'leaves selection attributes when text contains them', () => { @@ -194,6 +206,111 @@ describe( 'DataController utils', () => { expect( _getModelData( model ) ).to.equal( 'x<$text bold="true">a[]by' ); expect( doc.selection.getAttribute( 'bold' ) ).to.equal( true ); } ); + + it( 'clears selection attrs when replacing the entire content with a paragraph', () => { + _setModelData( + model, + '[<$text bold="true">foobar]', + { + selectionAttributes: { + bold: true + } + } + ); + + deleteContent( model, doc.selection ); + + expect( _getModelData( model ) ).to.equal( '[]' ); + expect( doc.selection.getAttribute( 'bold' ) ).to.undefined; + } ); + + it( 'preserves selection attrs when deleting the entire content of a single paragraph', () => { + _setModelData( + model, + '[<$text bold="true">foo]', + { + selectionAttributes: { + bold: true + } + } + ); + + deleteContent( model, doc.selection ); + + expect( _getModelData( model ) ).to.equal( + '<$text bold="true">[]' + ); + expect( doc.selection.getAttribute( 'bold' ) ).to.equal( true ); + } ); + + it( 'does not restore attrs when the live selection is already in an unrelated empty paragraph', () => { + const setSelectionAttributeSpy = sinon.spy( ModelWriter.prototype, 'setSelectionAttribute' ); + + _setModelData( + model, + '[]' + + 'foo' + + 'bar', + { + selectionAttributes: { + bold: true + } + } + ); + setSelectionAttributeSpy.resetHistory(); + + const range = new ModelRange( + new ModelPosition( doc.getRoot(), [ 1, 0 ] ), + new ModelPosition( doc.getRoot(), [ 1, 3 ] ) + ); + + const selection = new ModelSelection( [ range ] ); + + deleteContent( model, selection ); + + expect( setSelectionAttributeSpy.called ).to.be.false; + expect( _getModelData( model ) ).to.equal( + '<$text bold="true">[]' + + '' + + 'bar' + ); + } ); + + it( 'does not restore attrs when the document selection anchor was in an already empty paragraph', () => { + const setSelectionAttributeSpy = sinon.spy( ModelWriter.prototype, 'setSelectionAttribute' ); + + // Paragraph 0 ("x") and paragraph 3 ("y") are outside the selection so that + // shouldEntireContentBeReplacedWithParagraph() returns false and the normal + // deletion path is taken. Paragraph 1 is empty – this is where the selection + // will be anchored. Paragraph 2 contains the content that will be deleted. + _setModelData( + model, + 'x' + + '[]' + + '<$text bold="true">foo' + + 'y' + ); + + // Extend the document selection so it is non-collapsed but still anchored + // inside the already-empty paragraph 1. + model.change( writer => { + const root = doc.getRoot(); + + writer.setSelection( writer.createRange( + writer.createPositionAt( root.getChild( 1 ), 0 ), + writer.createPositionAt( root.getChild( 2 ), 'end' ) + ) ); + writer.setSelectionAttribute( 'bold', true ); + } ); + + setSelectionAttributeSpy.resetHistory(); + + deleteContent( model, doc.selection ); + + // The anchor paragraph was already empty before the deletion, so attributes + // must not be restored – the user did not have the caret inside formatted content. + expect( setSelectionAttributeSpy.called ).to.be.false; + } ); } ); // Note: The algorithm does not care what kind of it's merging as it knows nothing useful about these elements. diff --git a/packages/ckeditor5-list/tests/listformatting.js b/packages/ckeditor5-list/tests/listformatting.js index 3eba1caaebe..e0988cbb5bd 100644 --- a/packages/ckeditor5-list/tests/listformatting.js +++ b/packages/ckeditor5-list/tests/listformatting.js @@ -436,7 +436,7 @@ describe( 'ListFormatting', () => { } ); describe( 'removing text node from a list item', () => { - it( 'should remove attribute from li if all formatted text is removed', () => { + it( 'should preserve attribute on li if all formatted text is removed from a single list item', () => { _setModelData( model, '' + '[<$text inlineFormat="foo">foo]' + @@ -446,7 +446,23 @@ describe( 'ListFormatting', () => { editor.execute( 'delete' ); expect( _getModelData( model, { withoutSelection: true } ) ).to.equalMarkup( - '' + '' + + '' + ); + } ); + + it( 'should remove attribute from li if the entire content was removed from multiple list items', () => { + _setModelData( model, + '' + + '[<$text inlineFormat="foo">foo' + + '' + + '<$text inlineFormat="foo">bar]' + ); + + editor.execute( 'delete' ); + + expect( _getModelData( model, { withoutSelection: true } ) ).to.equalMarkup( + '' ); } );