ソースを参照

Merge branch 'master' into t/ckeditor5/1151

Aleksander Nowodzinski 6 年 前
コミット
0779524d3d

+ 5 - 13
packages/ckeditor5-table/src/converters/table-cell-content-post-fixer.js

@@ -4,16 +4,13 @@
  */
 
 /**
- * @module table/converters/table-cell-content-post-fixer
+ * @module table/converters/table-cell-paragraph-post-fixer
  */
 
 /**
- * Injects a table cell post-fixer into the model.
+ * Injects a table cell post-fixer into the model which inserts `paragraph` element into empty table cells.
  *
- * The role of the table post-fixer is to ensure that the table cells have the correct content
- * after a {@link module:engine/model/model~Model#change `change()`} block was executed.
- *
- * A table cells must contains at least one block as a child. The empty table cell will have empty `<paragraph>` as a child.
+ * A table cell must contain at least one block element as a child. An empty table cell will have empty `paragraph` as a child.
  *
  *		<table>
  *			<tableRow>
@@ -31,7 +28,7 @@
  *
  * @param {module:engine/model/model~Model} model
  */
-export default function injectTableCellContentPostFixer( model ) {
+export default function injectTableCellParagraphPostFixer( model ) {
 	model.document.registerPostFixer( writer => tableCellContentsPostFixer( writer, model ) );
 }
 
@@ -45,11 +42,6 @@ function tableCellContentsPostFixer( writer, model ) {
 	let wasFixed = false;
 
 	for ( const entry of changes ) {
-		// Enforce paragraph in tableCell even after other feature remove its contents.
-		if ( entry.type == 'remove' && entry.position.parent.is( 'tableCell' ) ) {
-			wasFixed = fixTableCellContent( entry.position.parent, writer ) || wasFixed;
-		}
-
 		// Analyze table cells on insertion.
 		if ( entry.type == 'insert' ) {
 			if ( entry.name == 'table' ) {
@@ -112,7 +104,7 @@ function fixTableCellContent( tableCell, writer ) {
 		return true;
 	}
 
-	// Check table cell children for directly placed $text nodes.
+	// Check table cell children for directly placed text nodes.
 	// Temporary solution. See https://github.com/ckeditor/ckeditor5/issues/1464.
 	const textNodes = Array.from( tableCell.getChildren() ).filter( child => child.is( 'text' ) );
 

+ 41 - 0
packages/ckeditor5-table/src/converters/table-cell-refresh-post-fixer.js

@@ -0,0 +1,41 @@
+/**
+ * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+/**
+ * @module table/converters/table-cell-refresh-post-fixer
+ */
+
+/**
+ * Injects a table cell post-fixer into the model which marks the table cell in the differ to have it re-rendered.
+ *
+ * Model `paragraph` inside a table cell can be rendered as `<span>` or `<p>`. It is rendered as `<span>` if this is the only block
+ * element in that table cell and it doesn't have any attributes. It is rendered as `<p>` otherwise.
+ *
+ * When table cell content changes, for example a second `paragraph` element is added we need to ensure that the first `paragraph` is
+ * re-rendered so it changes to `<p>` from `<span>`. The easiest way to do it is to re-render whole table cell.
+ *
+ * @param {module:engine/model/model~Model} model
+ */
+export default function injectTableCellRefreshPostFixer( model ) {
+	model.document.registerPostFixer( () => tableCellRefreshPostFixer( model ) );
+}
+
+function tableCellRefreshPostFixer( model ) {
+	const differ = model.document.differ;
+
+	let fixed = false;
+
+	for ( const change of differ.getChanges() ) {
+		const parent = change.type == 'insert' || change.type == 'remove' ? change.position.parent : change.range.start.parent;
+
+		if ( parent.is( 'tableCell' ) ) {
+			differ.refreshItem( parent );
+
+			fixed = true;
+		}
+	}
+
+	return fixed;
+}

+ 0 - 198
packages/ckeditor5-table/src/converters/tablecell-post-fixer.js

@@ -1,198 +0,0 @@
-/**
- * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
- * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
- */
-
-/**
- * @module table/converters/tablecell-post-fixer
- */
-
-/**
- * Injects a table cell post-fixer into the editing controller.
- *
- * The role of the table cell post-fixer is to ensure that the table cell contents in the editing view are properly converted.
- *
- * This post-fixer will ensure that after model changes in the editing view:
- * * single paragraphs are rendered as `<span>
- * * single paragraphs with one or more attributes are rendered as `<p>`
- * * single paragraphs in table cell with other block elements are rendered as `<p>`
- * * paragraphs in table cells with other block elements (including other paragraphs) are rendered as `<p>`.
- *
- * In the model each table cell has always at least one block element inside. If no other block was defined (empty table cell) the table
- * feature will insert empty `<paragraph>`. Similarly text nodes will be wrapped in paragraphs. Rendering in the data pipeline differs
- * from rendering in the editing pipeline - text nodes in single `<paragraph>` are rendered in the data pipeline as direct children
- * of the `<td>` or `<th>` elements. In other cases `<paragraph>` elements are rendered as `<p>` blocks.
- *
- * To ensure proper mappings between model and view elements and positions in the editing pipeline the table feature will always render
- * an element in the view: `<span>` for single or empty `<paragraph>` and `<p>` otherwise.
- *
- * Example:
- *
- *		<table>
- *			<tableRow>
- *				<tableCell><paragraph></paragraph></tableCell>
- *				<tableCell><paragraph>foo</paragraph></tableCell>
- *				<tableCell><paragraph baz="bar">foo</paragraph></tableCell>
- *				<tableCell><heading2>bar</heading2><paragraph>baz</paragraph></tableCell>
- *			</tableRow>
- *		</table>
- *
- * The editor will render in the data pipeline:
- *
- *		<figure>
- *			<table>
- *				<tbody>
- *					<tr>
- *						<td></td>
- *						<td>foo</td>
- *						<td><p baz="bar">foo</p></td>
- *						<td><h3>bar</h3><p>baz</p></td>
- *					</tr>
- *				</tbody>
- *			</table>
- *		</figure>
- *
- * and in the editing view (without widget markup):
- *
- *		<figure>
- *			<table>
- *				<tbody>
- *					<tr>
- *						<td><span></span></td>
- *						<td><span>foo</span></td>
- *						<td><p baz="bar">foo</p></td>
- *						<td><h3>bar</h3><p>baz</p></td>
- *					</tr>
- *				</tbody>
- *			</table>
- *		</figure>
- *
- * @param {module:engine/model/model~Model} model
- * @param {module:engine/controller/editingcontroller~EditingController} editing
- */
-export default function injectTableCellPostFixer( model, editing ) {
-	editing.view.document.registerPostFixer( writer => tableCellPostFixer( writer, model, editing.mapper, editing.view ) );
-}
-
-// The table cell post-fixer.
-//
-// @param {module:engine/view/writer~Writer} writer
-// @param {module:engine/model/model~Model} model
-// @param {module:engine/conversion/mapper~Mapper} mapper
-function tableCellPostFixer( writer, model, mapper, view ) {
-	let wasFixed = false;
-
-	const elementsToCheck = getElementsToCheck( view );
-
-	for ( const element of elementsToCheck ) {
-		wasFixed = ensureProperElementName( element, mapper, writer ) || wasFixed;
-	}
-
-	// Selection in the view might not be updated to renamed elements. Happens mostly when other feature inserts paragraph to the table cell
-	// (ie. when deleting table cell contents) and sets selection to it while table-post fixer changes view <p> to <span> element.
-	// The view.selection would have outdated nodes.
-	if ( wasFixed && selectionNeedsFix( view.document.selection, mapper ) ) {
-		updateRangesInViewSelection( model.document.selection, mapper, writer );
-	}
-
-	return wasFixed;
-}
-
-// Returns view elements changed in current view.change() block.
-//
-// **Note**: Currently it uses private property of the view: _renderer to get changed view elements to check.
-//
-// @param {module:engine/view/view~View} view
-function getElementsToCheck( view ) {
-	const elementsWithChangedAttributes = Array.from( view._renderer.markedAttributes )
-		.filter( el => !!el.parent )
-		.filter( isSpanOrP )
-		.filter( el => isTdOrTh( el.parent ) );
-
-	const changedChildren = Array.from( view._renderer.markedChildren )
-		.filter( el => !!el.parent )
-		.filter( isTdOrTh )
-		.reduce( ( prev, element ) => {
-			const childrenToCheck = Array.from( element.getChildren() ).filter( isSpanOrP );
-
-			return [ ...prev, ...childrenToCheck ];
-		}, [] );
-
-	return [ ...elementsWithChangedAttributes, ...changedChildren ];
-}
-
-// This method checks if view element for model's <paragraph> was properly converter.
-// Paragraph should be either
-// - span: for single paragraph with no attributes.
-// - p   : in other cases.
-function ensureProperElementName( currentViewElement, mapper, writer ) {
-	// This situation may happen if a view element was changed and removed at the same time.
-	// In this case, the view element is already unbound so the post-fixer would crash.
-	if ( !currentViewElement.root.is( 'rootElement' ) ) {
-		return false;
-	}
-
-	const modelParagraph = mapper.toModelElement( currentViewElement );
-	const expectedViewElementName = getExpectedElementName( modelParagraph.parent, modelParagraph );
-
-	if ( currentViewElement.name !== expectedViewElementName ) {
-		// Unbind current view element as it should be cleared from mapper.
-		mapper.unbindViewElement( currentViewElement );
-
-		const renamedViewElement = writer.rename( expectedViewElementName, currentViewElement );
-
-		// Bind paragraph inside table cell to the renamed view element.
-		mapper.bindElements( modelParagraph, renamedViewElement );
-
-		return true;
-	}
-
-	return false;
-}
-
-// Expected view element name depends on model elements:
-// - <paragraph> with any attribute set should be rendered as <p>
-// - all <paragraphs> in <tableCell> that has more then one children should be rendered as <p>
-// - an only <paragraph> child with no attributes should be rendered as <span>
-//
-// @param {module:engine/model/element~Element} tableCell
-// @param {module:engine/model/element~Element} paragraph
-// @returns {String}
-function getExpectedElementName( tableCell, paragraph ) {
-	const isOnlyChild = tableCell.childCount > 1;
-	const hasAttributes = !![ ...paragraph.getAttributes() ].length;
-
-	return ( isOnlyChild || hasAttributes ) ? 'p' : 'span';
-}
-
-// Method to filter out <span> and <p> elements.
-//
-// @param {module:engine/view/element~Element} element
-function isSpanOrP( element ) {
-	return element.is( 'p' ) || element.is( 'span' );
-}
-
-// Method to filter out <td> and <th> elements.
-//
-// @param {module:engine/view/element~Element} element
-function isTdOrTh( element ) {
-	return element.is( 'td' ) || element.is( 'th' );
-}
-
-// Resets view selections based on model selection.
-function updateRangesInViewSelection( selection, mapper, writer ) {
-	const fixedRanges = Array.from( selection.getRanges() )
-		.map( range => mapper.toViewRange( range ) );
-
-	writer.setSelection( fixedRanges, { backward: selection.isBackward } );
-}
-
-// Checks if selection needs to be fixed by ensuring that current view selection position's parents are present in the editable view.
-//
-// @param {module:engine/view/selection~Selection} viewSelection
-function selectionNeedsFix( viewSelection ) {
-	const anchor = viewSelection.anchor;
-	const focus = viewSelection.focus;
-
-	return viewSelection.rangeCount && ( !anchor.root.is( 'rootElement' ) || !focus.root.is( 'rootElement' ) );
-}

+ 4 - 5
packages/ckeditor5-table/src/tableediting.js

@@ -32,8 +32,8 @@ import { findAncestor } from './commands/utils';
 import TableUtils from '../src/tableutils';
 
 import injectTableLayoutPostFixer from './converters/table-layout-post-fixer';
-import injectTableCellContentsPostFixer from './converters/table-cell-content-post-fixer';
-import injectTableCellPostFixer from './converters/tablecell-post-fixer';
+import injectTableCellParagraphPostFixer from './converters/table-cell-paragraph-post-fixer';
+import injectTableCellRefreshPostFixer from './converters/table-cell-refresh-post-fixer';
 
 import '../theme/tableediting.css';
 
@@ -111,8 +111,6 @@ export default class TableEditing extends Plugin {
 		conversion.for( 'editingDowncast' ).add( downcastTableHeadingRowsChange( { asWidget: true } ) );
 		conversion.for( 'dataDowncast' ).add( downcastTableHeadingRowsChange() );
 
-		injectTableCellPostFixer( editor.model, editor.editing );
-
 		// Define all the commands.
 		editor.commands.add( 'insertTable', new InsertTableCommand( editor ) );
 		editor.commands.add( 'insertTableRowAbove', new InsertRowCommand( editor, { order: 'above' } ) );
@@ -135,7 +133,8 @@ export default class TableEditing extends Plugin {
 		editor.commands.add( 'setTableRowHeader', new SetHeaderRowCommand( editor ) );
 
 		injectTableLayoutPostFixer( model );
-		injectTableCellContentsPostFixer( model );
+		injectTableCellRefreshPostFixer( model );
+		injectTableCellParagraphPostFixer( model );
 
 		// Handle tab key navigation.
 		this.editor.keystrokes.set( 'Tab', ( ...args ) => this._handleTabOnSelectedTable( ...args ), { priority: 'low' } );

+ 1 - 1
packages/ckeditor5-table/tests/converters/table-cell-content-post-fixer.js

@@ -11,7 +11,7 @@ import TableEditing from '../../src/tableediting';
 import { formatTable } from './../_utils/utils';
 import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting';
 
-describe( 'Table cell content post-fixer', () => {
+describe( 'Table cell paragraph post-fixer', () => {
 	let editor, model, root;
 
 	beforeEach( () => {

+ 3 - 30
packages/ckeditor5-table/tests/converters/tablecell-post-fixer.js

@@ -8,7 +8,7 @@
 import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
 
 import { defaultConversion, defaultSchema, formatTable, formattedViewTable, viewTable } from '../_utils/utils';
-import injectTableCellPostFixer from '../../src/converters/tablecell-post-fixer';
+import injectTableCellRefreshPostFixer from '../../src/converters/table-cell-refresh-post-fixer';
 
 import env from '@ckeditor/ckeditor5-utils/src/env';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
@@ -16,7 +16,7 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictest
 
 import Delete from '@ckeditor/ckeditor5-typing/src/delete';
 
-describe( 'TableCell post-fixer', () => {
+describe( 'Table cell refresh post-fixer', () => {
 	let editor, model, doc, root, view;
 
 	testUtils.createSinonSandbox();
@@ -47,7 +47,7 @@ describe( 'TableCell post-fixer', () => {
 				model.schema.extend( '$block', { allowAttributes: 'foo' } );
 				editor.conversion.attributeToAttribute( { model: 'foo', view: 'foo' } );
 
-				injectTableCellPostFixer( model, editor.editing );
+				injectTableCellRefreshPostFixer( model );
 			} );
 	} );
 
@@ -215,33 +215,6 @@ describe( 'TableCell post-fixer', () => {
 		], { asWidget: true } ) );
 	} );
 
-	it( 'should not crash when view.change() block was called in model.change()', () => {
-		editor.setData( viewTable( [ [ '<p>foobar</p>' ] ] ) );
-
-		const table = root.getChild( 0 );
-
-		expect( formatTable( getViewData( view, { withoutSelection: true } ) ) )
-			.to.equal( formattedViewTable( [ [ 'foobar' ] ], { asWidget: true } ) );
-
-		expect( () => {
-			model.change( writer => {
-				const tableCell = table.getNodeByPath( [ 0, 0 ] );
-
-				writer.insertElement( 'paragraph', null, writer.createPositionAt( tableCell, 'end' ) );
-				writer.setSelection( writer.createRangeIn( tableCell ) );
-
-				// Do some change in the view while inside model change.
-				editor.editing.view.change( writer => {
-					writer.addClass( 'foo', editor.editing.mapper.toViewElement( tableCell ) );
-				} );
-			} );
-		} ).to.not.throw();
-
-		expect( formatTable( getViewData( view ) ) ).to.equal( formattedViewTable( [
-			[ { class: 'foo', contents: '<p>{foobar</p><p>]</p>' } ]
-		], { asWidget: true } ) );
-	} );
-
 	it( 'should keep <p> in the view when <paragraph> attribute value is changed (table cell with multiple blocks)', () => {
 		editor.setData( viewTable( [ [ '<p>00</p><p>00</p>' ] ] ) );