Explorar o código

Range order should not affect how various algorithms work.

Piotrek Koszuliński %!s(int64=5) %!d(string=hai) anos
pai
achega
53d7458af8

+ 17 - 1
packages/ckeditor5-table/src/utils.js

@@ -81,7 +81,7 @@ export function getTableWidgetAncestor( selection ) {
 export function getSelectedTableCells( selection ) {
 export function getSelectedTableCells( selection ) {
 	const cells = [];
 	const cells = [];
 
 
-	for ( const range of selection.getRanges() ) {
+	for ( const range of sortRanges( selection.getRanges() ) ) {
 		const element = range.getContainedElement();
 		const element = range.getContainedElement();
 
 
 		if ( element && element.is( 'tableCell' ) ) {
 		if ( element && element.is( 'tableCell' ) ) {
@@ -136,3 +136,19 @@ export function getSelectionAffectedTableCells( selection ) {
 
 
 	return getTableCellsContainingSelection( selection );
 	return getTableCellsContainingSelection( selection );
 }
 }
+
+function sortRanges( rangesIterator ) {
+	return Array.from( rangesIterator ).sort( compareRangeOrder );
+}
+
+function compareRangeOrder( rangeA, rangeB ) {
+	// Since table cell ranges are disjoint, it's enough to check their start positions.
+	const posA = rangeA.start;
+	const posB = rangeB.start;
+
+	if ( posA.isEqual( posB ) ) {
+		return 0;
+	}
+
+	return posA.isBefore( posB ) ? -1 : 1;
+}

+ 1 - 1
packages/ckeditor5-table/tests/commands/removerowcommand.js

@@ -281,7 +281,7 @@ describe( 'RemoveRowCommand', () => {
 				command.execute();
 				command.execute();
 
 
 				assertEqualMarkup( getData( model ), modelTable( [
 				assertEqualMarkup( getData( model ), modelTable( [
-					[ '10', '[]11' ]
+					[ '[]10', '11' ]
 				] ) );
 				] ) );
 			} );
 			} );
 		} );
 		} );

+ 42 - 5
packages/ckeditor5-table/tests/tableselection.js

@@ -5,7 +5,7 @@
 
 
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
-import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+import { setData as setModelData, stringify as stringifyModel } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
 
 import TableEditing from '../src/tableediting';
 import TableEditing from '../src/tableediting';
 import TableSelection from '../src/tableselection';
 import TableSelection from '../src/tableselection';
@@ -81,7 +81,7 @@ describe( 'table selection', () => {
 				lastCell
 				lastCell
 			);
 			);
 
 
-			expect( Array.from( tableSelection.getSelectedTableCells() ) ).to.deep.equal( [
+			expect( tableSelection.getSelectedTableCells() ).to.deep.equal( [
 				firstCell, lastCell
 				firstCell, lastCell
 			] );
 			] );
 		} );
 		} );
@@ -95,7 +95,7 @@ describe( 'table selection', () => {
 				lastCell
 				lastCell
 			);
 			);
 
 
-			expect( Array.from( tableSelection.getSelectedTableCells() ) ).to.deep.equal( [
+			expect( tableSelection.getSelectedTableCells() ).to.deep.equal( [
 				firstCell, modelRoot.getNodeByPath( [ 0, 0, 1 ] ), modelRoot.getNodeByPath( [ 0, 1, 0 ] ), lastCell
 				firstCell, modelRoot.getNodeByPath( [ 0, 0, 1 ] ), modelRoot.getNodeByPath( [ 0, 1, 0 ] ), lastCell
 			] );
 			] );
 		} );
 		} );
@@ -109,7 +109,7 @@ describe( 'table selection', () => {
 				lastCell
 				lastCell
 			);
 			);
 
 
-			expect( Array.from( tableSelection.getSelectedTableCells() ) ).to.deep.equal( [
+			expect( tableSelection.getSelectedTableCells() ).to.deep.equal( [
 				firstCell, modelRoot.getNodeByPath( [ 0, 0, 1 ] ), lastCell
 				firstCell, modelRoot.getNodeByPath( [ 0, 0, 1 ] ), lastCell
 			] );
 			] );
 		} );
 		} );
@@ -120,10 +120,21 @@ describe( 'table selection', () => {
 
 
 			tableSelection._setCellSelection( firstCell, lastCell );
 			tableSelection._setCellSelection( firstCell, lastCell );
 
 
-			expect( Array.from( tableSelection.getSelectedTableCells() ) ).to.deep.equal( [
+			expect( tableSelection.getSelectedTableCells() ).to.deep.equal( [
 				firstCell, modelRoot.getNodeByPath( [ 0, 1, 1 ] ), lastCell
 				firstCell, modelRoot.getNodeByPath( [ 0, 1, 1 ] ), lastCell
 			] );
 			] );
 		} );
 		} );
+
+		it( 'should return cells in source order despite backward selection', () => {
+			const firstCell = modelRoot.getNodeByPath( [ 0, 0, 2 ] );
+			const lastCell = modelRoot.getNodeByPath( [ 0, 0, 1 ] );
+
+			tableSelection._setCellSelection( firstCell, lastCell );
+
+			expect( tableSelection.getSelectedTableCells() ).to.deep.equal( [
+				lastCell, firstCell
+			] );
+		} );
 	} );
 	} );
 
 
 	describe( 'getSelectionAsFragment()', () => {
 	describe( 'getSelectionAsFragment()', () => {
@@ -139,5 +150,31 @@ describe( 'table selection', () => {
 
 
 			expect( tableSelection.getSelectionAsFragment() ).to.be.instanceOf( DocumentFragment );
 			expect( tableSelection.getSelectionAsFragment() ).to.be.instanceOf( DocumentFragment );
 		} );
 		} );
+
+		it( 'should return cells in the source order in case of forward selection', () => {
+			tableSelection._setCellSelection(
+				modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+				modelRoot.getNodeByPath( [ 0, 1, 1 ] )
+			);
+
+			expect( stringifyModel( tableSelection.getSelectionAsFragment() ) ).to.equal( modelTable( [
+				[ '11', '12' ],
+				[ '21', '22' ]
+			] ) );
+		} );
+
+		it( 'should return cells in the source order in case of backward selection', () => {
+			tableSelection._setCellSelection(
+				modelRoot.getNodeByPath( [ 0, 1, 1 ] ),
+				modelRoot.getNodeByPath( [ 0, 0, 0 ] )
+			);
+
+			expect( editor.model.document.selection.isBackward ).to.be.true;
+
+			expect( stringifyModel( tableSelection.getSelectionAsFragment() ) ).to.equal( modelTable( [
+				[ '11', '12' ],
+				[ '21', '22' ]
+			] ) );
+		} );
 	} );
 	} );
 } );
 } );

+ 68 - 4
packages/ckeditor5-table/tests/utils.js

@@ -94,7 +94,7 @@ describe( 'table utils', () => {
 
 
 			tableSelection._setCellSelection( firstCell, lastCell );
 			tableSelection._setCellSelection( firstCell, lastCell );
 
 
-			expect( Array.from( getSelectedTableCells( selection ) ) ).to.have.ordered.members( [
+			expect( getSelectedTableCells( selection ) ).to.have.ordered.members( [
 				firstCell, lastCell
 				firstCell, lastCell
 			] );
 			] );
 		} );
 		} );
@@ -105,7 +105,7 @@ describe( 'table utils', () => {
 
 
 			tableSelection._setCellSelection( firstCell, lastCell );
 			tableSelection._setCellSelection( firstCell, lastCell );
 
 
-			expect( Array.from( getSelectedTableCells( selection ) ) ).to.have.ordered.members( [
+			expect( getSelectedTableCells( selection ) ).to.have.ordered.members( [
 				firstCell,
 				firstCell,
 				modelRoot.getNodeByPath( [ 0, 0, 1 ] ),
 				modelRoot.getNodeByPath( [ 0, 0, 1 ] ),
 				modelRoot.getNodeByPath( [ 0, 1, 0 ] ),
 				modelRoot.getNodeByPath( [ 0, 1, 0 ] ),
@@ -119,7 +119,7 @@ describe( 'table utils', () => {
 
 
 			tableSelection._setCellSelection( firstCell, lastCell );
 			tableSelection._setCellSelection( firstCell, lastCell );
 
 
-			expect( Array.from( getSelectedTableCells( selection ) ) ).to.have.ordered.members( [
+			expect( getSelectedTableCells( selection ) ).to.have.ordered.members( [
 				firstCell,
 				firstCell,
 				modelRoot.getNodeByPath( [ 0, 0, 1 ] ),
 				modelRoot.getNodeByPath( [ 0, 0, 1 ] ),
 				lastCell
 				lastCell
@@ -132,12 +132,76 @@ describe( 'table utils', () => {
 
 
 			tableSelection._setCellSelection( firstCell, lastCell );
 			tableSelection._setCellSelection( firstCell, lastCell );
 
 
-			expect( Array.from( getSelectedTableCells( selection ) ) ).to.have.ordered.members( [
+			expect( getSelectedTableCells( selection ) ).to.have.ordered.members( [
 				firstCell,
 				firstCell,
 				modelRoot.getNodeByPath( [ 0, 1, 1 ] ),
 				modelRoot.getNodeByPath( [ 0, 1, 1 ] ),
 				lastCell
 				lastCell
 			] );
 			] );
 		} );
 		} );
+
+		it( 'should return cells in source order despite backward selection and forward ranges', () => {
+			const leftCell = modelRoot.getNodeByPath( [ 0, 0, 1 ] );
+			const rightCell = modelRoot.getNodeByPath( [ 0, 0, 2 ] );
+
+			editor.model.change( writer => {
+				writer.setSelection(
+					[ writer.createRangeOn( leftCell ), writer.createRangeOn( rightCell ) ],
+					{ backward: true }
+				);
+			} );
+
+			expect( Array.from( tableSelection.getSelectedTableCells() ) ).to.deep.equal( [
+				leftCell, rightCell
+			] );
+		} );
+
+		it( 'should return cells in source order despite backward selection and backward ranges', () => {
+			const leftCell = modelRoot.getNodeByPath( [ 0, 0, 1 ] );
+			const rightCell = modelRoot.getNodeByPath( [ 0, 0, 2 ] );
+
+			editor.model.change( writer => {
+				writer.setSelection(
+					[ writer.createRangeOn( rightCell ), writer.createRangeOn( leftCell ) ],
+					{ backward: true }
+				);
+			} );
+
+			expect( Array.from( tableSelection.getSelectedTableCells() ) ).to.deep.equal( [
+				leftCell, rightCell
+			] );
+		} );
+
+		// Backward direction does not have to equal ranges in the reversed order.
+		it( 'should return cells in source order despite forward selection and backward ranges', () => {
+			const leftCell = modelRoot.getNodeByPath( [ 0, 0, 1 ] );
+			const rightCell = modelRoot.getNodeByPath( [ 0, 0, 2 ] );
+
+			editor.model.change( writer => {
+				writer.setSelection( [ writer.createRangeOn( rightCell ), writer.createRangeOn( leftCell ) ] );
+			} );
+
+			expect( Array.from( tableSelection.getSelectedTableCells() ) ).to.deep.equal( [
+				leftCell, rightCell
+			] );
+		} );
+
+		it( 'should return cells in source order despite selection with mixed range order', () => {
+			const leftCell = modelRoot.getNodeByPath( [ 0, 0, 0 ] );
+			const midCell = modelRoot.getNodeByPath( [ 0, 0, 1 ] );
+			const rightCell = modelRoot.getNodeByPath( [ 0, 0, 2 ] );
+
+			editor.model.change( writer => {
+				writer.setSelection( [
+					writer.createRangeOn( rightCell ),
+					writer.createRangeOn( leftCell ),
+					writer.createRangeOn( midCell )
+				] );
+			} );
+
+			expect( Array.from( tableSelection.getSelectedTableCells() ) ).to.deep.equal( [
+				leftCell, midCell, rightCell
+			] );
+		} );
 	} );
 	} );
 
 
 	describe( 'getTableCellsContainingSelection()', () => {
 	describe( 'getTableCellsContainingSelection()', () => {