Przeglądaj źródła

Tests: Added the selection-post-fixer test to make sure it does not set the new selection if no range was "post-fixed".

Aleksander Nowodzinski 5 lat temu
rodzic
commit
e9d5a0d351

+ 7 - 3
packages/ckeditor5-engine/src/model/utils/selection-post-fixer.js

@@ -82,9 +82,13 @@ function selectionPostFixer( writer, model ) {
 		// Those ranges might overlap but will be corrected later.
 		const correctedRange = tryFixingRange( modelRange, schema );
 
-		// It may happen that a new range is returned but in fact it has the same positions as the original
-		// range anyway. If this branch was not discarded, a new selection would be set and that, for instance,
-		// would destroy the selection' attributes.
+		// "Selection fixing" algorithms sometimes get lost. In consequence, it may happen
+		// that a new range is returned but, in fact, it has the same positions as the original
+		// range anyway. If this range is not discarded, a new selection will be set and that,
+		// for instance, would destroy the selection attributes. Let's make sure that the post-fixer
+		// actually worked first before setting a new selection.
+		//
+		// https://github.com/ckeditor/ckeditor5/issues/6693
 		if ( correctedRange && !correctedRange.isEqual( modelRange ) ) {
 			ranges.push( correctedRange );
 			wasFixed = true;

+ 29 - 0
packages/ckeditor5-engine/tests/model/utils/selection-post-fixer.js

@@ -835,6 +835,35 @@ describe( 'Selection post-fixer', () => {
 					'</table>]'
 				);
 			} );
+
+			it( 'should not reset the selection if the final range is the same as the initial one', () => {
+				setModelData( model,
+					'<table>' +
+						'<tableRow>' +
+							'<tableCell>[<image></image>]</tableCell>' +
+						'</tableRow>' +
+					'</table>'
+				);
+
+				// Setting a selection attribute will trigger the post-fixer. However, because this
+				// action does not affect ranges, the post-fixer should not set a new selection and,
+				// in consequence, should not clear the selection attribute (like it normally would when
+				// a new selection is set).
+				// https://github.com/ckeditor/ckeditor5/issues/6693
+				model.change( writer => {
+					writer.setSelectionAttribute( 'foo', 'bar' );
+				} );
+
+				assertEqualMarkup( getModelData( model ),
+					'<table>' +
+						'<tableRow>' +
+							'<tableCell>[<image></image>]</tableCell>' +
+						'</tableRow>' +
+					'</table>'
+				);
+
+				expect( model.document.selection.hasAttribute( 'foo' ) ).to.be.true;
+			} );
 		} );
 
 		describe( 'non-collapsed selection - image scenarios', () => {