Merge pull request #14685 from ckeditor/ck/13892-find-and-replace-rep…

…lace-all-and-undo Fix (find-and-replace): Undo should restore every text occurrences replaced by replace all in the document at once. Closes #13892.
ckeditor · Jul 28, 2023 · cfab99d · cfab99d
2 parents 1a58a6b + 044ca7e
commit cfab99d
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 4 deletions.
diff --git a/packages/ckeditor5-find-and-replace/src/replaceallcommand.ts b/packages/ckeditor5-find-and-replace/src/replaceallcommand.ts
@@ -50,9 +50,12 @@ export default class ReplaceAllCommand extends ReplaceCommandBase {
 				) ), null as Collection<ResultType> | null )!;
 
 		if ( results.length ) {
-			[ ...results ].forEach( searchResult => {
-				// Just reuse logic from the replace command to replace a single match.
-				this._replace( newText, searchResult );
+			// Wrapped in single change will batch it into one transaction.
+			model.change( () => {
+				[ ...results ].forEach( searchResult => {
+					// Just reuse logic from the replace command to replace a single match.
+					this._replace( newText, searchResult );
+				} );
 			} );
 		}
 	}

diff --git a/packages/ckeditor5-find-and-replace/tests/replaceallcommand.js b/packages/ckeditor5-find-and-replace/tests/replaceallcommand.js
@@ -66,7 +66,7 @@ describe( 'ReplaceAllCommand', () => {
 			expect( editor.getData() ).to.equal( '<p>Foo bar baz</p><p>Foo bar baz</p>' );
 		} );
 
-		it( 'should replace all passed results in the document document', () => {
+		it( 'should replace all passed results in the document', () => {
 			setData( model, '<paragraph>Foo bar [b]az</paragraph><paragraph>[Foo] bar baz</paragraph>' );
 
 			const ranges = editor.model.document.selection.getRanges();
@@ -154,5 +154,47 @@ describe( 'ReplaceAllCommand', () => {
 
 			expect( getData( editor.model, { withoutSelection: true } ) ).to.equal( '<paragraph>Aaa Boo Coo Daa</paragraph>' );
 		} );
+
+		it( 'should restore every text occurrences replaced by `replace all` in the document at one undo step', () => {
+			setData( model, '<paragraph>Foo bar baz</paragraph><paragraph>Foo bar baz</paragraph><paragraph>Foo bar baz</paragraph>' );
+
+			editor.execute( 'replaceAll', 'new', 'bar' );
+
+			expect( editor.getData() ).to.equal( '<p>Foo new baz</p><p>Foo new baz</p><p>Foo new baz</p>' );
+
+			editor.execute( 'undo' );
+
+			expect( editor.getData() ).to.equal( '<p>Foo bar baz</p><p>Foo bar baz</p><p>Foo bar baz</p>' );
+		} );
+
+		it( 'should restore every text occurrences replaced by `replace all` in multiple roots at one undo step', async () => {
+			class MultiRootEditor extends ModelTestEditor {
+				constructor( config ) {
+					super( config );
+
+					this.model.document.createRoot( '$root', 'second' );
+				}
+			}
+
+			const multiRootEditor = await MultiRootEditor
+				.create( { plugins: [ FindAndReplaceEditing, Paragraph, UndoEditing ] } );
+
+			setData( multiRootEditor.model, '<paragraph>Foo bar baz</paragraph>', { rootName: 'main' } );
+			setData( multiRootEditor.model, '<paragraph>Ra baz baz</paragraph>', { rootName: 'second' } );
+
+			const { results } = multiRootEditor.execute( 'find', 'z' );
+
+			multiRootEditor.execute( 'replaceAll', 'r', results );
+
+			expect( multiRootEditor.getData( { rootName: 'main' } ) ).to.equal( '<p>Foo bar bar</p>' );
+			expect( multiRootEditor.getData( { rootName: 'second' } ) ).to.equal( '<p>Ra bar bar</p>' );
+
+			multiRootEditor.execute( 'undo' );
+
+			expect( multiRootEditor.getData( { rootName: 'main' } ) ).to.equal( '<p>Foo bar baz</p>' );
+			expect( multiRootEditor.getData( { rootName: 'second' } ) ).to.equal( '<p>Ra baz baz</p>' );
+
+			await multiRootEditor.destroy();
+		} );
 	} );
 } );