Merge pull request #17292 from ckeditor/ck/16648

Fix (find-and-replace): Find and replace no longer randomly jumps to the first found item after the replace operation. Closes #16648

packages/ckeditor5-find-and-replace/src/findandreplaceediting.ts

@@ -292,7 +292,7 @@ export default class FindAndReplaceEditing extends Plugin {
 			this.state!.highlightedResult = changedSearchResults[ 0 ];
 		} else {
 			// If there is already highlight item then refresh highlight offset after appending new items.
-			this.state!.refreshHighlightOffset();
+			this.state!.refreshHighlightOffset( model );
 		}
 	};
 }

packages/ckeditor5-find-and-replace/src/findandreplacestate.ts

@@ -118,7 +118,7 @@ export default class FindAndReplaceState extends /* #__PURE__ */ ObservableMixin
 		} );
 
 		this.on<ObservableChangeEvent<ResultType | null>>( 'change:highlightedResult', ( ) => {
-			this.refreshHighlightOffset();
+			this.refreshHighlightOffset( model );
 		} );
 	}
 
@@ -149,20 +149,35 @@ export default class FindAndReplaceState extends /* #__PURE__ */ ObservableMixin
 	/**
 	 * Refreshes the highlight result offset based on it's index within the result list.
 	 */
-	public refreshHighlightOffset(): void {
+	public refreshHighlightOffset( model: Model ): void {
 		const { highlightedResult, results } = this;
-		const sortMapping = { before: -1, same: 0, after: 1, different: 1 };
 
 		if ( highlightedResult ) {
-			this.highlightedOffset = Array.from( results )
-				.sort( ( a, b ) => sortMapping[ a.marker!.getStart().compareWith( b.marker!.getStart() ) ] )
-				.indexOf( highlightedResult ) + 1;
+			this.highlightedOffset = sortSearchResultsByMarkerPositions( model, [ ...results ] ).indexOf( highlightedResult ) + 1;
 		} else {
 			this.highlightedOffset = 0;
 		}
 	}
 }
 
+/**
+ * Sorts search results by marker positions. Make sure that the results are sorted in the same order as they appear in the document
+ * to avoid issues with the `find next` command. Apparently, the order of the results in the state might be different than the order
+ * of the markers in the model.
+ */
+export function sortSearchResultsByMarkerPositions( model: Model, results: Array<ResultType> ): Array<ResultType> {
+	const sortMapping = { before: -1, same: 0, after: 1, different: 1 };
+
+	// `compareWith` doesn't play well with multi-root documents, so we need to sort results by root name first
+	// and then sort them within each root. It prevents "random" order of results when the document has multiple roots.
+	// See more: https://github.com/ckeditor/ckeditor5/pull/17292#issuecomment-2442084549
+	return model.document.getRootNames().flatMap( rootName =>
+		results
+			.filter( result => result.marker!.getStart().root.rootName === rootName )
+			.sort( ( a, b ) => sortMapping[ a.marker!.getStart().compareWith( b.marker!.getStart() ) ] )
+	);
+}
+
 /**
  * The callback function used to find matches in the document.
  */

packages/ckeditor5-find-and-replace/src/replacecommand.ts

@@ -8,6 +8,7 @@
 */
 
 import type { ResultType } from './findandreplace.js';
+import { sortSearchResultsByMarkerPositions } from './findandreplacestate.js';
 import { ReplaceCommandBase } from './replacecommandbase.js';
 
 /**
@@ -22,6 +23,31 @@ export default class ReplaceCommand extends ReplaceCommandBase {
 	 * @fires execute
 	 */
 	public override execute( replacementText: string, result: ResultType ): void {
+		// We save highlight offset here, as the information about the highlighted result will be lost after the changes.
+		//
+		// It happens because result list is partially regenerated if the result is removed from the paragraph.
+		// Partially means that all sibling result items that are placed in the same paragraph are removed and added again,
+		// which causes the highlighted result to be malformed (usually it's set to first but it's not guaranteed).
+		//
+		// While this saving can be done in editing state, it's better to keep it here, as it's a part of the command logic
+		// and might be super tricky to implement in multi-root documents.
+		//
+		// Keep in mind that the highlighted offset is indexed from 1, as it's displayed to the user. It's why we subtract 1 here.
+		//
+		// More info: https://github.com/ckeditor/ckeditor5/issues/16648
+		const oldHighlightOffset = Math.max( this._state!.highlightedOffset - 1, 0 );
+
 		this._replace( replacementText, result );
+
+		// Let's revert the highlight offset to the previous value.
+		if ( this._state!.results.length ) {
+			// Highlight offset operates on sorted array, so we need to sort the results first.
+			// It's not guaranteed that items in state results are sorted, usually they are, but it's not guaranteed when
+			// the result is removed from the paragraph with other highlighted results.
+			const sortedResults = sortSearchResultsByMarkerPositions( this.editor.model, [ ...this._state!.results ] );
+
+			// Just make sure that we don't overflow the results array, so use modulo.
+			this._state!.highlightedResult = sortedResults[ oldHighlightOffset % sortedResults.length ];
+		}
 	}
 }

packages/ckeditor5-find-and-replace/tests/ui/findandreplaceformview.js

@@ -1286,7 +1286,7 @@ describe( 'FindAndReplaceFormView', () => {
 				expect( matchCounterElement.textContent ).to.equal( '2 of 3' );
 
 				replaceButton.fire( 'execute' );
-				expect( matchCounterElement.textContent ).to.equal( '1 of 2' );
+				expect( matchCounterElement.textContent ).to.equal( '2 of 2' );
 
 				replaceButton.fire( 'execute' );
 				expect( matchCounterElement.textContent ).to.equal( '1 of 1' );
@@ -1357,6 +1357,47 @@ describe( 'FindAndReplaceFormView', () => {
 				editor.execute( 'undo' );
 				expect( matchCounterElement.textContent ).to.equal( '4 of 4' );
 			} );
+
+			it( 'should keep highlighted offset after replacement', () => {
+				editor.setData(
+					`<p>
+						Chocolate <span style="color:#fff700;">cake</span> bar ice cream topping marzipan.
+						Powder gingerbread bear claw tootsie roll lollipop marzipan icing bonbon.
+					</p>
+					<p>
+						Chupa chups jelly beans halvah ice cream gingerbread bears candy halvah gummi bears.
+						cAke dragée dessert chocolate.
+					</p>
+					<p>
+						Sime text with text highlight: <mark class="marker-green">Chocolate</mark> bonbon
+						<mark class="marker-yellow">Chocolate</mark> ice cream <mark class="marker-blue">Chocolate</mark>
+						gummies <mark class="pen-green">Chocolate</mark> tootsie roll
+					</p>`
+				);
+
+				toggleDialog();
+
+				// Let's skip the first found item.
+				findInput.fieldView.value = 'Choco';
+				findButton.fire( 'execute' );
+				findNextButton.fire( 'execute' );
+
+				// Replace second and third one.
+				view._replaceInputView.fieldView.value = '###';
+
+				replaceButton.fire( 'execute' );
+				replaceButton.fire( 'execute' );
+
+				// And there check if the highlight is still in the right place.
+				replaceButton.fire( 'execute' );
+
+				expect( editor.getData() ).to.be.equal(
+					'<p>Chocolate cake bar ice cream topping marzipan. Powder gingerbread bear claw tootsie roll' +
+					' lollipop marzipan icing bonbon.</p><p>Chupa chups jelly beans halvah ice cream gingerbread ' +
+					'bears candy halvah gummi bears. cAke dragée dessert ###late.</p><p>Sime text with text highlight: ' +
+					'###late bonbon ###late ice cream Chocolate gummies Chocolate tootsie roll</p>'
+				);
+			} );
 		} );
 
 		describe( 'dirty state of the form', () => {