Browse Source

Merge pull request #1431 from ckeditor/t/ckeditor5-table/12

Feature: Introduced a selection post-fixer. Its role is to ensure that after all changes are applied the selection is placed in a correct position. Closes #1156. Closes #1176. Closes #1182. Closes ckeditor/ckeditor5-table#11. Closes ckeditor/ckeditor5-table#12. Closes ckeditor/ckeditor5-table#15. Closes ckeditor/ckeditor5#562. Closes ckeditor/ckeditor5#611.
Piotrek Koszuliński 7 years ago
parent
commit
2f8d012208

File diff suppressed because it is too large
+ 1 - 1
packages/ckeditor5-engine/CHANGELOG.md


+ 1 - 5
packages/ckeditor5-engine/src/controller/editingcontroller.js

@@ -13,11 +13,7 @@ import Mapper from '../conversion/mapper';
 import DowncastDispatcher from '../conversion/downcastdispatcher';
 import { insertText, remove } from '../conversion/downcast-converters';
 import { convertSelectionChange } from '../conversion/upcast-selection-converters';
-import {
-	convertRangeSelection,
-	convertCollapsedSelection,
-	clearAttributes
-} from '../conversion/downcast-selection-converters';
+import { clearAttributes, convertCollapsedSelection, convertRangeSelection } from '../conversion/downcast-selection-converters';
 
 import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';

+ 1 - 3
packages/ckeditor5-engine/src/dev-utils/model.js

@@ -13,7 +13,6 @@
 
 import RootElement from '../model/rootelement';
 import Model from '../model/model';
-import Batch from '../model/batch';
 import ModelRange from '../model/range';
 import ModelPosition from '../model/position';
 import ModelSelection from '../model/selection';
@@ -94,7 +93,6 @@ export function setData( model, data, options = {} ) {
 
 	let modelDocumentFragment, selection;
 	const modelRoot = model.document.getRoot( options.rootName || 'main' );
-	const batch = new Batch( options.batchType || 'transparent' );
 
 	// Parse data string to model.
 	const parsedResult = setData._parse( data, model.schema, {
@@ -111,7 +109,7 @@ export function setData( model, data, options = {} ) {
 		modelDocumentFragment = parsedResult;
 	}
 
-	model.enqueueChange( batch, writer => {
+	model.change( writer => {
 		// Replace existing model in document by new one.
 		writer.remove( ModelRange.createIn( modelRoot ) );
 		writer.insert( modelDocumentFragment, modelRoot );

+ 1 - 1
packages/ckeditor5-engine/src/model/document.js

@@ -155,7 +155,7 @@ export default class Document {
 		} );
 
 		// Wait for `_change` event from model, which signalizes that outermost change block has finished.
-		// When this happens, check if there were any changes done on document, and if so, call post fixers,
+		// When this happens, check if there were any changes done on document, and if so, call post-fixers,
 		// fire `change` event for features and conversion and then reset the differ.
 		// Fire `change:data` event when at least one operation or buffered marker changes the data.
 		this.listenTo( model, '_change', ( evt, writer ) => {

+ 3 - 0
packages/ckeditor5-engine/src/model/model.js

@@ -26,6 +26,7 @@ import insertContent from './utils/insertcontent';
 import deleteContent from './utils/deletecontent';
 import modifySelection from './utils/modifyselection';
 import getSelectedContent from './utils/getselectedcontent';
+import { injectSelectionPostFixer } from './utils/selection-post-fixer';
 
 /**
  * Editor's data model. Read about the model in the
@@ -111,6 +112,8 @@ export default class Model {
 		this.schema.register( '$marker', {
 			allowIn: [ '$root', '$block' ]
 		} );
+
+		injectSelectionPostFixer( this );
 	}
 
 	/**

+ 20 - 7
packages/ckeditor5-engine/src/model/schema.js

@@ -333,18 +333,24 @@ export default class Schema {
 
 	/**
 	 * Returns `true` if the given item is defined to be
-	 * a limit element by {@link module:engine/model/schema~SchemaItemDefinition}'s `isLimit` property.
+	 * a limit element by {@link module:engine/model/schema~SchemaItemDefinition}'s `isLimit` or `isObject` property
+	 * (all objects are also limits).
 	 *
 	 *		schema.isLimit( 'paragraph' ); // -> false
 	 *		schema.isLimit( '$root' ); // -> true
 	 *		schema.isLimit( editor.model.document.getRoot() ); // -> true
+	 *		schema.isLimit( 'image' ); // -> true
 	 *
 	 * @param {module:engine/model/item~Item|module:engine/model/schema~SchemaContextItem|String} item
 	 */
 	isLimit( item ) {
 		const def = this.getDefinition( item );
 
-		return !!( def && def.isLimit );
+		if ( !def ) {
+			return false;
+		}
+
+		return !!( def.isLimit || def.isObject );
 	}
 
 	/**
@@ -375,6 +381,11 @@ export default class Schema {
 	 *		} );
 	 *		schema.checkChild( model.document.getRoot(), paragraph ); // -> true
 	 *
+	 * Note: When verifying whether the given node can be a child of the given context,
+	 * schema also verifies the entire context – from its root to its last element. Therefore, it is possible
+	 * for `checkChild()` to return `false` even though context's last element can contain the checked child.
+	 * It happens if one of the context's elements does not allow its child.
+	 *
 	 * @fires checkChild
 	 * @param {module:engine/model/schema~SchemaContextDefinition} context Context in which the child will be checked.
 	 * @param {module:engine/model/node~Node|String} def The child to check.
@@ -742,8 +753,8 @@ export default class Schema {
 				return parent;
 			}
 
-			// Do not split limit elements and objects.
-			if ( this.isLimit( parent ) || this.isObject( parent ) ) {
+			// Do not split limit elements.
+			if ( this.isLimit( parent ) ) {
 				return null;
 			}
 
@@ -969,7 +980,7 @@ mix( Schema, ObservableMixin );
  * * `allowAttributesOf` – a string or an array of strings. Inherit attributes from other items.
  * * `inheritTypesFrom` – a string or an array of strings. Inherit `is*` properties of other items.
  * * `inheritAllFrom` – a string. A shorthand for `allowContentOf`, `allowWhere`, `allowAttributesOf`, `inheritTypesFrom`.
- * * additionall, you can define the following `is*` properties: `isBlock`, `isLimit`, `isObject`. Read about them below.
+ * * additionally, you can define the following `is*` properties: `isBlock`, `isLimit`, `isObject`. Read about them below.
  *
  * # The is* properties
  *
@@ -982,9 +993,11 @@ mix( Schema, ObservableMixin );
  * Most block type items will inherit from `$block` (through `inheritAllFrom`).
  * * `isLimit` – can be understood as whether this element should not be split by <kbd>Enter</kbd>.
  * Examples of limit elements – `$root`, table cell, image caption, etc. In other words, all actions which happen inside
- * a limit element are limitted to its content.
+ * a limit element are limited to its content. **Note:** all objects (`isObject`) are treated as limit elements too.
  * * `isObject` – whether item is "self-contained" and should be treated as a whole. Examples of object elements –
- * `image`, `table`, `video`, etc.
+ * `image`, `table`, `video`, etc. **Note:** an object is also a limit so
+ * {@link module:engine/model/schema~Schema#isLimit `isLimit()`}
+ * returns `true` for object elements automatically.
  *
  * # Generic items
  *

+ 1 - 1
packages/ckeditor5-engine/src/model/utils/deletecontent.js

@@ -185,7 +185,7 @@ function checkCanBeMerged( leftPos, rightPos, schema ) {
 	const rangeToCheck = new Range( leftPos, rightPos );
 
 	for ( const value of rangeToCheck.getWalker() ) {
-		if ( schema.isObject( value.item ) || schema.isLimit( value.item ) ) {
+		if ( schema.isLimit( value.item ) ) {
 			return false;
 		}
 	}

+ 1 - 1
packages/ckeditor5-engine/src/model/utils/insertcontent.js

@@ -145,7 +145,7 @@ class Insertion {
 			} );
 		}
 
-		// TMP this will become a postfixer.
+		// TMP this will become a post-fixer.
 		this.schema.removeDisallowedAttributes( this._filterAttributesOf, this.writer );
 		this._filterAttributesOf = [];
 	}

+ 235 - 0
packages/ckeditor5-engine/src/model/utils/selection-post-fixer.js

@@ -0,0 +1,235 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/**
+ * @module engine/mode/utils/selection-post-fixer
+ */
+
+import Range from '../range';
+import Position from '../position';
+
+/**
+ * Injects selection post-fixer to the model.
+ *
+ * The role of the selection post-fixer is to ensure that the selection is in a correct place
+ * after a {@link module:engine/model/model~Model#change `change()`} block was executed.
+ *
+ * The correct position means that:
+ *
+ * * All collapsed selection ranges are in a place where the {@link module:engine/model/schema~Schema}
+ * allows a `$text`.
+ * * None of selection's non-collapsed ranges crosses a {@link module:engine/model/schema~Schema#isLimit limit element}
+ * boundary (a range must be rooted within one limit element).
+ * * Only {@link module:engine/model/schema~Schema#isObject object elements} can be selected from outside
+ * (e.g. `[<paragraph>foo</paragraph>]` is invalid). This rule applies independently to both selection ends, so this
+ * selection is correct – `<paragraph>f[oo</paragraph><image></image>]`.
+ *
+ * If the position is not correct, the post-fixer will automatically correct it.
+ *
+ * ## Fixing a non-collapsed selection
+ *
+ * See as an example a selection that starts in a P1 element and ends inside a text of a TD element
+ * (`[` and `]` are range boundaries and `(l)` denotes element defines as `isLimit=true`):
+ *
+ *		root
+ *		 |- element P1
+ *		 |   |- "foo"                                      root
+ *		 |- element TABLE (l)                   P1         TABLE             P2
+ *		 |   |- element TR (l)                 f o[o     TR      TR         b a r
+ *		 |   |   |- element TD (l)                       TD      TD
+ *		 |   |       |- "aaa"                          a]a a    b b b
+ *		 |   |- element TR (l)
+ *		 |   |   |- element TD (l)                           ||
+ *		 |   |       |- "bbb"                                ||
+ *		 |- element P2                                       VV
+ *		 |   |- "bar"
+ *		                                                   root
+ *		                                        P1         TABLE]            P2
+ *		                                       f o[o     TR      TR         b a r
+ *		                                                 TD      TD
+ *		                                               a a a    b b b
+ *
+ * In the above example, the TABLE, TR and TD are defined as `isLimit=true` in the schema. The range which is not contained within
+ * a single limit element must be expanded to select the outer most limit element. The range end is inside text node of TD element.
+ * As TD element is a child of TR element and TABLE elements which both are defined as `isLimit=true` in schema the range must be expanded
+ * to select whole TABLE element.
+ *
+ * **Note** If selection contains multiple ranges the method returns minimal set of ranges that are not intersecting after expanding them
+ * to select `isLimit=true` elements.
+ *
+ * @param {module:engine/model/model~Model} model
+ */
+export function injectSelectionPostFixer( model ) {
+	model.document.registerPostFixer( writer => selectionPostFixer( writer, model ) );
+}
+
+// The selection post-fixer.
+//
+// @param {module:engine/model/writer~Writer} writer
+// @param {module:engine/model/model~Model} model
+function selectionPostFixer( writer, model ) {
+	const selection = model.document.selection;
+	const schema = model.schema;
+
+	const ranges = [];
+
+	let wasFixed = false;
+
+	for ( const modelRange of selection.getRanges() ) {
+		// Go through all ranges in selection and try fixing each of them.
+		// Those ranges might overlap but will be corrected later.
+		const correctedRange = tryFixingRange( modelRange, schema );
+
+		if ( correctedRange ) {
+			ranges.push( correctedRange );
+			wasFixed = true;
+		} else {
+			ranges.push( modelRange );
+		}
+	}
+
+	// If any of ranges were corrected update the selection.
+	if ( wasFixed ) {
+		// The above algorithm might create ranges that intersects each other when selection contains more then one range.
+		// This is case happens mostly on Firefox which creates multiple ranges for selected table.
+		const combinedRanges = combineOverlapingRanges( ranges );
+
+		writer.setSelection( combinedRanges, { backward: selection.isBackward } );
+	}
+}
+
+// Tries fixing a range if it's incorrect.
+//
+// @param {module:engine/model/range~Range} range
+// @param {module:engine/model/schema~Schema} schema
+// @returns {module:engine/model/range~Range|null} Returns fixed range or null if range is valid.
+function tryFixingRange( range, schema ) {
+	if ( range.isCollapsed ) {
+		return tryFixingCollapsedRange( range, schema );
+	}
+
+	return tryFixingNonCollpasedRage( range, schema );
+}
+
+// Tries to fix collapsed ranges.
+//
+// * Fixes situation when a range is in a place where $text is not allowed
+//
+// @param {module:engine/model/range~Range} range Collapsed range to fix.
+// @param {module:engine/model/schema~Schema} schema
+// @returns {module:engine/model/range~Range|null} Returns fixed range or null if range is valid.
+function tryFixingCollapsedRange( range, schema ) {
+	const originalPosition = range.start;
+
+	const nearestSelectionRange = schema.getNearestSelectionRange( originalPosition );
+
+	// This might be null ie when editor data is empty.
+	// In such cases there is no need to fix the selection range.
+	if ( !nearestSelectionRange ) {
+		return null;
+	}
+
+	const fixedPosition = nearestSelectionRange.start;
+
+	// Fixed position is the same as original - no need to return corrected range.
+	if ( originalPosition.isEqual( fixedPosition ) ) {
+		return null;
+	}
+
+	// Check single node selection (happens in tables).
+	if ( fixedPosition.nodeAfter && schema.isLimit( fixedPosition.nodeAfter ) ) {
+		return new Range( fixedPosition, Position.createAfter( fixedPosition.nodeAfter ) );
+	}
+
+	return new Range( fixedPosition );
+}
+
+// Tries to fix a expanded range that overlaps limit nodes.
+//
+// @param {module:engine/model/range~Range} range Expanded range to fix.
+// @param {module:engine/model/schema~Schema} schema
+// @returns {module:engine/model/range~Range|null} Returns fixed range or null if range is valid.
+function tryFixingNonCollpasedRage( range, schema ) {
+	// No need to check flat ranges as they will not cross node boundary.
+	if ( range.isFlat ) {
+		return null;
+	}
+
+	const start = range.start;
+	const end = range.end;
+
+	const updatedStart = expandSelectionOnIsLimitNode( start, schema, 'start' );
+	const updatedEnd = expandSelectionOnIsLimitNode( end, schema, 'end' );
+
+	if ( !start.isEqual( updatedStart ) || !end.isEqual( updatedEnd ) ) {
+		return new Range( updatedStart, updatedEnd );
+	}
+
+	return null;
+}
+
+// Expands selection so it contains whole limit node.
+//
+// @param {module:engine/model/position~Position} position
+// @param {module:engine/model/schema~Schema} schema
+// @param {String} expandToDirection Direction of expansion - either 'start' or 'end' of the range.
+// @returns {module:engine/model/position~Position}
+function expandSelectionOnIsLimitNode( position, schema, expandToDirection ) {
+	let node = position.parent;
+	let parent = node;
+
+	// Find outer most isLimit block as such blocks might be nested (ie. in tables).
+	while ( schema.isLimit( parent ) && parent.parent ) {
+		node = parent;
+		parent = parent.parent;
+	}
+
+	if ( node === parent ) {
+		// If there is not is limit block the return original position.
+		return position;
+	}
+
+	// Depending on direction of expanding selection return position before or after found node.
+	return expandToDirection === 'start' ? Position.createBefore( node ) : Position.createAfter( node );
+}
+
+// Returns minimal set of continuous ranges.
+//
+// @param {Array.<module:engine/model/range~Range>} ranges
+// @returns {Array.<module:engine/model/range~Range>}
+function combineOverlapingRanges( ranges ) {
+	const combinedRanges = [];
+
+	// Seed the state.
+	let previousRange = ranges[ 0 ];
+	combinedRanges.push( previousRange );
+
+	// Go through each ranges and check if it can be merged with previous one.
+	for ( const range of ranges ) {
+		// Do not push same ranges (ie might be created in a table).
+		if ( range.isEqual( previousRange ) ) {
+			continue;
+		}
+
+		// Merge intersecting range into previous one.
+		if ( range.isIntersecting( previousRange ) ) {
+			const newStart = previousRange.start.isBefore( range.start ) ? previousRange.start : range.start;
+			const newEnd = range.end.isAfter( previousRange.end ) ? range.end : previousRange.end;
+			const combinedRange = new Range( newStart, newEnd );
+
+			// Replace previous range with the combined one.
+			combinedRanges.splice( combinedRanges.indexOf( previousRange ), 1, combinedRange );
+
+			previousRange = combinedRange;
+
+			continue;
+		}
+
+		previousRange = range;
+		combinedRanges.push( range );
+	}
+
+	return combinedRanges;
+}

+ 1 - 1
packages/ckeditor5-engine/src/view/placeholder.js

@@ -29,7 +29,7 @@ export function attachPlaceholder( view, element, placeholderText, checkFunction
 	if ( !documentPlaceholders.has( document ) ) {
 		documentPlaceholders.set( document, new Map() );
 
-		// Create view post fixer that will add placeholder where needed.
+		// Create view post-fixer that will add placeholder where needed.
 		document.registerPostFixer( writer => updateAllPlaceholders( document, writer ) );
 	}
 

+ 8 - 7
packages/ckeditor5-engine/src/view/view.js

@@ -325,13 +325,14 @@ export default class View {
 			 * Thrown when there is an attempt to make changes to the view tree when it is in incorrect state. This may
 			 * cause some unexpected behaviour and inconsistency between the DOM and the view.
 			 * This may be caused by:
-			 *   * calling {@link #change} or {@link #render} during rendering process,
-			 *   * calling {@link #change} or {@link #render} inside of
-			 *   {@link module:engine/view/document~Document#registerPostFixer post fixer function}.
+			 *
+			 * * calling {@link #change} or {@link #render} during rendering process,
+			 * * calling {@link #change} or {@link #render} inside of
+			 *   {@link module:engine/view/document~Document#registerPostFixer post-fixer function}.
 			 */
 			throw new CKEditorError(
 				'cannot-change-view-tree: ' +
-				'Attempting to make changes to the view when it is in incorrect state: rendering or post fixers are in progress. ' +
+				'Attempting to make changes to the view when it is in incorrect state: rendering or post-fixers are in progress. ' +
 				'This may cause some unexpected behaviour and inconsistency between the DOM and the view.'
 			);
 		}
@@ -349,7 +350,7 @@ export default class View {
 		callback( this._writer );
 		this._ongoingChange = false;
 
-		// Execute all document post fixers after the change.
+		// Execute all document post-fixers after the change.
 		this._postFixersInProgress = true;
 		this.document._callPostFixers( this._writer );
 		this._postFixersInProgress = false;
@@ -395,7 +396,7 @@ export default class View {
 
 	/**
 	 * Fired after a topmost {@link module:engine/view/view~View#change change block} and all
-	 * {@link module:engine/view/document~Document#registerPostFixer post fixers} are executed.
+	 * {@link module:engine/view/document~Document#registerPostFixer post-fixers} are executed.
 	 *
 	 * Actual rendering is performed as a first listener on 'normal' priority.
 	 *
@@ -405,7 +406,7 @@ export default class View {
 	 *
 	 * This event is useful when you want to update interface elements after the rendering, e.g. position of the
 	 * balloon panel. If you wants to change view structure use
-	 * {@link module:engine/view/document~Document#registerPostFixer post fixers}.
+	 * {@link module:engine/view/document~Document#registerPostFixer post-fixers}.
 	 *
 	 * @event module:engine/view/view~View#event:render
 	 */

+ 1 - 1
packages/ckeditor5-engine/tests/controller/editingcontroller.js

@@ -21,7 +21,7 @@ import ModelPosition from '../../src/model/position';
 import ModelRange from '../../src/model/range';
 import ModelDocumentFragment from '../../src/model/documentfragment';
 
-import { parse, getData as getModelData } from '../../src/dev-utils/model';
+import { getData as getModelData, parse } from '../../src/dev-utils/model';
 import { getData as getViewData } from '../../src/dev-utils/view';
 
 describe( 'EditingController', () => {

+ 51 - 0
packages/ckeditor5-engine/tests/manual/selection.css

@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+table {
+	border-collapse: collapse;
+	border-spacing: 0;
+	border-color: hsl(0,0%,87%);
+}
+
+table, th, td {
+	padding: 5px;
+	border: 1px inset hsl(0,0%,87%);
+}
+
+table th,
+table td {
+	min-width: 1em;
+}
+
+table th {
+	background: hsl(0,0%,96%);
+	font-weight: bold;
+}
+
+table thead th {
+	background: hsl(0,0%,90%);
+}
+
+.ck table.ck-widget th.ck-editor__nested-editable {
+	border-color: inherit;
+	border-style: inset;
+}
+
+.ck table.ck-widget td.ck-editor__nested-editable {
+	border-color: inherit;
+	border-style: inset;
+}
+
+.ck table.ck-widget td.ck-editor__nested-editable.ck-editor__nested-editable_focused,
+.ck table.ck-widget td.ck-editor__nested-editable:focus {
+	background-color: inherit;
+	color: inherit;
+}
+
+.ck table.ck-widget th.ck-editor__nested-editable.ck-editor__nested-editable_focused,
+.ck table.ck-widget th.ck-editor__nested-editable:focus {
+	background-color: inherit;
+	color: inherit;
+}

+ 48 - 0
packages/ckeditor5-engine/tests/manual/selection.html

@@ -0,0 +1,48 @@
+<div id="editor">
+	<p>foo</p>
+
+	<table>
+		<tr>
+			<td>a</td>
+			<td>b</td>
+			<td>c</td>
+		</tr>
+		<tr>
+			<td>d</td>
+			<td>e</td>
+			<td>f</td>
+		</tr>
+	</table>
+
+	<p>bar</p>
+	
+	<table>
+		<tr>
+			<td>1</td>
+			<td>1</td>
+			<td>1</td>
+		</tr>
+		<tr>
+			<td>1</td>
+			<td>1</td>
+			<td>1</td>
+		</tr>
+	</table>
+
+	<table>
+		<tr>
+			<td>2</td>
+			<td>2</td>
+			<td>2</td>
+		</tr>
+		<tr>
+			<td>2</td>
+			<td>2</td>
+			<td>2</td>
+		</tr>
+	</table>
+
+	<p>baz</p>
+</div>
+<h2>Model contents:</h2>
+<div id="model"></div>

+ 102 - 0
packages/ckeditor5-engine/tests/manual/selection.js

@@ -0,0 +1,102 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* global console */
+
+import { downcastElementToElement } from '../../src/conversion/downcast-converters';
+
+import { getData } from '../../src/dev-utils/model';
+import global from '@ckeditor/ckeditor5-utils/src/dom/global';
+
+import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
+import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
+import Enter from '@ckeditor/ckeditor5-enter/src/enter';
+import Typing from '@ckeditor/ckeditor5-typing/src/typing';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+import Undo from '@ckeditor/ckeditor5-undo/src/undo';
+import { upcastElementToElement } from '../../src/conversion/upcast-converters';
+
+import './selection.css';
+import { toWidget, toWidgetEditable } from '@ckeditor/ckeditor5-widget/src/utils';
+import Widget from '@ckeditor/ckeditor5-widget/src/widget';
+
+class SelectionTest extends Plugin {
+	init() {
+		const editor = this.editor;
+		const schema = editor.model.schema;
+
+		schema.register( 'table', {
+			allowWhere: '$block',
+			isObject: true,
+			isLimit: true
+		} );
+
+		schema.register( 'tableRow', {
+			allowIn: 'table',
+			isLimit: true
+		} );
+
+		schema.register( 'tableCell', {
+			allowIn: 'tableRow',
+			allowContentOf: '$block',
+			isLimit: true
+		} );
+
+		editor.conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'table', view: 'table' } ) );
+		editor.conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
+		editor.conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
+
+		editor.conversion.for( 'downcast' ).add( downcastElementToElement( {
+			model: 'table',
+			view: ( modelItem, viewWriter ) => {
+				return toWidget( viewWriter.createContainerElement( 'table' ), viewWriter );
+			}
+		} ) );
+		editor.conversion.for( 'downcast' ).add( downcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
+
+		editor.conversion.for( 'downcast' ).add( downcastElementToElement( {
+			model: 'tableCell',
+			view: ( modelItem, viewWriter ) => {
+				return toWidgetEditable( viewWriter.createEditableElement( 'td' ), viewWriter );
+			}
+		} ) );
+	}
+}
+
+ClassicEditor
+	.create( global.document.querySelector( '#editor' ), {
+		plugins: [ Enter, Typing, Paragraph, SelectionTest, Undo, Widget ],
+		toolbar: [ 'undo', 'redo' ]
+	} )
+	.then( editor => {
+		editor.model.document.on( 'change', () => {
+			printModelContents( editor );
+		} );
+
+		printModelContents( editor );
+	} )
+	.catch( err => {
+		console.error( err.stack );
+	} );
+
+const modelDiv = global.document.querySelector( '#model' );
+
+function printModelContents( editor ) {
+	modelDiv.innerText = formatTable( getData( editor.model ) );
+}
+
+function formatTable( tableString ) {
+	return tableString
+		.replace( /<table>/g, '\n<table>' )
+		.replace( /<tableRow>/g, '\n<tableRow>\n    ' )
+		.replace( /<thead>/g, '\n<thead>\n    ' )
+		.replace( /<tbody>/g, '\n<tbody>\n    ' )
+		.replace( /<tr>/g, '\n<tr>\n    ' )
+		.replace( /<\/tableRow>/g, '\n</tableRow>' )
+		.replace( /<\/thead>/g, '\n</thead>' )
+		.replace( /<\/tbody>/g, '\n</tbody>' )
+		.replace( /<\/tr>/g, '\n</tr>' )
+		.replace( /<\/table>/g, '\n</table>' );
+}

+ 10 - 0
packages/ckeditor5-engine/tests/manual/selection.md

@@ -0,0 +1,10 @@
+### Selection of limit nodes test
+
+* Try selecting various parts of table:
+  * collapsed selection between a table with "1"s and "2"s
+  * selecting part of text "foo" and "bar"
+  * selecting part of any table (should select whole table).
+
+* In any case whole table should be selected (check model contents).
+
+* Only contents of a table cell should be selectable.

+ 28 - 6
packages/ckeditor5-engine/tests/model/schema.js

@@ -316,6 +316,14 @@ describe( 'Schema', () => {
 			expect( schema.isLimit( 'foo' ) ).to.be.true;
 		} );
 
+		it( 'returns true if an item was registered as an object element (because all objects are limits too)', () => {
+			schema.register( 'foo', {
+				isObject: true
+			} );
+
+			expect( schema.isLimit( 'foo' ) ).to.be.true;
+		} );
+
 		it( 'returns false if an item was not registered as a limit element', () => {
 			schema.register( 'foo' );
 
@@ -343,6 +351,14 @@ describe( 'Schema', () => {
 			expect( schema.isObject( 'foo' ) ).to.be.true;
 		} );
 
+		it( 'returns false if an item was registered as a limit (because not all limits are objects)', () => {
+			schema.register( 'foo', {
+				isLimit: true
+			} );
+
+			expect( schema.isObject( 'foo' ) ).to.be.false;
+		} );
+
 		it( 'returns false if an item was not registered as an object', () => {
 			schema.register( 'foo' );
 
@@ -886,11 +902,13 @@ describe( 'Schema', () => {
 			schema.extend( 'article', { isLimit: true } );
 			schema.extend( 'section', { isLimit: true } );
 
-			setData( model, '<div><section><article>[foo</article><article>bar]</article></section></div>' );
+			model.enqueueChange( 'transparent', () => {
+				setData( model, '<div><section><article>[foo</article><article>bar]</article></section></div>' );
 
-			const section = root.getNodeByPath( [ 0, 0 ] );
+				const section = root.getNodeByPath( [ 0, 0 ] );
 
-			expect( schema.getLimitElement( doc.selection ) ).to.equal( section );
+				expect( schema.getLimitElement( doc.selection ) ).to.equal( section );
+			} );
 		} );
 
 		it( 'works fine with multi-range selections', () => {
@@ -1389,8 +1407,12 @@ describe( 'Schema', () => {
 
 		function test( testName, data, direction, expected ) {
 			it( testName, () => {
-				setData( model, data );
-				const range = schema.getNearestSelectionRange( selection.anchor, direction );
+				let range;
+
+				model.enqueueChange( 'transparent', () => {
+					setData( model, data );
+					range = schema.getNearestSelectionRange( selection.anchor, direction );
+				} );
 
 				if ( expected === null ) {
 					expect( range ).to.be.null;
@@ -2598,7 +2620,7 @@ describe( 'Schema', () => {
 		} );
 
 		it( 'image is block object', () => {
-			expect( schema.isLimit( 'image' ) ).to.be.false;
+			expect( schema.isLimit( 'image' ) ).to.be.true;
 			expect( schema.isBlock( 'image' ) ).to.be.true;
 			expect( schema.isObject( 'image' ) ).to.be.true;
 		} );

+ 4 - 2
packages/ckeditor5-engine/tests/model/utils/deletecontent.js

@@ -900,9 +900,11 @@ describe( 'DataController utils', () => {
 
 		function test( title, input, output, options ) {
 			it( title, () => {
-				setData( model, input );
+				model.enqueueChange( 'transparent', () => {
+					setData( model, input );
 
-				deleteContent( model, doc.selection, options );
+					deleteContent( model, doc.selection, options );
+				} );
 
 				expect( getData( model ) ).to.equal( output );
 			} );

+ 5 - 3
packages/ckeditor5-engine/tests/model/utils/insertcontent.js

@@ -813,9 +813,11 @@ describe( 'DataController utils', () => {
 			expect( getData( model ) ).to.equal( '<limit>foo bar[]</limit>' );
 		} );
 
-		it( 'should insert text into limit element', () => {
-			setData( model, '<limit>foo[</limit><limit>]bar</limit>' );
-			insertHelper( 'baz' );
+		it( 'should insert text into limit element when selection spans over many limit elements', () => {
+			model.enqueueChange( 'transparent', () => {
+				setData( model, '<limit>foo[</limit><limit>]bar</limit>' );
+				insertHelper( 'baz' );
+			} );
 
 			expect( getData( model ) ).to.equal( '<limit>foobaz[]</limit><limit>bar</limit>' );
 		} );

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

@@ -0,0 +1,344 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import Model from '../../../src/model/model';
+import ModelPosition from '../../../src/model/position';
+import ModelRange from '../../../src/model/range';
+
+import { injectSelectionPostFixer } from '../../../src/model/utils/selection-post-fixer';
+
+import { getData as getModelData, setData as setModelData } from '../../../src/dev-utils/model';
+
+describe( 'Selection post-fixer', () => {
+	describe( 'injectSelectionPostFixer()', () => {
+		it( 'is a function', () => {
+			expect( injectSelectionPostFixer ).to.be.a( 'function' );
+		} );
+	} );
+
+	describe( 'injected behavior', () => {
+		let model, modelRoot;
+
+		beforeEach( () => {
+			model = new Model();
+			modelRoot = model.document.createRoot();
+
+			model.schema.register( 'paragraph', { inheritAllFrom: '$block' } );
+			model.schema.register( 'table', {
+				allowWhere: '$block',
+				isObject: true,
+				isLimit: true
+			} );
+
+			model.schema.register( 'tableRow', {
+				allowIn: 'table',
+				isLimit: true
+			} );
+
+			model.schema.register( 'tableCell', {
+				allowIn: 'tableRow',
+				allowContentOf: '$block',
+				isLimit: true
+			} );
+
+			setModelData( model,
+				'<paragraph>[]foo</paragraph>' +
+				'<table>' +
+					'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+				'</table>' +
+				'<paragraph>bar</paragraph>'
+			);
+		} );
+
+		it( 'should not crash if there is no correct position for model selection', () => {
+			setModelData( model, '' );
+
+			expect( getModelData( model ) ).to.equal( '[]' );
+		} );
+
+		it( 'should react to structure changes', () => {
+			model.change( writer => {
+				writer.remove( modelRoot.getChild( 0 ) );
+			} );
+
+			expect( getModelData( model ) ).to.equal(
+				'[<table>' +
+					'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+				'</table>]' +
+				'<paragraph>bar</paragraph>'
+			);
+		} );
+
+		it( 'should react to selection changes', () => {
+			// <paragraph>foo</paragraph>[]<table>...
+			model.change( writer => {
+				writer.setSelection(
+					ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 1 )
+				);
+			} );
+
+			expect( getModelData( model ) ).to.equal(
+				'<paragraph>foo[]</paragraph>' +
+				'<table>' +
+					'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+				'</table>' +
+				'<paragraph>bar</paragraph>'
+			);
+		} );
+
+		describe( 'not collapsed selection', () => {
+			it( 'should fix #1', () => {
+				model.change( writer => {
+					writer.setSelection( ModelRange.createFromParentsAndOffsets(
+						modelRoot.getChild( 0 ), 1,
+						modelRoot.getChild( 1 ).getChild( 0 ), 1
+					) );
+				} );
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>f[oo</paragraph>' +
+					'<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>]' +
+					'<paragraph>bar</paragraph>'
+				);
+			} );
+
+			it( 'should fix #2', () => {
+				model.change( writer => {
+					writer.setSelection( ModelRange.createFromParentsAndOffsets(
+						modelRoot.getChild( 1 ).getChild( 0 ), 1,
+						modelRoot.getChild( 2 ), 1
+					) );
+				} );
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>foo</paragraph>' +
+					'[<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>' +
+					'<paragraph>b]ar</paragraph>'
+				);
+			} );
+
+			it( 'should fix #3', () => {
+				model.change( writer => {
+					writer.setSelection( ModelRange.createFromParentsAndOffsets(
+						modelRoot.getChild( 0 ), 1,
+						modelRoot.getChild( 1 ), 0
+					) );
+				} );
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>f[oo</paragraph>' +
+					'<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>]' +
+					'<paragraph>bar</paragraph>'
+				);
+			} );
+
+			it( 'should fix #4', () => {
+				model.change( writer => {
+					writer.setSelection( ModelRange.createFromParentsAndOffsets(
+						modelRoot.getChild( 1 ).getChild( 0 ).getChild( 0 ), 1,
+						modelRoot.getChild( 1 ).getChild( 0 ).getChild( 1 ), 2
+					) );
+				} );
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>foo</paragraph>' +
+					'[<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>]' +
+					'<paragraph>bar</paragraph>'
+				);
+			} );
+
+			it( 'should fix #5', () => {
+				setModelData( model,
+					'<paragraph>foo</paragraph>' +
+					'<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>' +
+					'[]<table>' +
+						'<tableRow><tableCell>xxx</tableCell><tableCell>yyy</tableCell></tableRow>' +
+					'</table>' +
+					'<paragraph>baz</paragraph>'
+				);
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>foo</paragraph>' +
+					'[<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>]' +
+					'<table>' +
+						'<tableRow><tableCell>xxx</tableCell><tableCell>yyy</tableCell></tableRow>' +
+					'</table>' +
+					'<paragraph>baz</paragraph>'
+				);
+			} );
+
+			it( 'should not fix #1', () => {
+				setModelData( model,
+					'<paragraph>foo</paragraph>' +
+					'<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>' +
+					'<paragraph>b[ar</paragraph>' +
+					'<paragraph>ba]z</paragraph>'
+				);
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>foo</paragraph>' +
+					'<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>' +
+					'<paragraph>b[ar</paragraph>' +
+					'<paragraph>ba]z</paragraph>'
+				);
+			} );
+
+			it( 'should fix multiple ranges #1', () => {
+				model.change( writer => {
+					const ranges = [
+						new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 1, 0 ] ) ),
+						new ModelRange( new ModelPosition( modelRoot, [ 1, 0, 0, 0 ] ), new ModelPosition( modelRoot, [ 1, 1 ] ) )
+					];
+					writer.setSelection( ranges );
+				} );
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>f[oo</paragraph>' +
+					'<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>]' +
+					'<paragraph>bar</paragraph>'
+				);
+			} );
+
+			it( 'should fix multiple ranges #2', () => {
+				model.change( writer => {
+					const ranges = [
+						new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 1, 0 ] ) ),
+						new ModelRange( new ModelPosition( modelRoot, [ 1, 0, 0, 0 ] ), new ModelPosition( modelRoot, [ 2, 2 ] ) )
+					];
+
+					writer.setSelection( ranges );
+				} );
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>f[oo</paragraph>' +
+					'<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>' +
+					'<paragraph>ba]r</paragraph>'
+				);
+			} );
+
+			it( 'should fix multiple ranges #3', () => {
+				setModelData( model,
+					'<paragraph>foo</paragraph>' +
+					'<table>' +
+						'<tableRow><tableCell>[aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+						'<tableRow>]<tableCell>[aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+						'<tableRow>]<tableCell>[aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+						'<tableRow>]<tableCell>[aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>' +
+					'<paragraph>b]az</paragraph>'
+				);
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>foo</paragraph>' +
+					'[<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>' +
+					'<paragraph>b]az</paragraph>'
+				);
+			} );
+
+			it( 'should fix multiple ranges #4', () => {
+				model.change( writer => {
+					const ranges = [
+						new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 1, 0 ] ) ),
+						new ModelRange( new ModelPosition( modelRoot, [ 1, 0, 0, 0 ] ), new ModelPosition( modelRoot, [ 2, 1 ] ) ),
+						new ModelRange( new ModelPosition( modelRoot, [ 2, 2 ] ), new ModelPosition( modelRoot, [ 2, 3 ] ) )
+					];
+
+					writer.setSelection( ranges );
+				} );
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>f[oo</paragraph>' +
+					'<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>' +
+					'<paragraph>b]a[r]</paragraph>'
+				);
+			} );
+		} );
+
+		describe( 'collapsed selection', () => {
+			it( 'should fix #1', () => {
+				// <table>[]<tableRow>...
+				model.change( writer => {
+					writer.setSelection(
+						ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 1 ), 0, modelRoot.getChild( 1 ), 0 )
+					);
+				} );
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>foo[]</paragraph>' +
+					'<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>' +
+					'<paragraph>bar</paragraph>'
+				);
+			} );
+
+			it( 'should fix #2', () => {
+				// <table><tableRow>[]<tableCell>...
+				model.change( writer => {
+					const row = modelRoot.getChild( 1 ).getChild( 0 );
+
+					writer.setSelection(
+						ModelRange.createFromParentsAndOffsets( row, 0, row, 0 )
+					);
+				} );
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>foo</paragraph>' +
+					'<table>' +
+						'<tableRow><tableCell>[]aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>' +
+					'<paragraph>bar</paragraph>'
+				);
+			} );
+
+			it( 'should fix multiple ranges #1', () => {
+				// []<paragraph></paragraph>[]<table>...
+				model.change( writer => {
+					writer.setSelection(
+						[
+							ModelRange.createFromParentsAndOffsets( modelRoot, 0, modelRoot, 0 ),
+							ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 1 )
+						]
+					);
+				} );
+
+				expect( getModelData( model ) ).to.equal(
+					'<paragraph>[]foo[]</paragraph>' +
+					'<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+					'</table>' +
+					'<paragraph>bar</paragraph>'
+				);
+			} );
+		} );
+	} );
+} );

+ 2 - 2
packages/ckeditor5-engine/tests/model/writer.js

@@ -2412,11 +2412,11 @@ describe( 'Writer', () => {
 		} );
 
 		it( 'should change document selection ranges', () => {
-			setSelection( new Position( root, [ 1 ] ) );
+			setSelection( new Position( root, [ 0, 0 ] ) );
 			setSelectionFocus( new Position( root, [ 2, 2 ] ) );
 
 			expect( model.document.selection._ranges.length ).to.equal( 1 );
-			expect( model.document.selection._ranges[ 0 ].start.path ).to.deep.equal( [ 1 ] );
+			expect( model.document.selection._ranges[ 0 ].start.path ).to.deep.equal( [ 0, 0 ] );
 			expect( model.document.selection._ranges[ 0 ].end.path ).to.deep.equal( [ 2, 2 ] );
 		} );
 	} );

+ 2 - 2
packages/ckeditor5-engine/tests/view/document.js

@@ -59,7 +59,7 @@ describe( 'Document', () => {
 		} );
 	} );
 
-	describe( 'post fixers', () => {
+	describe( 'post-fixers', () => {
 		it( 'should add a callback that is called on _callPostFixers', () => {
 			const spy1 = sinon.spy();
 			const spy2 = sinon.spy();
@@ -77,7 +77,7 @@ describe( 'Document', () => {
 			sinon.assert.calledWithExactly( spy2, writerMock );
 		} );
 
-		it( 'should call post fixer until all returns false', () => {
+		it( 'should call post-fixer until all returns false', () => {
 			let calls = 0;
 
 			const spy1 = sinon.spy( () => calls++ < 2 );

+ 3 - 3
packages/ckeditor5-engine/tests/view/view/view.js

@@ -471,7 +471,7 @@ describe( 'view', () => {
 			domDiv.remove();
 		} );
 
-		it( 'should throw when someone tries to use change() method in post fixer', () => {
+		it( 'should throw when someone tries to use change() method in post-fixer', () => {
 			const domDiv = document.createElement( 'div' );
 			createViewRoot( viewDocument, 'div', 'main' );
 			view.attachDomRoot( domDiv );
@@ -536,7 +536,7 @@ describe( 'view', () => {
 			sinon.assert.callOrder( renderSpy, eventSpy );
 		} );
 
-		it( 'should call post fixers after change but before rendering', () => {
+		it( 'should call post-fixers after change but before rendering', () => {
 			const postFixer1 = sinon.spy( () => false );
 			const postFixer2 = sinon.spy( () => false );
 			const changeSpy = sinon.spy();
@@ -556,7 +556,7 @@ describe( 'view', () => {
 			sinon.assert.callOrder( changeSpy, postFixer1, postFixer2, eventSpy );
 		} );
 
-		it( 'should call post fixers until all are done', () => {
+		it( 'should call post-fixers until all are done', () => {
 			let called = false;
 			const postFixer1 = sinon.spy();
 			const postFixer2 = sinon.spy();