8
0
Pārlūkot izejas kodu

Merge branch 'master' into t/1015

Szymon Kupś 8 gadi atpakaļ
vecāks
revīzija
3853038a2a
43 mainītis faili ar 1084 papildinājumiem un 322 dzēšanām
  1. 1 2
      packages/ckeditor5-engine/package.json
  2. 0 1
      packages/ckeditor5-engine/src/conversion/viewconsumable.js
  3. 0 1
      packages/ckeditor5-engine/src/dev-utils/view.js
  4. 2 2
      packages/ckeditor5-engine/src/model/delta/attributedelta.js
  5. 5 5
      packages/ckeditor5-engine/src/model/delta/transform.js
  6. 3 3
      packages/ckeditor5-engine/src/model/item.jsdoc
  7. 82 60
      packages/ckeditor5-engine/src/model/operation/transform.js
  8. 2 0
      packages/ckeditor5-engine/src/view/document.js
  9. 46 0
      packages/ckeditor5-engine/src/view/domconverter.js
  10. 3 3
      packages/ckeditor5-engine/src/view/item.jsdoc
  11. 0 1
      packages/ckeditor5-engine/src/view/matcher.js
  12. 15 0
      packages/ckeditor5-engine/src/view/node.js
  13. 21 15
      packages/ckeditor5-engine/src/view/observer/selectionobserver.js
  14. 11 6
      packages/ckeditor5-engine/src/view/range.js
  15. 38 46
      packages/ckeditor5-engine/src/view/renderer.js
  16. 53 0
      packages/ckeditor5-engine/src/view/selection.js
  17. 69 0
      packages/ckeditor5-engine/src/view/uielement.js
  18. 1 3
      packages/ckeditor5-engine/tests/dev-utils/view.js
  19. 6 5
      packages/ckeditor5-engine/tests/manual/markers.js
  20. 13 12
      packages/ckeditor5-engine/tests/manual/nestededitable.js
  21. 16 15
      packages/ckeditor5-engine/tests/manual/placeholder.js
  22. 11 10
      packages/ckeditor5-engine/tests/manual/tickets/401/1.js
  23. 20 19
      packages/ckeditor5-engine/tests/manual/tickets/462/1.js
  24. 15 14
      packages/ckeditor5-engine/tests/manual/tickets/603/1.js
  25. 11 10
      packages/ckeditor5-engine/tests/manual/tickets/629/1.js
  26. 15 13
      packages/ckeditor5-engine/tests/manual/tickets/880/1.js
  27. 11 10
      packages/ckeditor5-engine/tests/manual/tickets/887/1.js
  28. 52 5
      packages/ckeditor5-engine/tests/model/delta/transform/transform.js
  29. 49 28
      packages/ckeditor5-engine/tests/model/operation/transform.js
  30. 1 1
      packages/ckeditor5-engine/tests/model/selection.js
  31. 134 0
      packages/ckeditor5-engine/tests/view/document/jumpoveruielement.js
  32. 1 2
      packages/ckeditor5-engine/tests/view/domconverter/binding.js
  33. 103 1
      packages/ckeditor5-engine/tests/view/domconverter/domconverter.js
  34. 4 4
      packages/ckeditor5-engine/tests/view/domconverter/uielement.js
  35. 8 3
      packages/ckeditor5-engine/tests/view/manual/uielement.html
  36. 16 7
      packages/ckeditor5-engine/tests/view/manual/uielement.js
  37. 2 0
      packages/ckeditor5-engine/tests/view/manual/uielement.md
  38. 15 0
      packages/ckeditor5-engine/tests/view/node.js
  39. 34 0
      packages/ckeditor5-engine/tests/view/observer/mutationobserver.js
  40. 47 6
      packages/ckeditor5-engine/tests/view/observer/selectionobserver.js
  41. 28 2
      packages/ckeditor5-engine/tests/view/range.js
  42. 30 1
      packages/ckeditor5-engine/tests/view/renderer.js
  43. 90 6
      packages/ckeditor5-engine/tests/view/selection.js

+ 1 - 2
packages/ckeditor5-engine/package.json

@@ -19,8 +19,7 @@
     "@ckeditor/ckeditor5-paragraph": "^0.8.0",
     "@ckeditor/ckeditor5-typing": "^0.9.1",
     "@ckeditor/ckeditor5-undo": "^0.8.1",
-    "@ckeditor/ckeditor5-widget": "^0.1.1",
-    "eslint-config-ckeditor5": "^1.0.0",
+    "eslint-config-ckeditor5": "^1.0.5",
     "gulp": "^3.9.1",
     "guppy-pre-commit": "^0.4.0"
   },

+ 0 - 1
packages/ckeditor5-engine/src/conversion/viewconsumable.js

@@ -316,7 +316,6 @@ export default class ViewConsumable {
  * @private
  */
 class ViewElementConsumables {
-
 	/**
 	 * Creates ViewElementConsumables instance.
 	 */

+ 0 - 1
packages/ckeditor5-engine/src/dev-utils/view.js

@@ -355,7 +355,6 @@ export function parse( data, options = {} ) {
  * @private
  */
 class RangeParser {
-
 	/**
 	 * Create RangeParser instance.
 	 *

+ 2 - 2
packages/ckeditor5-engine/src/model/delta/attributedelta.js

@@ -212,9 +212,9 @@ function changeRange( batch, doc, attributeKey, attributeValue, range ) {
 	// Currently position in the scanning range. Because we need value after the position, it is not a current
 	// position of the iterator but the previous one (we need to iterate one more time to get the value after).
 	let position,
-	// Value before the currently position.
+		// Value before the currently position.
 		attributeValueBefore,
-	// Value after the currently position.
+		// Value after the currently position.
 		attributeValueAfter;
 
 	for ( const value of range ) {

+ 5 - 5
packages/ckeditor5-engine/src/model/delta/transform.js

@@ -444,21 +444,21 @@ function _setForceNotSticky( context ) {
 // always treats `RemoveOperation` as a stronger one, no matter how `context.isStrong` is set. It is like this
 // to provide better results when transformations happen.
 //
-// This is however works fine only when additional context is not used.
+// This, however, works fine only when additional context is not used.
 //
 // When additional context is used, we need a better way to decide whether `RemoveOperation` is "dominating" (or in other
 // words, whether nodes removed by given operation should stay in graveyard if other operation wants to move them).
 //
 // The answer to this is easy: if `RemoveOperation` has been already undone, we are not forcing given nodes to stay
 // in graveyard. In such scenario, we set `context.forceWeakRemove` to `true`. However, if the `RemoveOperation` has
-// not been undone, we set `context.forceWeakRemove` to `false` because we want remove to be "dominating".
+// not been undone, we set `context.forceWeakRemove` to `false` because we want the operation to be "dominating".
 function _setForceWeakRemove( b, context ) {
 	const history = context.document.history;
 	const originalB = context.originalDelta.get( b );
 
-	// If `b` delta is a remove delta, that has not been undone yet, forceWeakRemove should be `false`.
-	// It should be `true` in any other case, if additional context is used.
-	context.forceWeakRemove = !( originalB instanceof RemoveDelta && !history.isUndoneDelta( originalB ) );
+	// If `b` delta has not been undone yet, forceWeakRemove should be `false`.
+	// It should be `true`, in any other case, if additional context is used.
+	context.forceWeakRemove = history.isUndoneDelta( originalB );
 }
 
 // Sets `context.wasAffected` which holds context information about how transformed deltas are related. `context.wasAffected`

+ 3 - 3
packages/ckeditor5-engine/src/model/item.jsdoc

@@ -3,9 +3,9 @@
  * For licensing, see LICENSE.md.
  */
 
- /**
-  * @module engine/model/item
-  */
+/**
+ * @module engine/model/item
+ */
 
 /**
  * Item is a {@link module:engine/model/node~Node Node} or {module:engine/model/textproxy~TextProxy TextProxy}.

+ 82 - 60
packages/ckeditor5-engine/src/model/operation/transform.js

@@ -377,13 +377,17 @@ const ot = {
 			const rangeA = Range.createFromPositionAndShift( a.sourcePosition, a.howMany );
 			const rangeB = Range.createFromPositionAndShift( b.sourcePosition, b.howMany );
 
+			// Assign `context.isStrong` to a different variable, because the value may change during execution of
+			// this algorithm and we do not want to override original `context.isStrong` that will be used in later transformations.
+			let isStrong = context.isStrong;
+
 			// Whether range moved by operation `b` is includable in operation `a` move range.
 			// For this, `a` operation has to be sticky (so `b` sticks to the range) and context has to allow stickiness.
 			const includeB = a.isSticky && !context.forceNotSticky;
 
 			// Evaluate new target position for transformed operation.
-			// Check whether there is a forced order of nodes or use `context.isStrong` flag for conflict resolving.
-			const insertBefore = context.insertBefore === undefined ? !context.isStrong : context.insertBefore;
+			// Check whether there is a forced order of nodes or use `isStrong` flag for conflict resolving.
+			const insertBefore = context.insertBefore === undefined ? !isStrong : context.insertBefore;
 
 			// `a.targetPosition` could be affected by the `b` operation. We will transform it.
 			const newTargetPosition = a.targetPosition._getTransformedByMove(
@@ -425,7 +429,7 @@ const ot = {
 				rangeA.start = rangeA.start._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, !includeB );
 				rangeA.end = rangeA.end._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, includeB );
 
-				return makeMoveOperationsFromRanges( a, [ rangeA ], newTargetPosition );
+				return makeMoveOperationsFromRanges( [ rangeA ], newTargetPosition, a );
 			}
 
 			//
@@ -441,7 +445,7 @@ const ot = {
 				rangeA.start = rangeA.start._getCombined( b.sourcePosition, b.getMovedRangeStart() );
 				rangeA.end = rangeA.end._getCombined( b.sourcePosition, b.getMovedRangeStart() );
 
-				return makeMoveOperationsFromRanges( a, [ rangeA ], newTargetPosition );
+				return makeMoveOperationsFromRanges( [ rangeA ], newTargetPosition, a );
 			}
 			//
 			// End of special case #2.
@@ -463,7 +467,7 @@ const ot = {
 				rangeA.start = rangeA.start._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, !includeB );
 				rangeA.end = rangeA.end._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, includeB );
 
-				return makeMoveOperationsFromRanges( a, [ rangeA ], newTargetPosition );
+				return makeMoveOperationsFromRanges( [ rangeA ], newTargetPosition, a );
 			}
 			//
 			// End of special case #3.
@@ -473,22 +477,22 @@ const ot = {
 			// Default case - ranges are on the same level or are not connected with each other.
 			//
 			// Modifier for default case.
-			// Modifies `context.isStrong` in certain conditions.
+			// Modifies `isStrong` flag in certain conditions.
 			//
 			// If only one of operations is a remove operation, we force remove operation to be the "stronger" one
 			// to provide more expected results. This is done only if `context.forceWeakRemove` is set to `false`.
 			// `context.forceWeakRemove` is set to `true` in certain conditions when transformation takes place during undo.
 			if ( !context.forceWeakRemove ) {
 				if ( a instanceof RemoveOperation && !( b instanceof RemoveOperation ) ) {
-					context.isStrong = true;
+					isStrong = true;
 				} else if ( !( a instanceof RemoveOperation ) && b instanceof RemoveOperation ) {
-					context.isStrong = false;
+					isStrong = false;
 				}
 			}
 
 			// Handle operation's source ranges - check how `rangeA` is affected by `b` operation.
 			// This will aggregate transformed ranges.
-			let ranges = [];
+			const ranges = [];
 
 			// Get the "difference part" of `a` operation source range.
 			// This is an array with one or two ranges. Two ranges if `rangeB` is inside `rangeA`.
@@ -496,16 +500,20 @@ const ot = {
 
 			for ( const range of difference ) {
 				// Transform those ranges by `b` operation. For example if `b` moved range from before those ranges, fix those ranges.
-				range.start = range.start._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, !includeB );
-				range.end = range.end._getTransformedByMove( b.sourcePosition, b.targetPosition, b.howMany, includeB );
+				range.start = range.start._getTransformedByDeletion( b.sourcePosition, b.howMany );
+				range.end = range.end._getTransformedByDeletion( b.sourcePosition, b.howMany );
+
+				// If `b` operation targets into `rangeA` on the same level, spread `rangeA` into two ranges.
+				const shouldSpread = compareArrays( range.start.getParentPath(), b.getMovedRangeStart().getParentPath() ) == 'same';
+				const newRanges = range._getTransformedByInsertion( b.getMovedRangeStart(), b.howMany, shouldSpread, includeB );
 
-				ranges.push( range );
+				ranges.push( ...newRanges );
 			}
 
 			// Then, we have to manage the "common part" of both move ranges.
 			const common = rangeA.getIntersection( rangeB );
 
-			if ( common !== null && context.isStrong && !bTargetsToA ) {
+			if ( common !== null && isStrong && !bTargetsToA ) {
 				// Calculate the new position of that part of original range.
 				common.start = common.start._getCombined( b.sourcePosition, b.getMovedRangeStart() );
 				common.end = common.end._getCombined( b.sourcePosition, b.getMovedRangeStart() );
@@ -521,7 +529,7 @@ const ot = {
 				}
 				// If there is one difference range, we need to check whether common part was before it or after it.
 				else if ( ranges.length == 1 ) {
-					if ( rangeB.start.isBefore( rangeA.start ) ) {
+					if ( rangeB.start.isBefore( rangeA.start ) || rangeB.start.isEqual( rangeA.start ) ) {
 						ranges.unshift( common );
 					} else {
 						ranges.push( common );
@@ -536,34 +544,11 @@ const ot = {
 
 			if ( ranges.length === 0 ) {
 				// If there are no "source ranges", nothing should be changed.
-				// Note that this can happen only if `context.isStrong == false` and `rangeA.isEqual( rangeB )`.
+				// Note that this can happen only if `isStrong == false` and `rangeA.isEqual( rangeB )`.
 				return [ new NoOperation( a.baseVersion ) ];
 			}
 
-			// At this moment we have some ranges and a target position, to which those ranges should be moved.
-			// Order in `ranges` array is the go-to order of after transformation. We can reverse the `ranges`
-			// array so the last range will be moved first. This will ensure that each transformed `MoveOperation`
-			// can have same `targetPosition` and the order of ranges will be kept.
-			ranges = ranges.reverse();
-
-			// We are almost done. We have `ranges` and `targetPosition` to make operations from.
-			// Unfortunately, those operations may affect each other. Precisely, first operation after move
-			// may affect source range of second and third operation. Same with second operation affecting third.
-			// We need to fix those source ranges once again, before converting `ranges` to operations.
-			// Keep in mind that `targetPosition` is already set in stone thanks to the trick above.
-			for ( let i = 1; i < ranges.length; i++ ) {
-				for ( let j = 0; j < i; j++ ) {
-					const howMany = ranges[ j ].end.offset - ranges[ j ].start.offset;
-
-					// Thankfully, all ranges in `ranges` array are:
-					// * non-intersecting (these are part of original `a` operation source range), and
-					// * `newTargetPosition` does not target into them (opposite would mean that `a` operation targets "inside itself").
-					// This means that the transformation will be "clean" and always return one result.
-					ranges[ i ] = ranges[ i ]._getTransformedByMove( ranges[ j ].start, newTargetPosition, howMany )[ 0 ];
-				}
-			}
-
-			return makeMoveOperationsFromRanges( a, ranges, newTargetPosition );
+			return makeMoveOperationsFromRanges( ranges, newTargetPosition, a );
 		}
 	}
 };
@@ -646,29 +631,66 @@ function joinRanges( ranges ) {
 	}
 }
 
-// Map transformed range(s) to operations and return them.
-function makeMoveOperationsFromRanges( a, ranges, targetPosition ) {
-	return ranges.map( range => {
-		// We want to keep correct operation class.
-		let OperationClass;
-
-		if ( targetPosition.root.rootName == '$graveyard' ) {
-			OperationClass = RemoveOperation;
-		} else if ( range.start.root.rootName == '$graveyard' ) {
-			OperationClass = ReinsertOperation;
-		} else {
-			OperationClass = MoveOperation;
+// Helper function for `MoveOperation` x `MoveOperation` transformation.
+// Convert given ranges and target position to move operations and return them.
+// Ranges and target position will be transformed on-the-fly when generating operations.
+// Given `ranges` should be in the order of how they were in the original transformed operation.
+// Given `targetPosition` is the target position of the first range from `ranges`.
+function makeMoveOperationsFromRanges( ranges, targetPosition, a ) {
+	// At this moment we have some ranges and a target position, to which those ranges should be moved.
+	// Order in `ranges` array is the go-to order of after transformation.
+	//
+	// We are almost done. We have `ranges` and `targetPosition` to make operations from.
+	// Unfortunately, those operations may affect each other. Precisely, first operation after move
+	// may affect source range and target position of second and third operation. Same with second
+	// operation affecting third.
+	//
+	// We need to fix those source ranges and target positions once again, before converting `ranges` to operations.
+	const operations = [];
+
+	// Keep in mind that nothing will be transformed if there is just one range in `ranges`.
+	for ( let i = 0; i < ranges.length; i++ ) {
+		// Create new operation out of a range and target position.
+		const op = makeMoveOperation( ranges[ i ], targetPosition, a.isSticky );
+
+		operations.push( op );
+
+		// Transform other ranges by the generated operation.
+		for ( let j = i + 1; j < ranges.length; j++ ) {
+			// All ranges in `ranges` array should be:
+			// * non-intersecting (these are part of original operation source range), and
+			// * `targetPosition` does not target into them (opposite would mean that transformed operation targets "inside itself").
+			//
+			// This means that the transformation will be "clean" and always return one result.
+			ranges[ j ] = ranges[ j ]._getTransformedByMove( op.sourcePosition, op.targetPosition, op.howMany )[ 0 ];
 		}
 
-		const result = new OperationClass(
-			range.start,
-			range.end.offset - range.start.offset,
-			targetPosition,
-			0 // Is corrected anyway later.
-		);
+		targetPosition = targetPosition._getTransformedByMove( op.sourcePosition, op.targetPosition, op.howMany, true, false );
+	}
+
+	return operations;
+}
+
+function makeMoveOperation( range, targetPosition, isSticky ) {
+	// We want to keep correct operation class.
+	let OperationClass;
+
+	if ( targetPosition.root.rootName == '$graveyard' ) {
+		OperationClass = RemoveOperation;
+	} else if ( range.start.root.rootName == '$graveyard' ) {
+		OperationClass = ReinsertOperation;
+	} else {
+		OperationClass = MoveOperation;
+	}
+
+	const result = new OperationClass(
+		range.start,
+		range.end.offset - range.start.offset,
+		targetPosition,
+		0 // Is corrected anyway later.
+	);
 
-		result.isSticky = a.isSticky;
+	result.isSticky = isSticky;
 
-		return result;
-	} );
+	return result;
 }

+ 2 - 0
packages/ckeditor5-engine/src/view/document.js

@@ -12,6 +12,7 @@ import Renderer from './renderer';
 import DomConverter from './domconverter';
 import RootEditableElement from './rooteditableelement';
 import { injectQuirksHandling } from './filler';
+import { injectUiElementHandling } from './uielement';
 import log from '@ckeditor/ckeditor5-utils/src/log';
 import MutationObserver from './observer/mutationobserver';
 import SelectionObserver from './observer/selectionobserver';
@@ -126,6 +127,7 @@ export default class Document {
 		this.addObserver( FakeSelectionObserver );
 
 		injectQuirksHandling( this );
+		injectUiElementHandling( this );
 
 		this.decorate( 'render' );
 	}

+ 46 - 0
packages/ckeditor5-engine/src/view/domconverter.js

@@ -839,6 +839,52 @@ export default class DomConverter {
 	}
 
 	/**
+	 * Checks if given selection's boundaries are at correct places.
+	 *
+	 * The following places are considered as incorrect for selection boundaries:
+	 * * before or in the middle of the inline filler sequence,
+	 * * inside the DOM element which represents {@link module:engine/view/uielement~UIElement a view ui element}.
+	 *
+	 * @param {Selection} domSelection DOM Selection object to be checked.
+	 * @returns {Boolean} `true` if the given selection is at a correct place, `false` otherwise.
+	 */
+	isDomSelectionCorrect( domSelection ) {
+		return this._isDomSelectionPositionCorrect( domSelection.anchorNode, domSelection.anchorOffset ) &&
+			this._isDomSelectionPositionCorrect( domSelection.focusNode, domSelection.focusOffset );
+	}
+
+	/**
+	 * Checks if the given DOM position is a correct place for selection boundary. See {@link #isDomSelectionCorrect}.
+	 *
+	 * @private
+	 * @param {Element} domParent Position parent.
+	 * @param {Number} offset Position offset.
+	 * @returns {Boolean} `true` if given position is at a correct place for selection boundary, `false` otherwise.
+	 */
+	_isDomSelectionPositionCorrect( domParent, offset ) {
+		// If selection is before or in the middle of inline filler string, it is incorrect.
+		if ( this.isText( domParent ) && startsWithFiller( domParent ) && offset < INLINE_FILLER_LENGTH ) {
+			// Selection in a text node, at wrong position (before or in the middle of filler).
+			return false;
+		}
+
+		if ( this.isElement( domParent ) && startsWithFiller( domParent.childNodes[ offset ] ) ) {
+			// Selection in an element node, before filler text node.
+			return false;
+		}
+
+		const viewParent = this.mapDomToView( domParent );
+
+		// If selection is in `view.UIElement`, it is incorrect. Note that `mapDomToView()` returns `view.UIElement`
+		// also for any dom element that is inside the view ui element (so we don't need to perform any additional checks).
+		if ( viewParent && viewParent.is( 'uiElement' ) ) {
+			return false;
+		}
+
+		return true;
+	}
+
+	/**
 	 * Takes text data from given {@link module:engine/view/text~Text#data} and processes it so it is correctly displayed in DOM.
 	 *
 	 * Following changes are done:

+ 3 - 3
packages/ckeditor5-engine/src/view/item.jsdoc

@@ -3,9 +3,9 @@
  * For licensing, see LICENSE.md.
  */
 
- /**
-  * @module engine/view/item
-  */
+/**
+ * @module engine/view/item
+ */
 
 /**
  * Item is a {@link module:engine/view/node~Node Node} or {module:engine/view/textproxy~TextProxy TextProxy}.

+ 0 - 1
packages/ckeditor5-engine/src/view/matcher.js

@@ -212,7 +212,6 @@ export default class Matcher {
 
 		return ( typeof pattern != 'function' && name && !( name instanceof RegExp ) ) ? name : null;
 	}
-
 }
 
 // Returns match information if {@link module:engine/view/element~Element element} is matching provided pattern.

+ 15 - 0
packages/ckeditor5-engine/src/view/node.js

@@ -10,6 +10,7 @@
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
+import clone from '@ckeditor/ckeditor5-utils/src/lib/lodash/clone';
 
 /**
  * Abstract tree view node class.
@@ -182,6 +183,20 @@ export default class Node {
 	}
 
 	/**
+	 * Custom toJSON method to solve child-parent circular dependencies.
+	 *
+	 * @returns {Object} Clone of this object with the parent property removed.
+	 */
+	toJSON() {
+		const json = clone( this );
+
+		// Due to circular references we need to remove parent reference.
+		delete json.parent;
+
+		return json;
+	}
+
+	/**
 	 * Clones this node.
 	 *
 	 * @method #clone

+ 21 - 15
packages/ckeditor5-engine/src/view/observer/selectionobserver.js

@@ -148,7 +148,7 @@ export default class SelectionObserver extends Observer {
 		const domSelection = domDocument.defaultView.getSelection();
 		const newViewSelection = this.domConverter.domSelectionToView( domSelection );
 
-		if ( this.selection.isEqual( newViewSelection ) ) {
+		if ( this.selection.isEqual( newViewSelection ) && this.domConverter.isDomSelectionCorrect( domSelection ) ) {
 			return;
 		}
 
@@ -169,20 +169,26 @@ export default class SelectionObserver extends Observer {
 			return;
 		}
 
-		const data = {
-			oldSelection: this.selection,
-			newSelection: newViewSelection,
-			domSelection
-		};
-
-		// Should be fired only when selection change was the only document change.
-		this.document.fire( 'selectionChange', data );
-
-		// Call` #_fireSelectionChangeDoneDebounced` every time when `selectionChange` event is fired.
-		// This function is debounced what means that `selectionChangeDone` event will be fired only when
-		// defined int the function time will elapse since the last time the function was called.
-		// So `selectionChangeDone` will be fired when selection will stop changing.
-		this._fireSelectionChangeDoneDebounced( data );
+		if ( this.selection.isSimilar( newViewSelection ) ) {
+			// If selection was equal and we are at this point of algorithm, it means that it was incorrect.
+			// Just re-render it, no need to fire any events, etc.
+			this.document.render();
+		} else {
+			const data = {
+				oldSelection: this.selection,
+				newSelection: newViewSelection,
+				domSelection
+			};
+
+			// Prepare data for new selection and fire appropriate events.
+			this.document.fire( 'selectionChange', data );
+
+			// Call` #_fireSelectionChangeDoneDebounced` every time when `selectionChange` event is fired.
+			// This function is debounced what means that `selectionChangeDone` event will be fired only when
+			// defined int the function time will elapse since the last time the function was called.
+			// So `selectionChangeDone` will be fired when selection will stop changing.
+			this._fireSelectionChangeDoneDebounced( data );
+		}
 	}
 
 	/**

+ 11 - 6
packages/ckeditor5-engine/src/view/range.js

@@ -98,8 +98,8 @@ export default class Range {
 	 * @returns {module:engine/view/range~Range} Enlarged range.
 	 */
 	getEnlarged() {
-		let start = this.start.getLastMatchingPosition( enlargeShrinkSkip, { direction: 'backward' } );
-		let end = this.end.getLastMatchingPosition( enlargeShrinkSkip );
+		let start = this.start.getLastMatchingPosition( enlargeTrimSkip, { direction: 'backward' } );
+		let end = this.end.getLastMatchingPosition( enlargeTrimSkip );
 
 		// Fix positions, in case if they are in Text node.
 		if ( start.parent.is( 'text' ) && start.isAtStart ) {
@@ -130,8 +130,13 @@ export default class Range {
 	 * @returns {module:engine/view/range~Range} Shrink range.
 	 */
 	getTrimmed() {
-		let start = this.start.getLastMatchingPosition( enlargeShrinkSkip );
-		let end = this.end.getLastMatchingPosition( enlargeShrinkSkip, { direction: 'backward' } );
+		let start = this.start.getLastMatchingPosition( enlargeTrimSkip );
+
+		if ( start.isAfter( this.end ) || start.isEqual( this.end ) ) {
+			return new Range( start, start );
+		}
+
+		let end = this.end.getLastMatchingPosition( enlargeTrimSkip, { direction: 'backward' } );
 		const nodeAfterStart = start.nodeAfter;
 		const nodeBeforeEnd = end.nodeBefore;
 
@@ -437,8 +442,8 @@ export default class Range {
 	}
 }
 
-// Function used by getEnlagred and getShrinked methods.
-function enlargeShrinkSkip( value ) {
+// Function used by getEnlagred and getTrimmed methods.
+function enlargeTrimSkip( value ) {
 	if ( value.item.is( 'attributeElement' ) || value.item.is( 'uiElement' ) ) {
 		return true;
 	}

+ 38 - 46
packages/ckeditor5-engine/src/view/renderer.js

@@ -9,7 +9,6 @@
 
 import ViewText from './text';
 import ViewPosition from './position';
-import Selection from './selection';
 import { INLINE_FILLER, INLINE_FILLER_LENGTH, startsWithFiller, isInlineFiller, isBlockFiller } from './filler';
 
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
@@ -602,13 +601,45 @@ export default class Renderer {
 	 */
 	_updateDomSelection( domRoot ) {
 		const domSelection = domRoot.ownerDocument.defaultView.getSelection();
+
+		// Let's check whether DOM selection needs updating at all.
+		if ( !this._domSelectionNeedsUpdate( domSelection ) ) {
+			return;
+		}
+
+		// Multi-range selection is not available in most browsers, and, at least in Chrome, trying to
+		// set such selection, that is not continuous, throws an error. Because of that, we will just use anchor
+		// and focus of view selection.
+		// Since we are not supporting multi-range selection, we also do not need to check if proper editable is
+		// selected. If there is any editable selected, it is okay (editable is taken from selection anchor).
+		const anchor = this.domConverter.viewPositionToDom( this.selection.anchor );
+		const focus = this.domConverter.viewPositionToDom( this.selection.focus );
+
+		domSelection.collapse( anchor.parent, anchor.offset );
+		domSelection.extend( focus.parent, focus.offset );
+	}
+
+	/**
+	 * Checks whether given DOM selection needs to be updated.
+	 *
+	 * @private
+	 * @param {Selection} domSelection DOM selection to check.
+	 * @returns {Boolean}
+	 */
+	_domSelectionNeedsUpdate( domSelection ) {
+		if ( !this.domConverter.isDomSelectionCorrect( domSelection ) ) {
+			// Current DOM selection is in incorrect position. We need to update it.
+			return true;
+		}
+
 		const oldViewSelection = domSelection && this.domConverter.domSelectionToView( domSelection );
 
 		if ( oldViewSelection && this.selection.isEqual( oldViewSelection ) ) {
-			return;
+			return false;
 		}
 
-		if ( oldViewSelection && areSimilarSelections( oldViewSelection, this.selection ) ) {
+		// If selection is not collapsed, it does not need to be updated if it is similar.
+		if ( !this.selection.isCollapsed && this.selection.isSimilar( oldViewSelection ) ) {
 			const data = {
 				oldSelection: oldViewSelection,
 				currentSelection: this.selection
@@ -619,19 +650,12 @@ export default class Renderer {
 				data
 			);
 
-			return;
+			// Selection did not changed and is correct, do not update.
+			return false;
 		}
 
-		// Multi-range selection is not available in most browsers, and, at least in Chrome, trying to
-		// set such selection, that is not continuous, throws an error. Because of that, we will just use anchor
-		// and focus of view selection.
-		// Since we are not supporting multi-range selection, we also do not need to check if proper editable is
-		// selected. If there is any editable selected, it is okay (editable is taken from selection anchor).
-		const anchor = this.domConverter.viewPositionToDom( this.selection.anchor );
-		const focus = this.domConverter.viewPositionToDom( this.selection.focus );
-
-		domSelection.collapse( anchor.parent, anchor.offset );
-		domSelection.extend( focus.parent, focus.offset );
+		// Selections are not similar.
+		return true;
 	}
 
 	/**
@@ -684,35 +708,3 @@ export default class Renderer {
 }
 
 mix( Renderer, ObservableMixin );
-
-// Checks if two given selections are similar. Selections are considered similar if they are non-collapsed
-// and their trimmed (see {@link #_trimSelection}) representations are equal.
-//
-// @private
-// @param {module:engine/view/selection~Selection} selection1
-// @param {module:engine/view/selection~Selection} selection2
-// @returns {Boolean}
-function areSimilarSelections( selection1, selection2 ) {
-	return !selection1.isCollapsed && trimSelection( selection1 ).isEqual( trimSelection( selection2 ) );
-}
-
-// Creates a copy of a given selection with all of its ranges
-// trimmed (see {@link module:engine/view/range~Range#getTrimmed getTrimmed}).
-//
-// @private
-// @param {module:engine/view/selection~Selection} selection
-// @returns {module:engine/view/selection~Selection} Selection copy with all ranges trimmed.
-function trimSelection( selection ) {
-	const newSelection = Selection.createFromSelection( selection );
-	const ranges = newSelection.getRanges();
-
-	const trimmedRanges = [];
-
-	for ( const range of ranges ) {
-		trimmedRanges.push( range.getTrimmed() );
-	}
-
-	newSelection.setRanges( trimmedRanges, newSelection.isBackward );
-
-	return newSelection;
-}

+ 53 - 0
packages/ckeditor5-engine/src/view/selection.js

@@ -13,6 +13,7 @@ import Position from './position';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
 import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin';
 import Element from './element';
+import count from '@ckeditor/ckeditor5-utils/src/count';
 
 /**
  * Class representing selection in tree view.
@@ -207,6 +208,7 @@ export default class Selection {
 	 *
 	 * @fires change
 	 * @param {module:engine/view/range~Range} range
+	 * @param {Boolean} isBackward
 	 */
 	addRange( range, isBackward ) {
 		if ( !( range instanceof Range ) ) {
@@ -339,6 +341,57 @@ export default class Selection {
 	}
 
 	/**
+	 * Checks whether this selection is similar to given selection. Selections are similar if they have same directions, same
+	 * number of ranges, and all {@link module:engine/view/range~Range#getTrimmed trimmed} ranges from one selection are
+	 * equal to any trimmed range from other selection.
+	 *
+	 * @param {module:engine/view/selection~Selection} otherSelection Selection to compare with.
+	 * @returns {Boolean} `true` if selections are similar, `false` otherwise.
+	 */
+	isSimilar( otherSelection ) {
+		if ( this.isBackward != otherSelection.isBackward ) {
+			return false;
+		}
+
+		const numOfRangesA = count( this.getRanges() );
+		const numOfRangesB = count( otherSelection.getRanges() );
+
+		// If selections have different number of ranges, they cannot be similar.
+		if ( numOfRangesA != numOfRangesB ) {
+			return false;
+		}
+
+		// If both selections have no ranges, they are similar.
+		if ( numOfRangesA == 0 ) {
+			return true;
+		}
+
+		// Check if each range in one selection has a similar range in other selection.
+		for ( let rangeA of this.getRanges() ) {
+			rangeA = rangeA.getTrimmed();
+
+			let found = false;
+
+			for ( let rangeB of otherSelection.getRanges() ) {
+				rangeB = rangeB.getTrimmed();
+
+				if ( rangeA.start.isEqual( rangeB.start ) && rangeA.end.isEqual( rangeB.end ) ) {
+					found = true;
+					break;
+				}
+			}
+
+			// For `rangeA`, neither range in `otherSelection` was similar. So selections are not similar.
+			if ( !found ) {
+				return false;
+			}
+		}
+
+		// There were no ranges that weren't matched. Selections are similar.
+		return true;
+	}
+
+	/**
 	 * Removes all ranges that were added to the selection.
 	 *
 	 * @fires change

+ 69 - 0
packages/ckeditor5-engine/src/view/uielement.js

@@ -10,6 +10,7 @@
 import Element from './element';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import Node from './node';
+import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
 
 /**
  * UIElement class. It is used to represent UI not a content of the document.
@@ -82,9 +83,77 @@ export default class UIElement extends Element {
 	}
 }
 
+/**
+ * This function injects UI element handling to the given {@link module:engine/view/document~Document document}.
+ *
+ * A callback is added to {@link module:engine/view/document~Document#event:keydown document keydown event}.
+ * The callback handles the situation when right arrow key is pressed and selection is collapsed before a UI element.
+ * Without this handler, it would be impossible to "jump over" UI element using right arrow key.
+ *
+ * @param {module:engine/view/document~Document} document Document to which the quirks handling will be injected.
+ */
+export function injectUiElementHandling( document ) {
+	document.on( 'keydown', ( evt, data ) => jumpOverUiElement( evt, data, document.domConverter ) );
+}
+
 // Returns `null` because block filler is not needed for UIElements.
 //
 // @returns {null}
 function getFillerOffset() {
 	return null;
 }
+
+// Selection cannot be placed in a `UIElement`. Whenever it is placed there, it is moved before it. This
+// causes a situation when it is impossible to jump over `UIElement` using right arrow key, because the selection
+// ends up in ui element (in DOM) and is moved back to the left. This handler fixes this situation.
+function jumpOverUiElement( evt, data, domConverter ) {
+	if ( data.keyCode == keyCodes.arrowright ) {
+		const domSelection = data.domTarget.ownerDocument.defaultView.getSelection();
+		const domSelectionCollapsed = domSelection.rangeCount == 1 && domSelection.getRangeAt( 0 ).collapsed;
+
+		// Jump over UI element if selection is collapsed or shift key is pressed. These are the cases when selection would extend.
+		if ( domSelectionCollapsed || data.shiftKey ) {
+			const domParent = domSelection.focusNode;
+			const domOffset = domSelection.focusOffset;
+
+			const viewPosition = domConverter.domPositionToView( domParent, domOffset );
+
+			// In case if dom element is not converted to view or is not mapped or something. Happens for example in some tests.
+			if ( viewPosition === null ) {
+				return;
+			}
+
+			// Skip all following ui elements.
+			let jumpedOverAnyUiElement = false;
+
+			const nextViewPosition = viewPosition.getLastMatchingPosition( value => {
+				if ( value.item.is( 'uiElement' ) ) {
+					// Remember that there was at least one ui element.
+					jumpedOverAnyUiElement = true;
+				}
+
+				// Jump over ui elements, jump over empty attribute elements, move up from inside of attribute element.
+				if ( value.item.is( 'uiElement' ) || value.item.is( 'attributeElement' ) ) {
+					return true;
+				}
+
+				// Don't jump over text or don't get out of container element.
+				return false;
+			} );
+
+			// If anything has been skipped, fix position.
+			// This `if` could be possibly omitted but maybe it is better not to mess with DOM selection if not needed.
+			if ( jumpedOverAnyUiElement ) {
+				const newDomPosition = domConverter.viewPositionToDom( nextViewPosition );
+
+				if ( domSelectionCollapsed ) {
+					// Selection was collapsed, so collapse it at further position.
+					domSelection.collapse( newDomPosition.parent, newDomPosition.offset );
+				} else {
+					// Selection was not collapse, so extend it instead of collapsing.
+					domSelection.extend( newDomPosition.parent, newDomPosition.offset );
+				}
+			}
+		}
+	}
+}

+ 1 - 3
packages/ckeditor5-engine/tests/dev-utils/view.js

@@ -611,9 +611,7 @@ describe( 'view test utils', () => {
 		} );
 
 		it( 'should throw when wrong type is provided', () => {
-			sinon.stub( XmlDataProcessor.prototype, 'toView', () => {
-				return new ContainerElement( 'invalidType:b' );
-			} );
+			sinon.stub( XmlDataProcessor.prototype, 'toView' ).returns( new ContainerElement( 'invalidType:b' ) );
 
 			expect( () => {
 				parse( 'sth' );

+ 6 - 5
packages/ckeditor5-engine/tests/manual/markers.js

@@ -23,10 +23,11 @@ const markerNames = [];
 let model = null;
 let _uid = 1;
 
-ClassicEditor.create( document.querySelector( '#editor' ), {
-	plugins: [ Enter, Typing, Paragraph, Bold, Italic, List, Heading, Undo ],
-	toolbar: [ 'headings', 'bold', 'italic', 'bulletedList', 'numberedList', 'undo', 'redo' ]
-} )
+ClassicEditor
+	.create( document.querySelector( '#editor' ), {
+		plugins: [ Enter, Typing, Paragraph, Bold, Italic, List, Heading, Undo ],
+		toolbar: [ 'headings', 'bold', 'italic', 'bulletedList', 'numberedList', 'undo', 'redo' ]
+	} )
 	.then( editor => {
 		window.editor = editor;
 		model = window.editor.editing.model;
@@ -74,7 +75,7 @@ ClassicEditor.create( document.querySelector( '#editor' ), {
 
 		model.enqueueChanges( () => {
 			const root = model.getRoot();
-			const range = new Range( new Position( root, [ 0, 10 ] ), new Position( root, [ 1, 5 ] ) );
+			const range = new Range( new Position( root, [ 0, 10 ] ), new Position( root, [ 0, 16 ] ) );
 			const name = 'highlight:yellow:' + uid();
 
 			markerNames.push( name );

+ 13 - 12
packages/ckeditor5-engine/tests/manual/nestededitable.js

@@ -69,21 +69,22 @@ class NestedEditable extends Plugin {
 	}
 }
 
-ClassicEditor.create( global.document.querySelector( '#editor' ), {
-	plugins: [ Enter, Typing, Paragraph, NestedEditable, Undo ],
-	toolbar: [ 'undo', 'redo' ]
-} )
-.then( editor => {
-	editor.document.on( 'changesDone', () => {
+ClassicEditor
+	.create( global.document.querySelector( '#editor' ), {
+		plugins: [ Enter, Typing, Paragraph, NestedEditable, Undo ],
+		toolbar: [ 'undo', 'redo' ]
+	} )
+	.then( editor => {
+		editor.document.on( 'changesDone', () => {
+			printModelContents( editor );
+		} );
+
 		printModelContents( editor );
+	} )
+	.catch( err => {
+		console.error( err.stack );
 	} );
 
-	printModelContents( editor );
-} )
-.catch( err => {
-	console.error( err.stack );
-} );
-
 const modelDiv = global.document.querySelector( '#model' );
 function printModelContents( editor ) {
 	modelDiv.innerText = getData( editor.document );

+ 16 - 15
packages/ckeditor5-engine/tests/manual/placeholder.js

@@ -14,19 +14,20 @@ import Heading from '@ckeditor/ckeditor5-heading/src/heading';
 import global from '@ckeditor/ckeditor5-utils/src/dom/global';
 import { attachPlaceholder } from '../../src/view/placeholder';
 
-ClassicEditor.create( global.document.querySelector( '#editor' ), {
-	plugins: [ Enter, Typing, Paragraph, Undo, Heading ],
-	toolbar: [ 'headings', 'undo', 'redo' ]
-} )
-.then( editor => {
-	const viewDoc = editor.editing.view;
-	const header = viewDoc.getRoot().getChild( 0 );
-	const paragraph = viewDoc.getRoot().getChild( 1 );
+ClassicEditor
+	.create( global.document.querySelector( '#editor' ), {
+		plugins: [ Enter, Typing, Paragraph, Undo, Heading ],
+		toolbar: [ 'headings', 'undo', 'redo' ]
+	} )
+	.then( editor => {
+		const viewDoc = editor.editing.view;
+		const header = viewDoc.getRoot().getChild( 0 );
+		const paragraph = viewDoc.getRoot().getChild( 1 );
 
-	attachPlaceholder( header, 'Type some header text...' );
-	attachPlaceholder( paragraph, 'Type some paragraph text...' );
-	viewDoc.render();
-} )
-.catch( err => {
-	console.error( err.stack );
-} );
+		attachPlaceholder( header, 'Type some header text...' );
+		attachPlaceholder( paragraph, 'Type some paragraph text...' );
+		viewDoc.render();
+	} )
+	.catch( err => {
+		console.error( err.stack );
+	} );

+ 11 - 10
packages/ckeditor5-engine/tests/manual/tickets/401/1.js

@@ -11,13 +11,14 @@ import Typing from '@ckeditor/ckeditor5-typing/src/typing';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
 
-ClassicEditor.create( document.getElementById( 'editor' ), {
-	plugins: [ Enter, Typing, Paragraph, Bold ],
-	toolbar: [ 'bold' ]
-} )
-.then( editor => {
-	window.editor = editor;
-} )
-.catch( err => {
-	console.error( err.stack );
-} );
+ClassicEditor
+	.create( document.getElementById( 'editor' ), {
+		plugins: [ Enter, Typing, Paragraph, Bold ],
+		toolbar: [ 'bold' ]
+	} )
+	.then( editor => {
+		window.editor = editor;
+	} )
+	.catch( err => {
+		console.error( err.stack );
+	} );

+ 20 - 19
packages/ckeditor5-engine/tests/manual/tickets/462/1.js

@@ -12,25 +12,26 @@ import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
 import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic';
 
-ClassicEditor.create( document.querySelector( '#editor' ), {
-	plugins: [ Enter, Typing, Paragraph, Bold, Italic ],
-	toolbar: [ 'bold', 'italic' ]
-} )
-.then( editor => {
-	window.editor = editor;
+ClassicEditor
+	.create( document.querySelector( '#editor' ), {
+		plugins: [ Enter, Typing, Paragraph, Bold, Italic ],
+		toolbar: [ 'bold', 'italic' ]
+	} )
+	.then( editor => {
+		window.editor = editor;
 
-	setInterval( () => {
-		console.clear();
+		setInterval( () => {
+			console.clear();
 
-		const domSelection = document.getSelection();
-		const selectionExists = domSelection && domSelection.anchorNode;
+			const domSelection = document.getSelection();
+			const selectionExists = domSelection && domSelection.anchorNode;
 
-		console.log( editor.editing.view.getDomRoot().innerHTML.replace( /\u200b/g, '@' ) );
-		console.log( 'selection.hasAttribute( italic ):', editor.document.selection.hasAttribute( 'italic' ) );
-		console.log( 'selection.hasAttribute( bold ):', editor.document.selection.hasAttribute( 'bold' ) );
-		console.log( 'selection anchor\'s parentNode:', selectionExists ? domSelection.anchorNode.parentNode : 'no DOM selection' );
-	}, 2000 );
-} )
-.catch( err => {
-	console.error( err.stack );
-} );
+			console.log( editor.editing.view.getDomRoot().innerHTML.replace( /\u200b/g, '@' ) );
+			console.log( 'selection.hasAttribute( italic ):', editor.document.selection.hasAttribute( 'italic' ) );
+			console.log( 'selection.hasAttribute( bold ):', editor.document.selection.hasAttribute( 'bold' ) );
+			console.log( 'selection anchor\'s parentNode:', selectionExists ? domSelection.anchorNode.parentNode : 'no DOM selection' );
+		}, 2000 );
+	} )
+	.catch( err => {
+		console.error( err.stack );
+	} );

+ 15 - 14
packages/ckeditor5-engine/tests/manual/tickets/603/1.js

@@ -13,20 +13,21 @@ import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
 import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic';
 
-ClassicEditor.create( document.querySelector( '#editor' ), {
-	plugins: [ Enter, Typing, Paragraph, Heading, Bold, Italic ],
-	toolbar: [ 'headings', 'bold', 'italic' ]
-} )
-.then( editor => {
-	window.editor = editor;
+ClassicEditor
+	.create( document.querySelector( '#editor' ), {
+		plugins: [ Enter, Typing, Paragraph, Heading, Bold, Italic ],
+		toolbar: [ 'headings', 'bold', 'italic' ]
+	} )
+	.then( editor => {
+		window.editor = editor;
 
-	const sel = editor.document.selection;
+		const sel = editor.document.selection;
 
-	sel.on( 'change', ( evt, data ) => {
-		const date = new Date();
-		console.log( `${ date.getSeconds() }s${ String( date.getMilliseconds() ).slice( 0, 2 ) }ms`, evt.name, data );
+		sel.on( 'change', ( evt, data ) => {
+			const date = new Date();
+			console.log( `${ date.getSeconds() }s${ String( date.getMilliseconds() ).slice( 0, 2 ) }ms`, evt.name, data );
+		} );
+	} )
+	.catch( err => {
+		console.error( err.stack );
 	} );
-} )
-.catch( err => {
-	console.error( err.stack );
-} );

+ 11 - 10
packages/ckeditor5-engine/tests/manual/tickets/629/1.js

@@ -11,13 +11,14 @@ import Typing from '@ckeditor/ckeditor5-typing/src/typing';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
 
-ClassicEditor.create( document.querySelector( '#editor' ), {
-	plugins: [ Enter, Typing, Paragraph, Bold ],
-	toolbar: [ 'bold' ]
-} )
-.then( editor => {
-	window.editor = editor;
-} )
-.catch( err => {
-	console.error( err.stack );
-} );
+ClassicEditor
+	.create( document.querySelector( '#editor' ), {
+		plugins: [ Enter, Typing, Paragraph, Bold ],
+		toolbar: [ 'bold' ]
+	} )
+	.then( editor => {
+		window.editor = editor;
+	} )
+	.catch( err => {
+		console.error( err.stack );
+	} );

+ 15 - 13
packages/ckeditor5-engine/tests/manual/tickets/880/1.js

@@ -10,18 +10,20 @@ import EssentialsPreset from '@ckeditor/ckeditor5-presets/src/essentials';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
 
-ClassicEditor.create( document.querySelector( '#editor' ), {
-	plugins: [ EssentialsPreset, Paragraph, Bold ],
-	toolbar: [ 'undo', 'redo' ]
-} )
-.then( editor => {
-	window.editor = editor;
+ClassicEditor
+	.create( document.querySelector( '#editor' ), {
+		plugins: [ EssentialsPreset, Paragraph, Bold ],
+		toolbar: [ 'undo', 'redo' ]
+	} )
+	.then( editor => {
+		window.editor = editor;
 
-	editor.editing.view.on( 'selectionChange', () => {
-		editor.document.enqueueChanges( () => {} );
-		console.log( 'selectionChange', ( new Date() ).getTime() );
+		editor.editing.view.on( 'selectionChange', () => {
+			editor.document.enqueueChanges( () => {
+			} );
+			console.log( 'selectionChange', ( new Date() ).getTime() );
+		} );
+	} )
+	.catch( err => {
+		console.error( err.stack );
 	} );
-} )
-.catch( err => {
-	console.error( err.stack );
-} );

+ 11 - 10
packages/ckeditor5-engine/tests/manual/tickets/887/1.js

@@ -10,13 +10,14 @@ import EssentialsPreset from '@ckeditor/ckeditor5-presets/src/essentials';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
 
-ClassicEditor.create( document.querySelector( '#editor' ), {
-	plugins: [ EssentialsPreset, Paragraph, Bold ],
-	toolbar: [ 'undo', 'redo' ]
-} )
-.then( editor => {
-	window.editor = editor;
-} )
-.catch( err => {
-	console.error( err.stack );
-} );
+ClassicEditor
+	.create( document.querySelector( '#editor' ), {
+		plugins: [ EssentialsPreset, Paragraph, Bold ],
+		toolbar: [ 'undo', 'redo' ]
+	} )
+	.then( editor => {
+		window.editor = editor;
+	} )
+	.catch( err => {
+		console.error( err.stack );
+	} );

+ 52 - 5
packages/ckeditor5-engine/tests/model/delta/transform/transform.js

@@ -17,6 +17,7 @@ import InsertDelta from '../../../../src/model/delta/insertdelta';
 import RemoveDelta from '../../../../src/model/delta/removedelta';
 import MoveDelta from '../../../../src/model/delta/movedelta';
 import SplitDelta from '../../../../src/model/delta/splitdelta';
+import UnwrapDelta from '../../../../src/model/delta/unwrapdelta';
 
 import NoOperation from '../../../../src/model/operation/nooperation';
 import MoveOperation from '../../../../src/model/operation/moveoperation';
@@ -30,7 +31,8 @@ import {
 	getSplitDelta,
 	getRemoveDelta,
 	getMoveDelta,
-	getMergeDelta
+	getMergeDelta,
+	getUnwrapDelta
 } from '../../../../tests/model/delta/transform/_utils/utils';
 
 describe( 'transform', () => {
@@ -115,7 +117,7 @@ describe( 'transform', () => {
 				operations: [
 					{
 						type: RemoveOperation,
-						sourcePosition: new Position( root, [ 1, 0 ] ),
+						sourcePosition: new Position( root, [ 0, 2 ] ),
 						howMany: 1,
 						baseVersion: 3
 					}
@@ -127,7 +129,7 @@ describe( 'transform', () => {
 				operations: [
 					{
 						type: RemoveOperation,
-						sourcePosition: new Position( root, [ 0, 2 ] ),
+						sourcePosition: new Position( root, [ 1, 0 ] ),
 						howMany: 1,
 						baseVersion: 4
 					}
@@ -163,7 +165,7 @@ describe( 'transform', () => {
 				operations: [
 					{
 						type: RemoveOperation,
-						sourcePosition: new Position( root, [ 1, 0 ] ),
+						sourcePosition: new Position( root, [ 0, 2 ] ),
 						howMany: 1,
 						baseVersion: 3
 					}
@@ -175,7 +177,7 @@ describe( 'transform', () => {
 				operations: [
 					{
 						type: RemoveOperation,
-						sourcePosition: new Position( root, [ 0, 2 ] ),
+						sourcePosition: new Position( root, [ 1, 0 ] ),
 						howMany: 1,
 						baseVersion: 4
 					}
@@ -341,6 +343,51 @@ describe( 'transform', () => {
 				} );
 			} );
 
+			// #1053.
+			it( 'remove operation importance should be set correctly also for deltas other than remove delta', () => {
+				const unwrapDelta = getUnwrapDelta( new Position( root, [ 0 ] ), 2, baseVersion );
+
+				const nodeCopy = new Element( 'p' );
+				const splitDelta = getSplitDelta( new Position( root, [ 0, 0, 1 ] ), nodeCopy, 1, baseVersion );
+				const mergeDelta = splitDelta.getReversed();
+
+				const { deltasA } = transformDeltaSets( [ unwrapDelta ], [ splitDelta, mergeDelta ], doc );
+
+				expect( deltasA.length ).to.equal( 2 );
+
+				expectDelta( deltasA[ 0 ], {
+					type: MoveDelta,
+					operations: [
+						{
+							type: MoveOperation,
+							sourcePosition: new Position( root, [ 0, 0 ] ),
+							howMany: 1,
+							targetPosition: new Position( root, [ 0 ] ),
+							baseVersion: baseVersion + 4
+						}
+					]
+				} );
+
+				expectDelta( deltasA[ 1 ], {
+					type: UnwrapDelta,
+					operations: [
+						{
+							type: MoveOperation,
+							sourcePosition: new Position( root, [ 1, 0 ] ),
+							howMany: 1,
+							targetPosition: new Position( root, [ 1 ] ),
+							baseVersion: baseVersion + 5
+						},
+						{
+							type: RemoveOperation,
+							sourcePosition: new Position( root, [ 2 ] ),
+							howMany: 1,
+							baseVersion: baseVersion + 6
+						}
+					]
+				} );
+			} );
+
 			it( 'should keep correct order of nodes if additional context is used (insert before)', () => {
 				// Assume document state: 'a^bcd'. User presses 'delete' key twice.
 				const removeDelta1 = getRemoveDelta( new Position( root, [ 1 ] ), 1, baseVersion ); // 'acd'.

+ 49 - 28
packages/ckeditor5-engine/tests/model/operation/transform.js

@@ -37,18 +37,18 @@ describe( 'transform', () => {
 		for ( const i in params ) {
 			if ( params.hasOwnProperty( i ) ) {
 				if ( i == 'type' ) {
-					expect( op ).to.be.instanceof( params[ i ] );
+					expect( op, 'type' ).to.be.instanceof( params[ i ] );
 				}
 				else if ( params[ i ] instanceof Array ) {
-					expect( op[ i ].length ).to.equal( params[ i ].length );
+					expect( op[ i ].length, i ).to.equal( params[ i ].length );
 
 					for ( let j = 0; j < params[ i ].length; j++ ) {
 						expect( op[ i ][ j ] ).to.equal( params[ i ][ j ] );
 					}
 				} else if ( params[ i ] instanceof Position || params[ i ] instanceof Range ) {
-					expect( op[ i ].isEqual( params[ i ] ) ).to.be.true;
+					expect( op[ i ].isEqual( params[ i ] ), i ).to.be.true;
 				} else {
-					expect( op[ i ] ).to.equal( params[ i ] );
+					expect( op[ i ], i ).to.equal( params[ i ] );
 				}
 			}
 		}
@@ -2241,7 +2241,7 @@ describe( 'transform', () => {
 				expectOperation( transOp[ 0 ], expected );
 			} );
 
-			it( 'target inside transforming move range: expand move range', () => {
+			it( 'target inside transforming move range: split into two operations', () => {
 				const transformBy = new MoveOperation(
 					new Position( root, [ 4, 1, 0 ] ),
 					2,
@@ -2251,11 +2251,15 @@ describe( 'transform', () => {
 
 				const transOp = transform( op, transformBy );
 
-				expect( transOp.length ).to.equal( 1 );
-
-				expected.howMany = 4;
+				expect( transOp.length ).to.equal( 2 );
 
+				expected.howMany = 1;
 				expectOperation( transOp[ 0 ], expected );
+
+				expected.sourcePosition.path = [ 2, 2, 6 ];
+				expected.targetPosition = targetPosition.getShiftedBy( 1 );
+				expected.baseVersion++;
+				expectOperation( transOp[ 1 ], expected );
 			} );
 
 			it( 'target at start boundary of transforming move range: increment source offset', () => {
@@ -2530,12 +2534,13 @@ describe( 'transform', () => {
 
 				expect( transOp.length ).to.equal( 2 );
 
-				expected.sourcePosition.path = [ 2, 2, 3 ];
+				expected.sourcePosition = new Position( otherRoot, [ 4, 1, 1 ] );
 				expected.howMany = 1;
 
 				expectOperation( transOp[ 0 ], expected );
 
-				expected.sourcePosition = new Position( otherRoot, [ 4, 1, 1 ] );
+				expected.sourcePosition = new Position( root, [ 2, 2, 3 ] );
+				expected.targetPosition = targetPosition.getShiftedBy( 1 );
 				expected.baseVersion++;
 
 				expectOperation( transOp[ 1 ], expected );
@@ -2569,18 +2574,19 @@ describe( 'transform', () => {
 
 				expect( transOp.length ).to.equal( 2 );
 
-				expected.sourcePosition.path = [ 4, 1, 0 ];
+				expected.sourcePosition = sourcePosition;
 				expected.howMany = 1;
 
 				expectOperation( transOp[ 0 ], expected );
 
-				expected.sourcePosition.path = op.sourcePosition.path;
+				expected.sourcePosition = new Position( root, [ 4, 1, 0 ] );
+				expected.targetPosition = targetPosition.getShiftedBy( 1 );
 				expected.baseVersion++;
 
 				expectOperation( transOp[ 1 ], expected );
 			} );
 
-			it( 'range intersects on left side, target inside transforming range and is important: expand move range', () => {
+			it( 'range intersects on left side, target inside transforming range and is important: split into two operations', () => {
 				op.howMany = 4;
 
 				const transformBy = new MoveOperation(
@@ -2592,15 +2598,22 @@ describe( 'transform', () => {
 
 				const transOp = transform( op, transformBy );
 
-				expect( transOp.length ).to.equal( 1 );
+				expect( transOp.length ).to.equal( 2 );
 
 				expected.sourcePosition.path = [ 2, 2, 3 ];
-				expected.howMany = 5;
+				expected.howMany = 1;
 
 				expectOperation( transOp[ 0 ], expected );
+
+				expected.sourcePosition.path = [ 2, 2, 5 ];
+				expected.howMany = 2;
+				expected.targetPosition = targetPosition.getShiftedBy( 1 );
+				expected.baseVersion++;
+
+				expectOperation( transOp[ 1 ], expected );
 			} );
 
-			it( 'range intersects on left side, target inside transforming range and is less important: expand move range', () => {
+			it( 'range intersects on left side, target inside transforming range and is less important: split into two operations', () => {
 				op.howMany = 4;
 
 				const transformBy = new MoveOperation(
@@ -2612,12 +2625,19 @@ describe( 'transform', () => {
 
 				const transOp = transform( op, transformBy, { isStrong: true } );
 
-				expect( transOp.length ).to.equal( 1 );
+				expect( transOp.length ).to.equal( 2 );
 
 				expected.sourcePosition.path = [ 2, 2, 3 ];
-				expected.howMany = 5;
+				expected.howMany = 1;
 
 				expectOperation( transOp[ 0 ], expected );
+
+				expected.sourcePosition.path = [ 2, 2, 5 ];
+				expected.howMany = 2;
+				expected.targetPosition = targetPosition.getShiftedBy( 1 );
+				expected.baseVersion++;
+
+				expectOperation( transOp[ 1 ], expected );
 			} );
 
 			it( 'range intersects on right side of transforming range and is important: shrink range', () => {
@@ -2648,12 +2668,13 @@ describe( 'transform', () => {
 
 				expect( transOp.length ).to.equal( 2 );
 
-				expected.sourcePosition.path = [ 4, 1, 0 ];
+				expected.sourcePosition = sourcePosition;
 				expected.howMany = 1;
 
 				expectOperation( transOp[ 0 ], expected );
 
-				expected.sourcePosition.path = op.sourcePosition.path;
+				expected.sourcePosition = new Position( root, [ 4, 1, 0 ] );
+				expected.targetPosition = targetPosition.getShiftedBy( 1 );
 				expected.baseVersion++;
 
 				expectOperation( transOp[ 1 ], expected );
@@ -2673,15 +2694,14 @@ describe( 'transform', () => {
 
 				expect( transOp.length ).to.equal( 2 );
 
-				expected.sourcePosition.path = [ 4, 1, 0 ];
+				expected.sourcePosition.path = [ 2, 2, 4 ];
 				expected.targetPosition.path = [ 4, 1, 2 ];
 				expected.howMany = 1;
 
 				expectOperation( transOp[ 0 ], expected );
 
-				expected.sourcePosition.path = [ 2, 2, 4 ];
-				expected.targetPosition.path = [ 4, 1, 2 ];
-				expected.howMany = 1;
+				expected.sourcePosition.path = [ 4, 1, 0 ];
+				expected.targetPosition.path = [ 4, 1, 3 ];
 				expected.baseVersion++;
 
 				expectOperation( transOp[ 1 ], expected );
@@ -2722,13 +2742,12 @@ describe( 'transform', () => {
 
 				expect( transOp.length ).to.equal( 2 );
 
+				expected.sourcePosition.path = [ 2, 2, 4 ];
 				expected.howMany = 1;
-				expected.sourcePosition.path = [ 2, 2, 5 ];
 
 				expectOperation( transOp[ 0 ], expected );
 
-				expected.howMany = 1;
-				expected.sourcePosition.path = [ 2, 2, 4 ];
+				expected.targetPosition = targetPosition.getShiftedBy( 1 );
 				expected.baseVersion++;
 
 				expectOperation( transOp[ 1 ], expected );
@@ -2748,18 +2767,20 @@ describe( 'transform', () => {
 
 				expect( transOp.length ).to.equal( 3 );
 
-				expected.sourcePosition.path = [ 2, 2, 5 ];
+				expected.sourcePosition.path = [ 2, 2, 4 ];
 				expected.howMany = 1;
 
 				expectOperation( transOp[ 0 ], expected );
 
 				expected.sourcePosition.path = [ 4, 1, 0 ];
+				expected.targetPosition = targetPosition.getShiftedBy( 1 );
 				expected.howMany = 2;
 				expected.baseVersion++;
 
 				expectOperation( transOp[ 1 ], expected );
 
 				expected.sourcePosition.path = [ 2, 2, 4 ];
+				expected.targetPosition = targetPosition.getShiftedBy( 3 );
 				expected.howMany = 1;
 				expected.baseVersion++;
 

+ 1 - 1
packages/ckeditor5-engine/tests/model/selection.js

@@ -468,7 +468,7 @@ describe( 'Selection', () => {
 			const startPos = Position.createAt( root, 1 );
 			const endPos = Position.createAt( root, 2 );
 			const newEndPos = Position.createAt( root, 4 );
-			const spy = testUtils.sinon.stub( Position, 'createAt', () => newEndPos );
+			const spy = testUtils.sinon.stub( Position, 'createAt' ).returns( newEndPos );
 
 			selection.addRange( new Range( startPos, endPos ) );
 

+ 134 - 0
packages/ckeditor5-engine/tests/view/document/jumpoveruielement.js

@@ -0,0 +1,134 @@
+/**
+ * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* globals document */
+
+import ViewDocument from '../../../src/view/document';
+import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
+import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';
+import { setData } from '../../../src/dev-utils/view';
+
+describe( 'Document', () => {
+	let viewDocument, domRoot;
+
+	beforeEach( () => {
+		domRoot = createElement( document, 'div', {
+			contenteditable: 'true'
+		} );
+		document.body.appendChild( domRoot );
+
+		viewDocument = new ViewDocument();
+		viewDocument.createRoot( domRoot );
+
+		document.getSelection().removeAllRanges();
+
+		viewDocument.isFocused = true;
+	} );
+
+	afterEach( () => {
+		viewDocument.destroy();
+
+		domRoot.parentElement.removeChild( domRoot );
+	} );
+
+	describe( 'jump over ui element handler', () => {
+		it( 'jump over ui element when right arrow is pressed before ui element', () => {
+			setData( viewDocument, '<container:p>foo{}<ui:span></ui:span>bar</container:p>' );
+			viewDocument.render();
+
+			viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowright, domTarget: viewDocument.domRoots.get( 'main' ) } );
+
+			const domSelection = document.getSelection();
+
+			expect( domSelection.anchorNode.nodeName.toUpperCase() ).to.equal( 'P' );
+			expect( domSelection.anchorOffset ).to.equal( 2 );
+			expect( domSelection.isCollapsed ).to.be.true;
+		} );
+
+		it( 'should do nothing when another key is pressed', () => {
+			setData( viewDocument, '<container:p>foo<ui:span></ui:span>{}bar</container:p>' );
+			viewDocument.render();
+
+			viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowleft, domTarget: viewDocument.domRoots.get( 'main' ) } );
+
+			const domSelection = document.getSelection();
+
+			expect( domSelection.anchorNode.data ).to.equal( 'bar' );
+			expect( domSelection.anchorOffset ).to.equal( 0 );
+			expect( domSelection.isCollapsed ).to.be.true;
+		} );
+
+		it( 'should do nothing if range is not collapsed', () => {
+			setData( viewDocument, '<container:p>f{oo}<ui:span></ui:span>bar</container:p>' );
+			viewDocument.render();
+
+			viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowright, domTarget: viewDocument.domRoots.get( 'main' ) } );
+
+			const domSelection = document.getSelection();
+
+			expect( domSelection.anchorNode.data ).to.equal( 'foo' );
+			expect( domSelection.anchorOffset ).to.equal( 1 );
+			expect( domSelection.focusNode.data ).to.equal( 'foo' );
+			expect( domSelection.focusOffset ).to.equal( 3 );
+			expect( domSelection.isCollapsed ).to.be.false;
+		} );
+
+		it( 'jump over ui element if selection is not collapsed but shift key is pressed', () => {
+			setData( viewDocument, '<container:p>fo{o}<ui:span></ui:span>bar</container:p>' );
+			viewDocument.render();
+
+			viewDocument.fire(
+				'keydown',
+				{ keyCode: keyCodes.arrowright, shiftKey: true, domTarget: viewDocument.domRoots.get( 'main' ) }
+			);
+
+			const domSelection = document.getSelection();
+
+			expect( domSelection.anchorNode.nodeName.toUpperCase() ).to.equal( '#TEXT' );
+			expect( domSelection.anchorOffset ).to.equal( 2 );
+			expect( domSelection.focusNode.nodeName.toUpperCase() ).to.equal( 'P' );
+			expect( domSelection.focusOffset ).to.equal( 2 );
+		} );
+
+		it( 'jump over ui element if selection is in attribute element', () => {
+			setData( viewDocument, '<container:p><attribute:b>foo{}</attribute:b><ui:span></ui:span>bar</container:p>' );
+			viewDocument.render();
+
+			viewDocument.fire(
+				'keydown',
+				{ keyCode: keyCodes.arrowright, shiftKey: true, domTarget: viewDocument.domRoots.get( 'main' ) }
+			);
+
+			const domSelection = document.getSelection();
+
+			expect( domSelection.anchorNode.nodeName.toUpperCase() ).to.equal( 'P' );
+			expect( domSelection.anchorOffset ).to.equal( 2 );
+			expect( domSelection.isCollapsed ).to.be.true;
+		} );
+
+		it( 'should do nothing if caret is not directly before ui element', () => {
+			setData( viewDocument, '<container:p>fo{}o<ui:span></ui:span>bar</container:p>' );
+			viewDocument.render();
+
+			viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowright, domTarget: viewDocument.domRoots.get( 'main' ) } );
+
+			const domSelection = document.getSelection();
+
+			expect( domSelection.anchorNode.data ).to.equal( 'foo' );
+			expect( domSelection.anchorOffset ).to.equal( 2 );
+			expect( domSelection.isCollapsed ).to.be.true;
+		} );
+
+		it( 'should do nothing if dom position cannot be converted to view position', () => {
+			const newDiv = document.createElement( 'div' );
+			const domSelection = document.getSelection();
+
+			document.body.appendChild( newDiv );
+			domSelection.collapse( newDiv, 0 );
+
+			viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowright, domTarget: viewDocument.domRoots.get( 'main' ) } );
+		} );
+	} );
+} );

+ 1 - 2
packages/ckeditor5-engine/tests/view/domconverter/binding.js

@@ -277,7 +277,7 @@ describe( 'DomConverter', () => {
 
 		it( 'should bind DOM element to selection', () => {
 			const bindSelection = converter.fakeSelectionToView( domEl );
-			expect( bindSelection ).to.be.defined;
+			expect( bindSelection ).to.not.be.undefined;
 			expect( bindSelection.isEqual( selection ) ).to.be.true;
 		} );
 
@@ -287,7 +287,6 @@ describe( 'DomConverter', () => {
 			selection.addRange( ViewRange.createIn( new ViewElement() ), true );
 			const bindSelection = converter.fakeSelectionToView( domEl );
 
-			expect( bindSelection ).to.be.defined;
 			expect( bindSelection ).to.not.equal( selection );
 			expect( bindSelection.isEqual( selection ) ).to.be.false;
 			expect( bindSelection.isEqual( selectionCopy ) ).to.be.true;

+ 103 - 1
packages/ckeditor5-engine/tests/view/domconverter/domconverter.js

@@ -8,7 +8,9 @@
 import DomConverter from '../../../src/view/domconverter';
 import ViewEditable from '../../../src/view/editableelement';
 import ViewDocument from '../../../src/view/document';
-import { BR_FILLER, NBSP_FILLER } from '../../../src/view/filler';
+import ViewUIElement from '../../../src/view/uielement';
+import ViewContainerElement from '../../../src/view/containerelement';
+import { BR_FILLER, NBSP_FILLER, INLINE_FILLER, INLINE_FILLER_LENGTH } from '../../../src/view/filler';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import global from '@ckeditor/ckeditor5-utils/src/dom/global';
 
@@ -190,4 +192,104 @@ describe( 'DomConverter', () => {
 			} );
 		} );
 	} );
+
+	describe( 'isDomSelectionCorrect()', () => {
+		function domSelection( anchorParent, anchorOffset, focusParent, focusOffset ) {
+			const sel = document.getSelection();
+
+			sel.collapse( anchorParent, anchorOffset );
+			sel.extend( focusParent, focusOffset );
+
+			return sel;
+		}
+
+		let domP, domFillerTextNode, domUiSpan, domUiDeepSpan;
+
+		beforeEach( () => {
+			// <p>INLINE_FILLERfoo<span></span></p>.
+			domP = document.createElement( 'p' );
+			domFillerTextNode = document.createTextNode( INLINE_FILLER + 'foo' );
+			domUiSpan = document.createElement( 'span' );
+
+			domUiDeepSpan = document.createElement( 'span' );
+			domUiSpan.appendChild( domUiDeepSpan );
+
+			const viewUiSpan = new ViewUIElement( 'span' );
+			const viewElementSpan = new ViewContainerElement( 'span' );
+
+			domP.appendChild( domFillerTextNode );
+			domP.appendChild( domUiSpan );
+
+			converter.bindElements( domUiSpan, viewUiSpan );
+			converter.bindElements( domUiDeepSpan, viewElementSpan );
+
+			document.body.appendChild( domP );
+		} );
+
+		it( 'should return true for correct dom selection', () => {
+			// <p>INLINE_FILLER{foo}<span></span></p>.
+			const sel1 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
+			expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.true;
+
+			// <p>INLINE_FILLERfoo[]<span></span></p>.
+			const sel2 = domSelection( domP, 1, domP, 1 );
+			expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.true;
+
+			// <p>INLINE_FILLERfoo<span></span>[]</p>.
+			const sel3 = domSelection( domP, 2, domP, 2 );
+			expect( converter.isDomSelectionCorrect( sel3 ) ).to.be.true;
+		} );
+
+		describe( 'should return false', () => {
+			it( 'if anchor or focus is before filler node', () => {
+				// Tests forward and backward selection.
+				// <p>[INLINE_FILLERfoo]<span-ui><span-container></span></span></p>.
+				const sel1 = domSelection( domP, 0, domP, 1 );
+				expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.false;
+
+				const sel2 = domSelection( domP, 1, domP, 0 );
+				expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.false;
+			} );
+
+			it( 'if anchor or focus is before filler sequence', () => {
+				// Tests forward and backward selection.
+				// <p>{INLINE_FILLERfoo}<span-ui><span-container></span></span></p>.
+				const sel1 = domSelection( domFillerTextNode, 0, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
+				expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.false;
+
+				const sel2 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH + 3, domFillerTextNode, 0 );
+				expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.false;
+			} );
+
+			it( 'if anchor or focus is in the middle of filler sequence', () => {
+				// Tests forward and backward selection.
+				// <p>I{NLINE_FILLERfoo}<span-ui><span-container></span></span></p>.
+				const sel1 = domSelection( domFillerTextNode, 1, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
+				expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.false;
+
+				const sel2 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH + 3, domFillerTextNode, 1 );
+				expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.false;
+			} );
+
+			it( 'if anchor or focus is directly inside dom element that represents view ui element', () => {
+				// Tests forward and backward selection.
+				// <p>INLINE_FILLER{foo<span-ui>]<span-container></span></span></p>.
+				const sel1 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH + 3, domUiSpan, 0 );
+				expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.false;
+
+				const sel2 = domSelection( domUiSpan, 0, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
+				expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.false;
+			} );
+
+			it( 'if anchor or focus is inside deep ui element structure (not directly in ui element)', () => {
+				// Tests forward and backward selection.
+				// <p>INLINE_FILLER{foo<span-ui><span-container>]</span></span></p>.
+				const sel1 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH + 3, domUiDeepSpan, 0 );
+				expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.false;
+
+				const sel2 = domSelection( domUiDeepSpan, 0, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
+				expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.false;
+			} );
+		} );
+	} );
 } );

+ 4 - 4
packages/ckeditor5-engine/tests/view/domconverter/uielement.js

@@ -41,14 +41,14 @@ describe( 'DOMConverter UIElement integration', () => {
 			expect( domElement.innerHTML ).to.equal( '<p><span>foo</span> bar</p>' );
 		} );
 
-		it( 'should bind only UIElement not child elements', () => {
+		it( 'should create DOM structure that all is mapped to single UIElement', () => {
 			const myElement = new MyUIElement( 'div' );
 			const domElement = converter.viewToDom( myElement, document, { bind: true } );
-			const domSpan = domElement.childNodes[ 0 ];
+			const domParagraph = domElement.childNodes[ 0 ];
 
 			expect( converter.mapDomToView( domElement ) ).to.equal( myElement );
-			expect( converter.mapDomToView( domSpan ) ).to.be.falsy;
-			expect( converter.mapDomToView( domSpan.childNodes[ 0 ] ) ).to.be.falsy;
+			expect( converter.mapDomToView( domParagraph ) ).to.equal( myElement );
+			expect( converter.mapDomToView( domParagraph.childNodes[ 0 ] ) ).to.equal( myElement );
 		} );
 	} );
 

Failā izmaiņas netiks attēlotas, jo tās ir par lielu
+ 8 - 3
packages/ckeditor5-engine/tests/view/manual/uielement.html


+ 16 - 7
packages/ckeditor5-engine/tests/view/manual/uielement.js

@@ -3,21 +3,22 @@
  * For licensing, see LICENSE.md.
  */
 
-/* global document */
+/* globals console, window, document */
 
 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 Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
+import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic';
 import Undo from '@ckeditor/ckeditor5-undo/src/undo';
-import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import UIElement from '../../../src/view/uielement';
 
 class MyUIElement extends UIElement {
 	render( domDocument ) {
 		const root = super.render( domDocument );
 
-		root.setAttribute( 'contenteditable', 'false' );
 		root.classList.add( 'ui-element' );
 		root.innerHTML = 'END OF PARAGRAPH';
 
@@ -38,7 +39,15 @@ class UIElementTestPlugin extends Plugin {
 	}
 }
 
-ClassicEditor.create( document.querySelector( '#editor' ), {
-	plugins: [ Enter, Typing, Paragraph, Undo, UIElementTestPlugin ],
-	toolbar: [ 'undo', 'redo' ]
-} );
+ClassicEditor
+	.create( document.querySelector( '#editor' ), {
+		plugins: [ Enter, Typing, Paragraph, Undo, Bold, Italic, UIElementTestPlugin ],
+		toolbar: [ 'undo', 'redo', 'bold', 'italic' ]
+	} )
+	.then( editor => {
+		window.editor = editor;
+	} )
+	.catch( err => {
+		console.error( err.stack );
+	} );
+

+ 2 - 0
packages/ckeditor5-engine/tests/view/manual/uielement.md

@@ -3,3 +3,5 @@
 1. Each paragraph should have UIElement at it's bottom showing "END OF PARAGRAPH".
 1. UIElement should not block typing or prevent regular editor usage.
 1. When paragraph is split or new paragraph is created - new UIElement should be created too.
+1. You should not be able to place selection inside ui element or type in it.
+1. Arrow keys should work correctly around ui element.

+ 15 - 0
packages/ckeditor5-engine/tests/view/node.js

@@ -279,6 +279,21 @@ describe( 'Node', () => {
 		} );
 	} );
 
+	describe( 'toJSON()', () => {
+		it( 'should prevent circular reference when stringifying a node', () => {
+			const char = new Text( 'a' );
+			const parent = new Element( 'p', null );
+			parent.appendChildren( char );
+
+			const json = JSON.stringify( char );
+			const parsed = JSON.parse( json );
+
+			expect( parsed ).to.deep.equal( {
+				_data: 'a'
+			} );
+		} );
+	} );
+
 	describe( 'change event', () => {
 		let root, text, img, rootChangeSpy;
 

+ 34 - 0
packages/ckeditor5-engine/tests/view/observer/mutationobserver.js

@@ -91,6 +91,40 @@ describe( 'MutationObserver', () => {
 		expect( lastMutations[ 0 ].oldChildren[ 0 ].data ).to.equal( 'foo' );
 	} );
 
+	it( 'should handle unbold', () => {
+		viewRoot.removeChildren( 0, viewRoot.childCount );
+		viewRoot.appendChildren( parse( '<container:p><attribute:b>foo</attribute:b></container:p>' ) );
+		viewDocument.render();
+
+		const domP = domEditor.childNodes[ 0 ];
+		const domB = domP.childNodes[ 0 ];
+		domP.removeChild( domB );
+		domP.appendChild( document.createTextNode( 'foo' ) );
+
+		mutationObserver.flush();
+
+		// "expectDomEditorNotToChange()".
+		expect( domEditor.childNodes.length ).to.equal( 1 );
+		expect( domEditor.childNodes[ 0 ].tagName ).to.equal( 'P' );
+
+		expect( domEditor.childNodes[ 0 ].childNodes.length ).to.equal( 1 );
+		expect( domEditor.childNodes[ 0 ].childNodes[ 0 ].tagName ).to.equal( 'B' );
+
+		expect( domEditor.childNodes[ 0 ].childNodes[ 0 ].childNodes.length ).to.equal( 1 );
+		expect( domEditor.childNodes[ 0 ].childNodes[ 0 ].childNodes[ 0 ].data ).to.equal( 'foo' );
+
+		// Check mutations.
+		expect( lastMutations.length ).to.equal( 1 );
+		expect( lastMutations[ 0 ].type ).to.equal( 'children' );
+		expect( lastMutations[ 0 ].node ).to.equal( viewRoot.getChild( 0 ) );
+
+		expect( lastMutations[ 0 ].newChildren.length ).to.equal( 1 );
+		expect( lastMutations[ 0 ].newChildren[ 0 ].data ).to.equal( 'foo' );
+
+		expect( lastMutations[ 0 ].oldChildren.length ).to.equal( 1 );
+		expect( lastMutations[ 0 ].oldChildren[ 0 ].name ).to.equal( 'b' );
+	} );
+
 	it( 'should deduplicate text changes', () => {
 		domEditor.childNodes[ 0 ].childNodes[ 0 ].data = 'foox';
 		domEditor.childNodes[ 0 ].childNodes[ 0 ].data = 'fooxy';

+ 47 - 6
packages/ckeditor5-engine/tests/view/observer/selectionobserver.js

@@ -34,7 +34,7 @@ describe( 'SelectionObserver', () => {
 		viewRoot = viewDocument.getRoot();
 
 		viewRoot.appendChildren( parse(
-			'<container:p>xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx</container:p>' +
+			'<container:p>xxx<ui:span></ui:span></container:p>' +
 			'<container:p>yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy</container:p>' ) );
 
 		viewDocument.render();
@@ -67,7 +67,7 @@ describe( 'SelectionObserver', () => {
 			expect( data.newSelection.rangeCount ).to.equal( 1 );
 
 			const newViewRange = data.newSelection.getFirstRange();
-			const viewFoo = viewDocument.getRoot().getChild( 0 ).getChild( 0 );
+			const viewFoo = viewDocument.getRoot().getChild( 1 ).getChild( 0 );
 
 			expect( newViewRange.start.parent ).to.equal( viewFoo );
 			expect( newViewRange.start.offset ).to.equal( 2 );
@@ -162,7 +162,7 @@ describe( 'SelectionObserver', () => {
 		viewDocument.selection.addRange( ViewRange.createFromParentsAndOffsets( viewFoo, 0, viewFoo, 0 ) );
 
 		return new Promise( ( resolve, reject ) => {
-			testUtils.sinon.stub( log, 'warn', msg => {
+			testUtils.sinon.stub( log, 'warn' ).callsFake( msg => {
 				expect( msg ).to.match( /^selectionchange-infinite-loop/ );
 
 				resolve();
@@ -197,7 +197,9 @@ describe( 'SelectionObserver', () => {
 	} );
 
 	it( 'should not be treated as an infinite loop if changes are not often', () => {
-		const clock = testUtils.sinon.useFakeTimers( 'setInterval', 'clearInterval' );
+		const clock = testUtils.sinon.useFakeTimers( {
+			toFake: [ 'setInterval', 'clearInterval' ]
+		} );
 		const stub = testUtils.sinon.stub( log, 'warn' );
 
 		// We need to recreate SelectionObserver, so it will use mocked setInterval.
@@ -264,7 +266,7 @@ describe( 'SelectionObserver', () => {
 				expect( data.newSelection.rangeCount ).to.equal( 1 );
 
 				const newViewRange = data.newSelection.getFirstRange();
-				const viewFoo = viewDocument.getRoot().getChild( 0 ).getChild( 0 );
+				const viewFoo = viewDocument.getRoot().getChild( 1 ).getChild( 0 );
 
 				expect( newViewRange.start.parent ).to.equal( viewFoo );
 				expect( newViewRange.start.offset ).to.equal( 3 );
@@ -301,9 +303,48 @@ describe( 'SelectionObserver', () => {
 		}, 100 );
 	} );
 
+	it( 'should re-render view if selections are similar if DOM selection is in incorrect place', done => {
+		const sel = domDocument.getSelection();
+
+		// Add rendering on selectionChange event to check this feature.
+		viewDocument.on( 'selectionChange', () => {
+			// Manually set selection because no handlers are set for selectionChange event in this test.
+			// Normally this is handled by view -> model -> view selection converters chain.
+			const viewSel = viewDocument.selection;
+
+			const viewAnchor = viewDocument.domConverter.domPositionToView( sel.anchorNode, sel.anchorOffset );
+			const viewFocus = viewDocument.domConverter.domPositionToView( sel.focusNode, sel.focusOffset );
+
+			viewSel.collapse( viewAnchor );
+			viewSel.setFocus( viewFocus );
+
+			viewDocument.render();
+		} );
+
+		viewDocument.once( 'selectionChange', () => {
+			// 2. Selection change has been handled.
+
+			selectionObserver.listenTo( domDocument, 'selectionchange', () => {
+				// 4. Check if view was re-rendered.
+				expect( viewDocument.render.called ).to.be.true;
+
+				done();
+			}, { priority: 'lowest' } );
+
+			// 3. Now, collapse selection in similar position, but in UI element.
+			// Current and new selection position are similar in view (but not equal!).
+			// Also add a spy to `viewDocument#render` to see if view will be re-rendered.
+			sel.collapse( domMain.childNodes[ 0 ].childNodes[ 1 ], 0 );
+			sinon.spy( viewDocument, 'render' );
+		}, { priority: 'lowest' } );
+
+		// 1. Collapse in a text node, before ui element, and wait for async selectionchange to fire selection change handling.
+		sel.collapse( domMain.childNodes[ 0 ].childNodes[ 0 ], 3 );
+	} );
+
 	function changeDomSelection() {
 		const domSelection = domDocument.getSelection();
-		const domFoo = domMain.childNodes[ 0 ].childNodes[ 0 ];
+		const domFoo = domMain.childNodes[ 1 ].childNodes[ 0 ];
 		const offset = domSelection.anchorOffset;
 
 		domSelection.removeAllRanges();

+ 28 - 2
packages/ckeditor5-engine/tests/view/range.js

@@ -162,16 +162,42 @@ describe( 'Range', () => {
 				.to.equal( '<p><b>f</b>{oo}<b><span></span>bar</b></p>' );
 		} );
 
-		it( 'case6', () => {
+		it( 'case 6', () => {
 			expect( trim( '<p>foo[</p><p>bar</p><p>]bom</p>' ) )
 				.to.equal( '<p>foo[</p><p>bar</p><p>]bom</p>' );
 		} );
 
-		it( 'case7', () => {
+		it( 'case 7', () => {
 			expect( trim( '<p>foo[<b><img></img></b>]bom</p>' ) )
 				.to.equal( '<p>foo<b>[<img></img>]</b>bom</p>' );
 		} );
 
+		// Other results may theoretically be correct too. It is not decided whether the trimmed range should
+		// be collapsed in attribute element, at its start or its end. This is one of possible correct results
+		// and we won't know for sure unless we have more cases. See #1058.
+		it( 'case 8', () => {
+			expect( trim( '<p>[<b></b>]</p>' ) )
+				.to.equal( '<p><b></b>[]</p>' );
+		} );
+
+		// As above.
+		it( 'case 9', () => {
+			expect( trim( '<p><b></b>[<b></b>]<b></b></p>' ) )
+				.to.equal( '<p><b></b><b></b><b></b>[]</p>' );
+		} );
+
+		// As above.
+		it( 'case 10', () => {
+			expect( trim( '<p>[<b></b><b></b>]</p>' ) )
+				.to.equal( '<p><b></b><b></b>[]</p>' );
+		} );
+
+		// As above.
+		it( 'case 11', () => {
+			expect( trim( '<p><b></b><b>[]</b><b></b></p>' ) )
+				.to.equal( '<p><b></b><b></b><b></b>[]</p>' );
+		} );
+
 		function trim( data ) {
 			data = data
 				.replace( /<p>/g, '<container:p>' )

+ 30 - 1
packages/ckeditor5-engine/tests/view/renderer.js

@@ -1438,7 +1438,7 @@ describe( 'Renderer', () => {
 				const container = domRoot.childNodes[ 1 ];
 
 				const bindSelection = renderer.domConverter.fakeSelectionToView( container );
-				expect( bindSelection ).to.be.defined;
+				expect( bindSelection ).to.not.be.undefined;
 				expect( bindSelection.isEqual( selection ) ).to.be.true;
 			} );
 
@@ -1604,6 +1604,35 @@ describe( 'Renderer', () => {
 				expect( logWarnStub.notCalled ).to.true;
 			} );
 
+			it( 'should always render selection (even if it is same in view) if current dom selection is in incorrect place', () => {
+				const domSelection = document.getSelection();
+
+				const { view: viewP, selection: newSelection } = parse( '<container:p>foo[]<ui:span></ui:span></container:p>' );
+
+				viewRoot.appendChildren( viewP );
+				selection.setTo( newSelection );
+
+				renderer.markToSync( 'children', viewRoot );
+				renderer.render();
+
+				// In DOM, set position to: <p>foo<span>[]</span></p>. This is incorrect DOM selection (it is in view ui element).
+				// Do not change view selection.
+				// When renderer will check if the DOM selection changed, it will convert DOM selection to a view selection.
+				// Selections (current view selection and view-from-dom selection) will be equal but we will still expect re-render
+				// because DOM selection is in incorrect place.
+				const domP = domRoot.childNodes[ 0 ];
+				const domSpan = domP.childNodes[ 1 ];
+				domSelection.collapse( domSpan, 0 );
+
+				renderer.render();
+
+				// Expect that after calling `renderer.render()` the DOM selection was re-rendered (and set at correct position).
+				expect( domSelection.anchorNode ).to.equal( domP );
+				expect( domSelection.anchorOffset ).to.equal( 1 );
+				expect( domSelection.focusNode ).to.equal( domP );
+				expect( domSelection.focusOffset ).to.equal( 1 );
+			} );
+
 			it( 'should not render non-collapsed selection it is similar (element start)', () => {
 				const domSelection = document.getSelection();
 

+ 90 - 6
packages/ckeditor5-engine/tests/view/selection.js

@@ -17,11 +17,14 @@ describe( 'Selection', () => {
 	let selection, el, range1, range2, range3;
 
 	beforeEach( () => {
+		const text = new Text( 'xxxxxxxxxxxxxxxxxxxx' );
+		el = new Element( 'p', null, text );
+
 		selection = new Selection();
-		el = new Element( 'p' );
-		range1 = Range.createFromParentsAndOffsets( el, 5, el, 10 );
-		range2 = Range.createFromParentsAndOffsets( el, 1, el, 2 );
-		range3 = Range.createFromParentsAndOffsets( el, 12, el, 14 );
+
+		range1 = Range.createFromParentsAndOffsets( text, 5, text, 10 );
+		range2 = Range.createFromParentsAndOffsets( text, 1, text, 2 );
+		range3 = Range.createFromParentsAndOffsets( text, 12, text, 14 );
 	} );
 
 	describe( 'constructor()', () => {
@@ -241,7 +244,7 @@ describe( 'Selection', () => {
 			const endPos = Position.createAt( el, 2 );
 			const newEndPos = Position.createAt( el, 4 );
 
-			const spy = sinon.stub( Position, 'createAt', () => newEndPos );
+			const spy = sinon.stub( Position, 'createAt' ).returns( newEndPos );
 
 			selection.addRange( new Range( startPos, endPos ) );
 			selection.setFocus( el, 'end' );
@@ -335,7 +338,8 @@ describe( 'Selection', () => {
 		} );
 
 		it( 'should throw when range is intersecting with already added range', () => {
-			const range2 = Range.createFromParentsAndOffsets( el, 7, el, 15 );
+			const text = el.getChild( 0 );
+			const range2 = Range.createFromParentsAndOffsets( text, 7, text, 15 );
 			selection.addRange( range1 );
 			expect( () => {
 				selection.addRange( range2 );
@@ -516,6 +520,86 @@ describe( 'Selection', () => {
 		} );
 	} );
 
+	describe( 'isSimilar', () => {
+		it( 'should return true if selections equal', () => {
+			selection.addRange( range1 );
+			selection.addRange( range2 );
+
+			const otherSelection = new Selection();
+			otherSelection.addRange( range1 );
+			otherSelection.addRange( range2 );
+
+			expect( selection.isSimilar( otherSelection ) ).to.be.true;
+		} );
+
+		it( 'should return false if ranges count does not equal', () => {
+			selection.addRange( range1 );
+			selection.addRange( range2 );
+
+			const otherSelection = new Selection();
+			otherSelection.addRange( range1 );
+
+			expect( selection.isSimilar( otherSelection ) ).to.be.false;
+		} );
+
+		it( 'should return false if trimmed ranges (other than the last added one) are not equal', () => {
+			selection.addRange( range1 );
+			selection.addRange( range3 );
+
+			const otherSelection = new Selection();
+			otherSelection.addRange( range2 );
+			otherSelection.addRange( range3 );
+
+			expect( selection.isSimilar( otherSelection ) ).to.be.false;
+		} );
+
+		it( 'should return false if directions are not equal', () => {
+			selection.addRange( range1 );
+
+			const otherSelection = new Selection();
+			otherSelection.addRange( range1, true );
+
+			expect( selection.isSimilar( otherSelection ) ).to.be.false;
+		} );
+
+		it( 'should return true if both selections are empty', () => {
+			const otherSelection = new Selection();
+
+			expect( selection.isSimilar( otherSelection ) ).to.be.true;
+		} );
+
+		it( 'should return true if all ranges trimmed from both selections are equal', () => {
+			const view = parse(
+				'<container:p><attribute:span></attribute:span></container:p>' +
+				'<container:p><attribute:span>xx</attribute:span></container:p>'
+			);
+
+			const p1 = view.getChild( 0 );
+			const p2 = view.getChild( 1 );
+			const span1 = p1.getChild( 0 );
+			const span2 = p2.getChild( 0 );
+
+			// <p>[<span>{]</span>}</p><p>[<span>{xx}</span>]</p>
+			const rangeA1 = Range.createFromParentsAndOffsets( p1, 0, span1, 0 );
+			const rangeB1 = Range.createFromParentsAndOffsets( span1, 0, p1, 1 );
+			const rangeA2 = Range.createFromParentsAndOffsets( p2, 0, p2, 1 );
+			const rangeB2 = Range.createFromParentsAndOffsets( span2, 0, span2, 1 );
+
+			selection.addRange( rangeA1 );
+			selection.addRange( rangeA2 );
+
+			const otherSelection = new Selection();
+			otherSelection.addRange( rangeB2 );
+			otherSelection.addRange( rangeB1 );
+
+			expect( selection.isSimilar( otherSelection ) ).to.be.true;
+			expect( otherSelection.isSimilar( selection ) ).to.be.true;
+
+			expect( selection.isEqual( otherSelection ) ).to.be.false;
+			expect( otherSelection.isEqual( selection ) ).to.be.false;
+		} );
+	} );
+
 	describe( 'removeAllRanges', () => {
 		it( 'should remove all ranges and fire change event', done => {
 			selection.addRange( range1 );