8
0
Просмотр исходного кода

Changed: Table post-fixer should not crash on table remove.

Maciej Gołaszewski 7 лет назад
Родитель
Сommit
83c4fe1a68

+ 30 - 27
packages/ckeditor5-table/src/tableediting.js

@@ -258,7 +258,7 @@ function tablePostFixer( writer, model, tableUtils ) {
 	for ( const entry of changes ) {
 		let table;
 
-		if ( entry.name == 'table' ) {
+		if ( entry.name == 'table' && entry.type == 'insert' ) {
 			table = entry.position.nodeAfter;
 		}
 
@@ -281,44 +281,44 @@ function tablePostFixer( writer, model, tableUtils ) {
 	return wasFixed;
 }
 
-function getRowsLengths( table ) {
-	const lengths = {};
-
+function getCellsToTrim( table ) {
 	const headingRows = parseInt( table.getAttribute( 'headingRows' ) || 0 );
 
 	const cellsToTrim = [];
 
-	for ( const data of new TableWalker( table ) ) {
-		const row = data.row;
-		const column = data.column;
-		const colspan = data.colspan;
-		const rowspan = data.rowspan;
-
-		if ( !lengths[ row ] ) {
-			// Le first row - the first column will be current width including rowspanned cells.
-			lengths[ row ] = column;
-		}
-
-		lengths[ row ] += colspan;
-
+	for ( const { row, rowspan, cell } of new TableWalker( table ) ) {
 		const maxRows = table.childCount;
 
 		if ( headingRows > row ) {
 			if ( row + rowspan > headingRows ) {
 				const newRowspan = headingRows - row;
 
-				cellsToTrim.push( { cell: data.cell, rowspan: newRowspan } );
+				cellsToTrim.push( { cell, rowspan: newRowspan } );
 			}
 		} else {
 			if ( row + rowspan + headingRows > maxRows ) {
 				const newRowspan = maxRows - row - headingRows + 1;
 
-				cellsToTrim.push( { cell: data.cell, rowspan: newRowspan } );
+				cellsToTrim.push( { cell, rowspan: newRowspan } );
 			}
 		}
 	}
 
-	return { lengths, cellsToTrim };
+	return cellsToTrim;
+}
+
+function getRowsLengths( table ) {
+	const lengths = {};
+
+	for ( const { row } of new TableWalker( table, { includeSpanned: true } ) ) {
+		if ( !lengths[ row ] ) {
+			lengths[ row ] = 0;
+		}
+
+		lengths[ row ] += 1;
+	}
+
+	return lengths;
 }
 
 function makeTableRowsSameLength( tableUtils, table, writer ) {
@@ -326,7 +326,16 @@ function makeTableRowsSameLength( tableUtils, table, writer ) {
 
 	const tableSize = tableUtils.getColumns( table );
 
-	const { lengths, cellsToTrim } = getRowsLengths( table );
+	// First: trim rowspanned table cells on section boundaries
+	const cellsToTrim = getCellsToTrim( table );
+
+	if ( cellsToTrim.length ) {
+		for ( const data of cellsToTrim ) {
+			updateNumericAttribute( 'rowspan', data.rowspan, data.cell, writer, 1 );
+		}
+	}
+
+	const lengths = getRowsLengths( table );
 
 	const isValid = Object.values( lengths ).every( length => length === tableSize );
 
@@ -346,11 +355,5 @@ function makeTableRowsSameLength( tableUtils, table, writer ) {
 		}
 	}
 
-	if ( cellsToTrim.length ) {
-		for ( const data of cellsToTrim ) {
-			updateNumericAttribute( 'rowspan', data.rowspan, data.cell, writer, 1 );
-		}
-	}
-
 	return wasFixed;
 }

+ 34 - 1
packages/ckeditor5-table/tests/tableediting.js

@@ -387,7 +387,26 @@ describe( 'TableEditing', () => {
 			] ) );
 		} );
 
-		it( 'should add missing columns to a tableRows that are shorter then longest table row (complex)', () => {
+		it( 'should add missing columns to a tableRows that are shorter then longest table row (complex 1)', () => {
+			const parsed = parse( modelTable( [
+				[ '00', { rowspan: 2, contents: '10' } ],
+				[ '10', { colspan: 2, contents: '12' } ],
+				[ '20', '21' ]
+			] ), model.schema );
+
+			model.change( writer => {
+				writer.remove( Range.createIn( root ) );
+				writer.insert( parsed, root );
+			} );
+
+			expect( formatTable( getModelData( model, { withoutSelection: true } ) ) ).to.equal( formattedModelTable( [
+				[ '00', { rowspan: 2, contents: '10' }, '', '' ],
+				[ '10', { colspan: 2, contents: '12' } ],
+				[ '20', '21', '', '' ]
+			] ) );
+		} );
+
+		it( 'should add missing columns to a tableRows that are shorter then longest table row (complex 2)', () => {
 			const parsed = parse( modelTable( [
 				[ { colspan: 6, contents: '00' } ],
 				[ { rowspan: 2, contents: '10' }, '11', { colspan: 3, contents: '12' } ],
@@ -570,6 +589,20 @@ describe( 'TableEditing', () => {
 				] ) );
 		} );
 
+		it( 'should not crash on table remove', () => {
+			setModelData( model, modelTable( [
+				[ '11', '12' ]
+			] ) );
+
+			expect( () => {
+				model.change( writer => {
+					writer.remove( Range.createIn( root ) );
+				} );
+			} ).to.not.throw();
+
+			expect( getModelData( model, { withoutSelection: true } ) ).to.equal( '<paragraph></paragraph>' );
+		} );
+
 		// Case: remove same column (undo does nothing on one client - NOOP in batch).
 		// Case: remove same row (undo does nothing on one client - NOOP in batch).
 		// Case: Typing over user selecting - typing in marker...