Merge pull request #153 from ckeditor/t/126

Fix: Make table a block element in the schema for quoting. Closes #126.

Author: scofalik

src/converters/downcast.js

@@ -51,14 +51,17 @@ export function downcastInsertTable( options = {} ) {
 			headingColumns: table.getAttribute( 'headingColumns' ) || 0
 		};
 
+		// Cache for created table rows.
+		const viewRows = new Map();
+
 		for ( const tableWalkerValue of tableWalker ) {
 			const { row, cell } = tableWalkerValue;
 
 			const tableSection = getOrCreateTableSection( getSectionName( row, tableAttributes ), tableElement, conversionApi );
 			const tableRow = table.getChild( row );
 
-			// Check if row was converted
-			const trElement = getOrCreateTr( tableRow, row, tableSection, conversionApi );
+			const trElement = viewRows.get( row ) || createTr( tableRow, row, tableSection, conversionApi );
+			viewRows.set( row, trElement );
 
 			// Consume table cell - it will be always consumed as we convert whole table at once.
 			conversionApi.consumable.consume( cell, 'insert' );
@@ -104,9 +107,14 @@ export function downcastInsertRow( options = {} ) {
 			headingColumns: table.getAttribute( 'headingColumns' ) || 0
 		};
 
+		// Cache for created table rows.
+		const viewRows = new Map();
+
 		for ( const tableWalkerValue of tableWalker ) {
 			const tableSection = getOrCreateTableSection( getSectionName( row, tableAttributes ), tableElement, conversionApi );
-			const trElement = getOrCreateTr( tableRow, row, tableSection, conversionApi );
+
+			const trElement = viewRows.get( row ) || createTr( tableRow, row, tableSection, conversionApi );
+			viewRows.set( row, trElement );
 
 			// Consume table cell - it will be always consumed as we convert whole row at once.
 			conversionApi.consumable.consume( tableWalkerValue.cell, 'insert' );
@@ -404,29 +412,25 @@ function createViewTableCellElement( tableWalkerValue, tableAttributes, insertPo
 	}
 }
 
-// Creates or returns an existing `<tr>` element from the view.
+// Creates `<tr>` view element.
 //
 // @param {module:engine/view/element~Element} tableRow
 // @param {Number} rowIndex
 // @param {module:engine/view/element~Element} tableSection
 // @param {Object} conversionApi
 // @returns {module:engine/view/element~Element}
-function getOrCreateTr( tableRow, rowIndex, tableSection, conversionApi ) {
-	let trElement = conversionApi.mapper.toViewElement( tableRow );
+function createTr( tableRow, rowIndex, tableSection, conversionApi ) {
+	// Will always consume since we're converting <tableRow> element from a parent <table>.
+	conversionApi.consumable.consume( tableRow, 'insert' );
 
-	if ( !trElement ) {
-		// Will always consume since we're converting <tableRow> element from a parent <table>.
-		conversionApi.consumable.consume( tableRow, 'insert' );
+	const trElement = conversionApi.writer.createContainerElement( 'tr' );
+	conversionApi.mapper.bindElements( tableRow, trElement );
 
-		trElement = conversionApi.writer.createContainerElement( 'tr' );
-		conversionApi.mapper.bindElements( tableRow, trElement );
+	const headingRows = tableRow.parent.getAttribute( 'headingRows' ) || 0;
+	const offset = headingRows > 0 && rowIndex >= headingRows ? rowIndex - headingRows : rowIndex;
 
-		const headingRows = tableRow.parent.getAttribute( 'headingRows' ) || 0;
-		const offset = headingRows > 0 && rowIndex >= headingRows ? rowIndex - headingRows : rowIndex;
-
-		const position = conversionApi.writer.createPositionAt( tableSection, offset );
-		conversionApi.writer.insert( position, trElement );
-	}
+	const position = conversionApi.writer.createPositionAt( tableSection, offset );
+	conversionApi.writer.insert( position, trElement );
 
 	return trElement;
 }

src/tableediting.js

@@ -56,7 +56,8 @@ export default class TableEditing extends Plugin {
 			allowWhere: '$block',
 			allowAttributes: [ 'headingRows', 'headingColumns' ],
 			isLimit: true,
-			isObject: true
+			isObject: true,
+			isBlock: true
 		} );
 
 		schema.register( 'tableRow', {

tests/_utils/utils.js

@@ -161,7 +161,8 @@ export function defaultSchema( schema, registerParagraph = true ) {
 		allowWhere: '$block',
 		allowAttributes: [ 'headingRows', 'headingColumns' ],
 		isLimit: true,
-		isObject: true
+		isObject: true,
+		isBlock: true
 	} );
 
 	schema.register( 'tableRow', {

tests/converters/downcast.js

@@ -171,6 +171,46 @@ describe( 'downcast converters', () => {
 			) );
 		} );
 
+		it( 'should re-create table on reinsert', () => {
+			model.schema.register( 'wrapper', {
+				allowWhere: '$block',
+				allowContentOf: '$root'
+			} );
+			editor.conversion.elementToElement( { model: 'wrapper', view: 'div' } );
+
+			setModelData( model, modelTable( [ [ '[]' ] ] ) );
+
+			expect( formatTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formatTable(
+				'<figure class="table">' +
+					'<table>' +
+						'<tbody>' +
+							'<tr><td></td></tr>' +
+						'</tbody>' +
+					'</table>' +
+				'</figure>'
+			) );
+
+			model.change( writer => {
+				const table = model.document.getRoot().getChild( 0 );
+				const range = writer.createRange( writer.createPositionBefore( table ), writer.createPositionAfter( table ) );
+				const wrapper = writer.createElement( 'wrapper' );
+
+				writer.wrap( range, wrapper );
+			} );
+
+			expect( formatTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formatTable(
+				'<div>' +
+					'<figure class="table">' +
+						'<table>' +
+							'<tbody>' +
+								'<tr><td></td></tr>' +
+							'</tbody>' +
+						'</table>' +
+					'</figure>' +
+				'</div>'
+			) );
+		} );
+
 		describe( 'headingColumns attribute', () => {
 			it( 'should mark heading columns table cells', () => {
 				setModelData( model, modelTable( [

tests/tableediting.js

@@ -45,6 +45,7 @@ describe( 'TableEditing', () => {
 		// Table:
 		expect( model.schema.isRegistered( 'table' ) ).to.be.true;
 		expect( model.schema.isObject( 'table' ) ).to.be.true;
+		expect( model.schema.isBlock( 'table' ) ).to.be.true;
 		expect( model.schema.isLimit( 'table' ) ).to.be.true;
 
 		expect( model.schema.checkChild( [ '$root' ], 'table' ) ).to.be.true;