Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
20 changes: 19 additions & 1 deletion packages/ckeditor5-clipboard/tests/pasteplaintext.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ describe( 'PastePlainText', () => {
expect( _getModelData( model ) ).to.equal( '<paragraph><$text test="true">Linked foo[].</$text></paragraph>' );
} );

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, '<paragraph><$text test="true">[Linked text.]</$text></paragraph>' );

viewDocument.fire( 'clipboardInput', {
Expand All @@ -258,6 +258,24 @@ describe( 'PastePlainText', () => {
expect( _getModelData( model ) ).to.equal( '<paragraph>foo[]</paragraph>' );
} );

it( 'should not preserve non formatting attribute if the entire content was fully selected across multiple paragraphs', () => {
_setModelData(
model,
'<paragraph><$text test="true">[Linked text.</$text></paragraph><paragraph><$text test="true">Other text.]</$text></paragraph>'
);

viewDocument.fire( 'clipboardInput', {
dataTransfer: createDataTransfer( {
'text/html': 'foo',
'text/plain': 'foo'
} ),
stopPropagation() {},
preventDefault() {}
} );

expect( _getModelData( model ) ).to.equal( '<paragraph>foo[]</paragraph>' );
} );

it( 'should not treat a pasted object as a plain text', () => {
model.schema.register( 'obj', {
allowWhere: '$block',
Expand Down
60 changes: 60 additions & 0 deletions packages/ckeditor5-engine/src/model/utils/deletecontent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -163,6 +173,10 @@ export function deleteContent(
insertParagraph( writer, startPosition, selection, attributesForAutoparagraph );
}

if ( selectionIsDocumentSelection ) {
restoreSelectionAttributesOnEmptyParent( writer, selectionAttributes, selectionParentWasEmpty );
}

startPosition.detach();
endPosition.detach();
} );
Expand Down Expand Up @@ -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 );
}
}
}
125 changes: 121 additions & 4 deletions packages/ckeditor5-engine/tests/model/utils/deletecontent.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,18 @@ 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';

describe( 'DataController utils', () => {
let model, doc;

afterEach( () => {
sinon.restore();
} );

describe( 'deleteContent', () => {
it( 'should use parent batch', () => {
model = new Model();
Expand Down Expand Up @@ -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)', () => {
Expand All @@ -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,
'<paragraph>x</paragraph><paragraph>[<$text bold="true">foo</$text>]</paragraph><paragraph>y</paragraph>'
'<paragraph>x</paragraph><paragraph><$text bold="true">[foo]</$text></paragraph><paragraph>y</paragraph>'
);

deleteContent( model, doc.selection );

expect( _getModelData( model ) ).to.equal( '<paragraph>x</paragraph><paragraph>[]</paragraph><paragraph>y</paragraph>' );
expect( doc.selection.getAttribute( 'bold' ) ).to.undefined;
expect( _getModelData( model ) ).to.equal(
'<paragraph>x</paragraph>' +
'<paragraph selection:bold="true"><$text bold="true">[]</$text></paragraph>' +
'<paragraph>y</paragraph>'
);
expect( doc.selection.getAttribute( 'bold' ) ).to.equal( true );
} );

it( 'leaves selection attributes when text contains them', () => {
Expand All @@ -194,6 +206,111 @@ describe( 'DataController utils', () => {
expect( _getModelData( model ) ).to.equal( '<paragraph>x<$text bold="true">a[]b</$text>y</paragraph>' );
expect( doc.selection.getAttribute( 'bold' ) ).to.equal( true );
} );

it( 'clears selection attrs when replacing the entire content with a paragraph', () => {
_setModelData(
model,
'<paragraph>[<$text bold="true">foo</$text></paragraph><paragraph>bar]</paragraph>',
{
selectionAttributes: {
bold: true
}
}
);

deleteContent( model, doc.selection );

expect( _getModelData( model ) ).to.equal( '<paragraph>[]</paragraph>' );
expect( doc.selection.getAttribute( 'bold' ) ).to.undefined;
} );

it( 'preserves selection attrs when deleting the entire content of a single paragraph', () => {
_setModelData(
model,
'<paragraph>[<$text bold="true">foo</$text>]</paragraph>',
{
selectionAttributes: {
bold: true
}
}
);

deleteContent( model, doc.selection );

expect( _getModelData( model ) ).to.equal(
'<paragraph selection:bold="true"><$text bold="true">[]</$text></paragraph>'
);
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,
'<paragraph>[]</paragraph>' +
'<paragraph>foo</paragraph>' +
'<paragraph>bar</paragraph>',
{
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(
'<paragraph selection:bold="true"><$text bold="true">[]</$text></paragraph>' +
'<paragraph></paragraph>' +
'<paragraph>bar</paragraph>'
);
} );

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,
'<paragraph>x</paragraph>' +
'<paragraph>[]</paragraph>' +
'<paragraph><$text bold="true">foo</$text></paragraph>' +
'<paragraph>y</paragraph>'
);

// 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.
Expand Down
20 changes: 18 additions & 2 deletions packages/ckeditor5-list/tests/listformatting.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
'<paragraph listIndent="0" listItemFormat="foo" listItemId="a" listType="numbered">' +
'[<$text inlineFormat="foo">foo</$text>]' +
Expand All @@ -446,7 +446,23 @@ describe( 'ListFormatting', () => {
editor.execute( 'delete' );

expect( _getModelData( model, { withoutSelection: true } ) ).to.equalMarkup(
'<paragraph listIndent="0" listItemId="a" listType="numbered"></paragraph>'
'<paragraph listIndent="0" listItemFormat="foo" listItemId="a" listType="numbered" selection:inlineFormat="foo">' +
'</paragraph>'
);
} );

it( 'should remove attribute from li if the entire content was removed from multiple list items', () => {
_setModelData( model,
'<paragraph listIndent="0" listItemFormat="foo" listItemId="a" listType="numbered">' +
'[<$text inlineFormat="foo">foo</$text></paragraph>' +
'<paragraph listIndent="0" listItemFormat="foo" listItemId="b" listType="numbered">' +
'<$text inlineFormat="foo">bar</$text>]</paragraph>'
);

editor.execute( 'delete' );

expect( _getModelData( model, { withoutSelection: true } ) ).to.equalMarkup(
'<paragraph></paragraph>'
);
} );

Expand Down