Преглед на файлове

Remove TwoStepCaretHandler class,

as the multi-attribute caret handling
needs to be orchestrated by  TwoStepCaretMovement class.
Avoid exposing implementation details into API for no specific reason.
Tomek Wytrębowicz преди 5 години
родител
ревизия
ede14650b6
променени са 2 файла, в които са добавени 377 реда и са изтрити 349 реда
  1. 285 340
      packages/ckeditor5-typing/src/twostepcaretmovement.js
  2. 92 9
      packages/ckeditor5-typing/tests/twostepcaretmovement.js

+ 285 - 340
packages/ckeditor5-typing/src/twostepcaretmovement.js

@@ -99,12 +99,21 @@ export default class TwoStepCaretMovement extends Plugin {
 		super( editor );
 
 		/**
-		 * A map of handlers for each attribute.
+		 * A set of attributes to handle.
 		 *
 		 * @protected
 		 * @property {module:typing/twostepcaretmovement~TwoStepCaretMovement}
 		 */
-		this._handlers = new Map();
+		this.attributes = new Set();
+
+		/**
+		 * The current UID of the overridden gravity, as returned by
+		 * {@link module:engine/model/writer~Writer#overrideSelectionGravity}.
+		 *
+		 * @private
+		 * @member {String}
+		 */
+		this._overrideUid = null;
 	}
 
 	/**
@@ -153,13 +162,21 @@ export default class TwoStepCaretMovement extends Plugin {
 			const contentDirection = locale.contentLanguageDirection;
 			let isMovementHandled = false;
 
+			let orientedHandlerMethod;
 			if ( ( contentDirection === 'ltr' && arrowRightPressed ) || ( contentDirection === 'rtl' && arrowLeftPressed ) ) {
-				for ( const [ , handler ] of this._handlers ) {
-					isMovementHandled = isMovementHandled || handler.handleForwardMovement( position, data );
-				}
+				orientedHandlerMethod = handleForwardMovement;
 			} else {
-				for ( const [ , handler ] of this._handlers ) {
-					isMovementHandled = isMovementHandled || handler.handleBackwardMovement( position, data );
+				orientedHandlerMethod = handleBackwardMovement;
+			}
+
+			for ( const attribute of this.attributes ) {
+				// If gravity was changed for at least one attribute, move on.
+				const result = orientedHandlerMethod( position, data, this, attribute );
+				if ( result === true ) {
+					isMovementHandled = true;
+					break;
+				} else if ( result === 'next' ) {
+					break;
 				}
 			}
 
@@ -169,70 +186,6 @@ export default class TwoStepCaretMovement extends Plugin {
 				evt.stop();
 			}
 		}, { priority: priorities.get( 'high' ) + 1 } );
-	}
-
-	/**
-	 * Registers a given attribute for the two-step caret movement.
-	 *
-	 * @param {String} attribute Name of the attribute to handle.
-	 */
-	registerAttribute( attribute ) {
-		this._handlers.set(
-			attribute,
-			new TwoStepCaretHandler( this.editor.model, this, attribute )
-		);
-	}
-}
-
-/**
- * This is a protected helper–class for {@link module:typing/twostepcaretmovement}.
- * It handles the state of the 2-step caret movement for a single {@link module:engine/model/model~Model}
- * attribute upon the `keypress` in the {@link module:engine/view/view~View}.
- *
- * @protected
- */
-export class TwoStepCaretHandler {
-	/*
-	 * Creates two step handler instance.
-	 *
-	 * @param {module:engine/model/model~Model} model Data model instance.
-	 * @param {module:utils/dom/emittermixin~Emitter} emitter The emitter to which this behavior should be added
-	 * (e.g. a plugin instance).
-	 * @param {String} attribute Attribute for which the behavior will be added.
-	 */
-	constructor( model, emitter, attribute ) {
-		/**
-		 * The model instance this class instance operates on.
-		 *
-		 * @readonly
-		 * @member {module:engine/model/model~Model#schema}
-		 */
-		this.model = model;
-
-		/**
-		 * The Attribute this class instance operates on.
-		 *
-		 * @readonly
-		 * @member {String}
-		 */
-		this.attribute = attribute;
-
-		/**
-		 * A reference to the document selection.
-		 *
-		 * @private
-		 * @member {module:engine/model/selection~Selection}
-		 */
-		this._modelSelection = model.document.selection;
-
-		/**
-		 * The current UID of the overridden gravity, as returned by
-		 * {@link module:engine/model/writer~Writer#overrideSelectionGravity}.
-		 *
-		 * @private
-		 * @member {String}
-		 */
-		this._overrideUid = null;
 
 		/**
 		 * A flag indicating that the automatic gravity restoration for this attribute
@@ -245,7 +198,7 @@ export class TwoStepCaretHandler {
 		this._isNextGravityRestorationSkipped = false;
 
 		// The automatic gravity restoration logic.
-		emitter.listenTo( this._modelSelection, 'change:range', ( evt, data ) => {
+		this.listenTo( modelSelection, 'change:range', ( evt, data ) => {
 			// Skipping the automatic restoration is needed if the selection should change
 			// but the gravity must remain overridden afterwards. See the #handleBackwardMovement
 			// to learn more.
@@ -264,7 +217,10 @@ export class TwoStepCaretHandler {
 			// Skip automatic restore when the change is indirect AND the selection is at the attribute boundary.
 			// It means that e.g. if the change was external (collaboration) and the user had their
 			// selection around the link, its gravity should remain intact in this change:range event.
-			if ( !data.directChange && isAtBoundary( this._modelSelection.getFirstPosition(), attribute ) ) {
+			const isAnyAtBoundary = Array.from( this.attributes )
+				.some( attribute => isAtBoundary( modelSelection.getFirstPosition(), attribute ) );
+
+			if ( !data.directChange && isAnyAtBoundary ) {
 				return;
 			}
 
@@ -273,321 +229,310 @@ export class TwoStepCaretHandler {
 	}
 
 	/**
-	 * Updates the document selection and the view according to the two–step caret movement state
-	 * when moving **forwards**. Executed upon `keypress` in the {@link module:engine/view/view~View}.
+	 * Registers a given attribute for the two-step caret movement.
 	 *
-	 * @param {module:engine/model/position~Position} position The model position at the moment of the key press.
-	 * @param {module:engine/view/observer/domeventdata~DomEventData} data Data of the key press.
-	 * @returns {Boolean} `true` when the handler prevented caret movement
+	 * @param {String} attribute Name of the attribute to handle.
 	 */
-	handleForwardMovement( position, data ) {
-		const attribute = this.attribute;
+	registerAttribute( attribute ) {
+		this.attributes.add( attribute );
+	}
 
-		// DON'T ENGAGE 2-SCM if gravity is already overridden. It means that we just entered
-		//
-		// 		<paragraph>foo<$text attribute>{}bar</$text>baz</paragraph>
-		//
-		// or left the attribute
-		//
-		// 		<paragraph>foo<$text attribute>bar</$text>{}baz</paragraph>
-		//
-		// and the gravity will be restored automatically.
-		if ( this._isGravityOverridden ) {
-			return;
-		}
+	/**
+	 * `true` when the gravity is overridden for the {@link #attribute}.
+	 *
+	 * @readonly
+	 * @private
+	 * @type {Boolean}
+	 */
+	get _isGravityOverridden() {
+		return !!this._overrideUid;
+	}
 
-		// DON'T ENGAGE 2-SCM when the selection is at the beginning of the block AND already has the
-		// attribute:
-		// * when the selection was initially set there using the mouse,
-		// * when the editor has just started
-		//
-		//		<paragraph><$text attribute>{}bar</$text>baz</paragraph>
-		//
-		if ( position.isAtStart && this._hasSelectionAttribute ) {
-			return;
-		}
+	/**
+	 * Overrides the gravity using the {@link module:engine/model/writer~Writer model writer}
+	 * and stores the information about this fact in the {@link #_overrideUid}.
+	 *
+	 * A shorthand for {@link module:engine/model/writer~Writer#overrideSelectionGravity}.
+	 *
+	 * @private
+	 */
+	_overrideGravity() {
+		this._overrideUid = this.editor.model.change( writer => {
+			return writer.overrideSelectionGravity();
+		} );
+	}
 
-		// ENGAGE 2-SCM when about to leave one attribute value and enter another:
+	/**
+	 * Restores the gravity using the {@link module:engine/model/writer~Writer model writer}.
+	 *
+	 * A shorthand for {@link module:engine/model/writer~Writer#restoreSelectionGravity}.
+	 *
+	 * @private
+	 */
+	_restoreGravity() {
+		this.editor.model.change( writer => {
+			writer.restoreSelectionGravity( this._overrideUid );
+			this._overrideUid = null;
+		} );
+	}
+}
+
+/**
+ * Updates the document selection and the view according to the two–step caret movement state
+ * when moving **forwards**. Executed upon `keypress` in the {@link module:engine/view/view~View}.
+ *
+ * @param {module:engine/model/position~Position} position The model position at the moment of the key press.
+ * @param {module:engine/view/observer/domeventdata~DomEventData} data Data of the key press.
+ * @param {module:typing/src/twostepcaretmovement~TwoStepCaretMovement} plugin 2SCM plugin.
+ * @param {String} attribute. The attribute to handle.
+ * @returns {Boolean} `true` when the handler prevented caret movement
+ */
+function handleForwardMovement( position, data, plugin, attribute ) {
+	const model = plugin.editor.model;
+	// DON'T ENGAGE 2-SCM if gravity is already overridden. It means that we just entered
+	//
+	// 		<paragraph>foo<$text attribute>{}bar</$text>baz</paragraph>
+	//
+	// or left the attribute
+	//
+	// 		<paragraph>foo<$text attribute>bar</$text>{}baz</paragraph>
+	//
+	// and the gravity will be restored automatically.
+	if ( plugin._isGravityOverridden ) {
+		return;
+	}
+
+	const hasSelectionAttribute = model.document.selection.hasAttribute( attribute );
+
+	// DON'T ENGAGE 2-SCM when the selection is at the beginning of the block AND already has the
+	// attribute:
+	// * when the selection was initially set there using the mouse,
+	// * when the editor has just started
+	//
+	//		<paragraph><$text attribute>{}bar</$text>baz</paragraph>
+	//
+	if ( position.isAtStart && hasSelectionAttribute ) {
+		return;
+	}
+
+	// ENGAGE 2-SCM when about to leave one attribute value and enter another:
+	//
+	// 		<paragraph><$text attribute="1">foo{}</$text><$text attribute="2">bar</$text></paragraph>
+	//
+	// but DON'T when already in between of them (no attribute selection):
+	//
+	// 		<paragraph><$text attribute="1">foo</$text>{}<$text attribute="2">bar</$text></paragraph>
+	//
+	if ( isBetweenDifferentValues( position, attribute ) && hasSelectionAttribute ) {
+		preventCaretMovement( data );
+		removeSelectionAttribute( model, attribute );
+
+		return true;
+	}
+
+	// ENGAGE 2-SCM when entering an attribute:
+	//
+	// 		<paragraph>foo{}<$text attribute>bar</$text>baz</paragraph>
+	//
+	if ( isAtStartBoundary( position, attribute ) ) {
+		preventCaretMovement( data );
+		plugin._overrideGravity();
+
+		return true;
+	}
+
+	// ENGAGE 2-SCM when leaving an attribute:
+	//
+	//		<paragraph>foo<$text attribute>bar{}</$text>baz</paragraph>
+	//
+	if ( isAtEndBoundary( position, attribute ) && hasSelectionAttribute ) {
+		preventCaretMovement( data );
+		plugin._overrideGravity();
+
+		return true;
+	}
+}
+
+/**
+ * Updates the document selection and the view according to the two–step caret movement state
+ * when moving **backwards**. Executed upon `keypress` in the {@link module:engine/view/view~View}.
+ *
+ * @param {module:engine/model/position~Position} position The model position at the moment of the key press.
+ * @param {module:engine/view/observer/domeventdata~DomEventData} data Data of the key press.
+ * @param {module:typing/src/twostepcaretmovement~TwoStepCaretMovement} plugin 2SCM plugin.
+ * @param {String} attribute. The attribute to handle.
+ * @returns {Boolean|String} `true` when the handler prevented caret movement,
+ * 							`'next'` when the next gravity restoration should be skipped.
+ */
+function handleBackwardMovement( position, data, plugin, attribute ) {
+	const model = plugin.editor.model;
+	const hasSelectionAttribute = model.document.selection.hasAttribute( attribute );
+	// When the gravity is already overridden...
+	if ( plugin._isGravityOverridden ) {
+		// ENGAGE 2-SCM & REMOVE SELECTION ATTRIBUTE
+		// when about to leave one attribute value and enter another:
 		//
-		// 		<paragraph><$text attribute="1">foo{}</$text><$text attribute="2">bar</$text></paragraph>
+		// 		<paragraph><$text attribute="1">foo</$text><$text attribute="2">{}bar</$text></paragraph>
 		//
 		// but DON'T when already in between of them (no attribute selection):
 		//
 		// 		<paragraph><$text attribute="1">foo</$text>{}<$text attribute="2">bar</$text></paragraph>
 		//
-		if ( isBetweenDifferentValues( position, attribute ) && this._hasSelectionAttribute ) {
-			this._preventCaretMovement( data );
-			this._removeSelectionAttribute();
+		if ( isBetweenDifferentValues( position, attribute ) && hasSelectionAttribute ) {
+			preventCaretMovement( data );
+			plugin._restoreGravity();
+			removeSelectionAttribute( plugin.editor.model, attribute );
 
 			return true;
 		}
 
-		// ENGAGE 2-SCM when entering an attribute:
+		// ENGAGE 2-SCM when at any boundary of the attribute:
 		//
-		// 		<paragraph>foo{}<$text attribute>bar</$text>baz</paragraph>
+		// 		<paragraph>foo<$text attribute>bar</$text>{}baz</paragraph>
+		// 		<paragraph>foo<$text attribute>{}bar</$text>baz</paragraph>
 		//
-		if ( isAtStartBoundary( position, attribute ) ) {
-			this._preventCaretMovement( data );
-			this._overrideGravity();
+		else {
+			preventCaretMovement( data );
+			plugin._restoreGravity();
+
+			// REMOVE SELECTION ATRIBUTE at the beginning of the block.
+			// It's like restoring gravity but towards a non-existent content when
+			// the gravity is overridden:
+			//
+			// 		<paragraph><$text attribute>{}bar</$text></paragraph>
+			//
+			// becomes:
+			//
+			// 		<paragraph>{}<$text attribute>bar</$text></paragraph>
+			//
+			if ( position.isAtStart ) {
+				removeSelectionAttribute( plugin.editor.model, attribute );
+			}
 
 			return true;
 		}
-
-		// ENGAGE 2-SCM when leaving an attribute:
+	} else {
+		// ENGAGE 2-SCM when between two different attribute values but selection has no attribute:
 		//
-		//		<paragraph>foo<$text attribute>bar{}</$text>baz</paragraph>
+		// 		<paragraph><$text attribute="1">foo</$text>{}<$text attribute="2">bar</$text></paragraph>
 		//
-		if ( isAtEndBoundary( position, attribute ) && this._hasSelectionAttribute ) {
-			this._preventCaretMovement( data );
-			this._overrideGravity();
+		if ( isBetweenDifferentValues( position, attribute ) && !hasSelectionAttribute ) {
+			preventCaretMovement( data );
+			setSelectionAttributeFromTheNodeBefore( plugin.editor.model, attribute, position );
 
 			return true;
 		}
-	}
 
-	/**
-	 * Updates the document selection and the view according to the two–step caret movement state
-	 * when moving **backwards**. Executed upon `keypress` in the {@link module:engine/view/view~View}.
-	 *
-	 * @param {module:engine/model/position~Position} position The model position at the moment of the key press.
-	 * @param {module:engine/view/observer/domeventdata~DomEventData} data Data of the key press.
-	 * @returns {Boolean} `true` when the handler prevented caret movement
-	 */
-	handleBackwardMovement( position, data ) {
-		const attribute = this.attribute;
-
-		// When the gravity is already overridden...
-		if ( this._isGravityOverridden ) {
-			// ENGAGE 2-SCM & REMOVE SELECTION ATTRIBUTE
-			// when about to leave one attribute value and enter another:
-			//
-			// 		<paragraph><$text attribute="1">foo</$text><$text attribute="2">{}bar</$text></paragraph>
-			//
-			// but DON'T when already in between of them (no attribute selection):
-			//
-			// 		<paragraph><$text attribute="1">foo</$text>{}<$text attribute="2">bar</$text></paragraph>
-			//
-			if ( isBetweenDifferentValues( position, attribute ) && this._hasSelectionAttribute ) {
-				this._preventCaretMovement( data );
-				this._restoreGravity();
-				this._removeSelectionAttribute();
-
-				return true;
-			}
-
-			// ENGAGE 2-SCM when at any boundary of the attribute:
+		// End of block boundary cases:
+		//
+		// 		<paragraph><$text attribute>bar{}</$text></paragraph>
+		// 		<paragraph><$text attribute>bar</$text>{}</paragraph>
+		//
+		if ( position.isAtEnd && isAtEndBoundary( position, attribute ) ) {
+			// DON'T ENGAGE 2-SCM if the selection has the attribute already.
+			// This is a common selection if set using the mouse.
 			//
-			// 		<paragraph>foo<$text attribute>bar</$text>{}baz</paragraph>
-			// 		<paragraph>foo<$text attribute>{}bar</$text>baz</paragraph>
+			// 		<paragraph><$text attribute>bar{}</$text></paragraph>
 			//
-			else {
-				this._preventCaretMovement( data );
-				this._restoreGravity();
-
-				// REMOVE SELECTION ATRIBUTE at the beginning of the block.
-				// It's like restoring gravity but towards a non-existent content when
-				// the gravity is overridden:
+			if ( hasSelectionAttribute ) {
+				// DON'T ENGAGE 2-SCM if the attribute at the end of the block which has length == 1.
+				// Make sure the selection will not the attribute after it moves backwards.
 				//
-				// 		<paragraph><$text attribute>{}bar</$text></paragraph>
+				// 		<paragraph>foo<$text attribute>b{}</$text></paragraph>
 				//
-				// becomes:
-				//
-				// 		<paragraph>{}<$text attribute>bar</$text></paragraph>
-				//
-				if ( position.isAtStart ) {
-					this._removeSelectionAttribute();
+				if ( isStepAfterTheAttributeBoundary( position, attribute ) ) {
+					// Skip the automatic gravity restore upon the next selection#change:range event.
+					// If not skipped, it would automatically restore the gravity, which should remain
+					// overridden.
+					plugin._isNextGravityRestorationSkipped = true;
+					plugin._overrideGravity();
+
+					// Don't return "true" here because we didn't call _preventCaretMovement.
+					// Returning here will destabilize the filler logic, which also listens to
+					// keydown (and the event would be stopped).
+					return 'next';
 				}
 
-				return true;
-			}
-		} else {
-			// ENGAGE 2-SCM when between two different attribute values but selection has no attribute:
-			//
-			// 		<paragraph><$text attribute="1">foo</$text>{}<$text attribute="2">bar</$text></paragraph>
-			//
-			if ( isBetweenDifferentValues( position, attribute ) && !this._hasSelectionAttribute ) {
-				this._preventCaretMovement( data );
-				this._setSelectionAttributeFromTheNodeBefore( position );
-
-				return true;
+				return;
 			}
-
-			// End of block boundary cases:
+			// ENGAGE 2-SCM if the selection has no attribute. This may happen when the user
+			// left the attribute using a FORWARD 2-SCM.
 			//
-			// 		<paragraph><$text attribute>bar{}</$text></paragraph>
 			// 		<paragraph><$text attribute>bar</$text>{}</paragraph>
 			//
-			if ( position.isAtEnd && isAtEndBoundary( position, attribute ) ) {
-				// DON'T ENGAGE 2-SCM if the selection has the attribute already.
-				// This is a common selection if set using the mouse.
-				//
-				// 		<paragraph><$text attribute>bar{}</$text></paragraph>
-				//
-				if ( this._hasSelectionAttribute ) {
-					// DON'T ENGAGE 2-SCM if the attribute at the end of the block which has length == 1.
-					// Make sure the selection will not the attribute after it moves backwards.
-					//
-					// 		<paragraph>foo<$text attribute>b{}</$text></paragraph>
-					//
-					if ( isStepAfterTheAttributeBoundary( position, attribute ) ) {
-						// Skip the automatic gravity restore upon the next selection#change:range event.
-						// If not skipped, it would automatically restore the gravity, which should remain
-						// overridden.
-						this._skipNextAutomaticGravityRestoration();
-						this._overrideGravity();
-
-						// Don't return "true" here because we didn't call _preventCaretMovement.
-						// Returning here will destabilize the filler logic, which also listens to
-						// keydown (and the event would be stopped).
-					}
-
-					return;
-				}
-				// ENGAGE 2-SCM if the selection has no attribute. This may happen when the user
-				// left the attribute using a FORWARD 2-SCM.
-				//
-				// 		<paragraph><$text attribute>bar</$text>{}</paragraph>
-				//
-				else {
-					this._preventCaretMovement( data );
-					this._setSelectionAttributeFromTheNodeBefore( position );
+			else {
+				preventCaretMovement( data );
+				setSelectionAttributeFromTheNodeBefore( plugin.editor.model, attribute, position );
 
-					return true;
-				}
+				return true;
 			}
+		}
 
-			// REMOVE SELECTION ATRIBUTE when restoring gravity towards a non-existent content at the
-			// beginning of the block.
-			//
-			// 		<paragraph>{}<$text attribute>bar</$text></paragraph>
-			//
-			if ( position.isAtStart ) {
-				if ( this._hasSelectionAttribute ) {
-					this._removeSelectionAttribute();
-					this._preventCaretMovement( data );
-
-					return true;
-				}
+		// REMOVE SELECTION ATRIBUTE when restoring gravity towards a non-existent content at the
+		// beginning of the block.
+		//
+		// 		<paragraph>{}<$text attribute>bar</$text></paragraph>
+		//
+		if ( position.isAtStart ) {
+			if ( hasSelectionAttribute ) {
+				removeSelectionAttribute( model, attribute );
+				preventCaretMovement( data );
 
-				return;
+				return true;
 			}
 
-			// DON'T ENGAGE 2-SCM when about to enter of leave an attribute.
-			// We need to check if the caret is a one position before the attribute boundary:
-			//
-			// 		<paragraph>foo<$text attribute>b{}ar</$text>baz</paragraph>
-			// 		<paragraph>foo<$text attribute>bar</$text>b{}az</paragraph>
-			//
-			if ( isStepAfterTheAttributeBoundary( position, attribute ) ) {
-				// Skip the automatic gravity restore upon the next selection#change:range event.
-				// If not skipped, it would automatically restore the gravity, which should remain
-				// overridden.
-				this._skipNextAutomaticGravityRestoration();
-				this._overrideGravity();
-
-				// Don't return "true" here because we didn't call _preventCaretMovement.
-				// Returning here will destabilize the filler logic, which also listens to
-				// keydown (and the event would be stopped).
-			}
+			return;
 		}
-	}
 
-	/**
-	 * `true` when the gravity is overridden for the {@link #attribute}.
-	 *
-	 * @readonly
-	 * @private
-	 * @type {Boolean}
-	 */
-	get _isGravityOverridden() {
-		return !!this._overrideUid;
-	}
-
-	/**
-	 * `true` when the {@link module:engine/model/selection~Selection} has the {@link #attribute}.
-	 *
-	 * @readonly
-	 * @private
-	 * @type {Boolean}
-	 */
-	get _hasSelectionAttribute() {
-		return this._modelSelection.hasAttribute( this.attribute );
-	}
-
-	/**
-	 * Overrides the gravity using the {@link module:engine/model/writer~Writer model writer}
-	 * and stores the information about this fact in the {@link #_overrideUid}.
-	 *
-	 * A shorthand for {@link module:engine/model/writer~Writer#overrideSelectionGravity}.
-	 *
-	 * @private
-	 */
-	_overrideGravity() {
-		this._overrideUid = this.model.change( writer => writer.overrideSelectionGravity() );
-	}
-
-	/**
-	 * Restores the gravity using the {@link module:engine/model/writer~Writer model writer}.
-	 *
-	 * A shorthand for {@link module:engine/model/writer~Writer#restoreSelectionGravity}.
-	 *
-	 * @private
-	 */
-	_restoreGravity() {
-		this.model.change( writer => {
-			writer.restoreSelectionGravity( this._overrideUid );
-			this._overrideUid = null;
-		} );
-	}
-
-	/**
-	 * Prevents the caret movement in the view by calling `preventDefault` on the event data.
-	 *
-	 * @private
-	 */
-	_preventCaretMovement( data ) {
-		data.preventDefault();
-	}
-
-	/**
-	 * Removes the {@link #attribute} from the selection using using the
-	 * {@link module:engine/model/writer~Writer model writer}.
-	 *
-	 * @private
-	 */
-	_removeSelectionAttribute() {
-		this.model.change( writer => {
-			writer.removeSelectionAttribute( this.attribute );
-		} );
+		// DON'T ENGAGE 2-SCM when about to enter of leave an attribute.
+		// We need to check if the caret is a one position before the attribute boundary:
+		//
+		// 		<paragraph>foo<$text attribute>b{}ar</$text>baz</paragraph>
+		// 		<paragraph>foo<$text attribute>bar</$text>b{}az</paragraph>
+		//
+		if ( isStepAfterTheAttributeBoundary( position, attribute ) ) {
+			// Skip the automatic gravity restore upon the next selection#change:range event.
+			// If not skipped, it would automatically restore the gravity, which should remain
+			// overridden.
+			plugin._isNextGravityRestorationSkipped = true;
+			plugin._overrideGravity();
+
+			// Don't return "true" here because we didn't call _preventCaretMovement.
+			// Returning here will destabilize the filler logic, which also listens to
+			// keydown (and the event would be stopped).
+			return 'next';
+		}
 	}
+}
 
-	/**
-	 * Applies the {@link #attribute} to the current selection using using the
-	 * value from the node before the current position. Uses
-	 * the {@link module:engine/model/writer~Writer model writer}.
-	 *
-	 * @private
-	 * @param {module:engine/model/position~Position} position
-	 */
-	_setSelectionAttributeFromTheNodeBefore( position ) {
-		const attribute = this.attribute;
+// Applies the {@link #attribute} to the current selection using using the
+// value from the node before the current position. Uses
+// the {@link module:engine/model/writer~Writer model writer}.
+//
+// @param {module:engine/model/model~Model}
+// @param {String} attribute
+// @param {module:engine/model/position~Position} position
+function setSelectionAttributeFromTheNodeBefore( model, attribute, position ) {
+	model.change( writer => {
+		writer.setSelectionAttribute( attribute, position.nodeBefore.getAttribute( attribute ) );
+	} );
+}
 
-		this.model.change( writer => {
-			writer.setSelectionAttribute( this.attribute, position.nodeBefore.getAttribute( attribute ) );
-		} );
-	}
+// Removes the {@link #attribute} from the selection using using the
+// {@link module:engine/model/writer~Writer model writer}.
+// @param {module:engine/model/model~Model}
+// @param {String} attribute
+function removeSelectionAttribute( model, attribute ) {
+	model.change( writer => {
+		writer.removeSelectionAttribute( attribute );
+	} );
+}
 
-	/**
-	 * Skips the next automatic selection gravity restoration upon the
-	 * {@link module:engine/model/selection~Selection#event:change:range} event.
-	 *
-	 * See {@link #_isNextGravityRestorationSkipped}.
-	 *
-	 * @private
-	 */
-	_skipNextAutomaticGravityRestoration() {
-		this._isNextGravityRestorationSkipped = true;
-	}
+// Prevents the caret movement in the view by calling `preventDefault` on the event data.
+//
+// @alias data.preventDefault
+function preventCaretMovement( data ) {
+	data.preventDefault();
 }
 
 // @param {module:engine/model/position~Position} position

+ 92 - 9
packages/ckeditor5-typing/tests/twostepcaretmovement.js

@@ -141,16 +141,20 @@ describe( 'TwoStepCaretMovement()', () => {
 
 		// https://github.com/ckeditor/ckeditor5-engine/issues/1301
 		it( 'should handle passing through the only character in the block', () => {
-			setData( model, '[]<$text a="1">x</$text>' );
+			setData( model, '<$text a="1">[]x</$text>' );
 
 			testTwoStepCaretMovement( [
-				{ selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 0, evtStop: 0 },
+				// <$text a="1">[]x</$text>
+				{ selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: 0 },
 				'→',
-				{ selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 0, evtStop: 0 },
+				// <$text a="1">x[]</$text>
+				{ selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: 1 },
 				'→',
-				{ selectionAttributes: [], isGravityOverridden: true, preventDefault: 1, evtStop: 1 },
+				// <$text a="1">x</$text>[]
+				{ selectionAttributes: [], isGravityOverridden: true, preventDefault: 1, evtStop: 1, caretPosition: 1 },
 				'→',
-				{ selectionAttributes: [], isGravityOverridden: true, preventDefault: 1, evtStop: 1 }
+				// Stays at <$text a="1">x</$text>[]
+				{ selectionAttributes: [], isGravityOverridden: true, preventDefault: 1, evtStop: 1, caretPosition: 1 }
 			] );
 		} );
 
@@ -640,6 +644,82 @@ describe( 'TwoStepCaretMovement()', () => {
 				{ selectionAttributes: [], isGravityOverridden: false, preventDefault: 4, evtStop: 4 }
 			] );
 		} );
+
+		describe( 'when two elements ends at the same position', () => {
+			it( 'moving the caret in should take 2 steps', () => {
+				setData( model, 'foo<$text a="true">foo</$text><$text a="true" c="true">bar</$text>q[]ux' );
+
+				testTwoStepCaretMovement( [
+					// foo<$text a="true">foo</$text><$text a="true" c="true">bar</$text>q[]ux
+					{ selectionAttributes: [], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: 10 },
+					'←',
+					// foo<$text a="true">foo</$text><$text a="true" c="true">bar</$text>[]qux
+					{ selectionAttributes: [], isGravityOverridden: true, preventDefault: 0, evtStop: 0, caretPosition: 9 },
+					'←',
+					// foo<$text a="true">foo</$text><$text a="true" c="true">bar[]</$text>qux
+					{ selectionAttributes: [ 'a', 'c' ], isGravityOverridden: false, preventDefault: 1, evtStop: 1, caretPosition: 9 },
+					'←',
+					// foo<$text a="true">foo</$text><$text a="true" c="true">ba[]r</$text>qux
+					{ selectionAttributes: [ 'a', 'c' ], isGravityOverridden: false, preventDefault: 1, evtStop: 1, caretPosition: 8 }
+				] );
+			} );
+
+			it( 'moving the caret out should take 2 steps', () => {
+				setData( model, 'foo<$text a="true">foo</$text><$text a="true" c="true">ba[]r</$text>qux' );
+
+				testTwoStepCaretMovement( [
+					// foo<$text a="true">foo</$text><$text a="true" c="true">ba[]r</$text>qux
+					{ selectionAttributes: [ 'a', 'c' ], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: 8 },
+					'→',
+					// foo<$text a="true">foo</$text><$text a="true" c="true">bar[]</$text>qux
+					{ selectionAttributes: [ 'a', 'c' ], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: 9 },
+					'→',
+					// foo<$text a="true">foo</$text><$text a="true" c="true">bar</$text>[]qux
+					{ selectionAttributes: [], isGravityOverridden: true, preventDefault: 1, evtStop: 1, caretPosition: 9 },
+					'→',
+					// foo<$text a="true">foo</$text><$text a="true" c="true">bar</$text>q[]ux
+					{ selectionAttributes: [], isGravityOverridden: false, preventDefault: 1, evtStop: 1, caretPosition: 10 }
+				] );
+			} );
+		} );
+
+		describe( 'when two elements starts at the same position', () => {
+			it( 'moving the caret in should take 2 steps', () => {
+				setData( model, 'fo[]o<$text a="true" c="true">bar</$text><$text a="true">baz</$text>qux' );
+
+				testTwoStepCaretMovement( [
+					// fo[]o<$text a="true" c="true">bar</$text><$text a="true">baz</$text>qux
+					{ selectionAttributes: [], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: 2 },
+					'→',
+					// foo[]<$text a="true" c="true">bar</$text><$text a="true">baz</$text>qux
+					{ selectionAttributes: [], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: 3 },
+					'→',
+					// foo<$text a="true" c="true">[]bar</$text><$text a="true">baz</$text>qux
+					{ selectionAttributes: [ 'a', 'c' ], isGravityOverridden: true, preventDefault: 1, evtStop: 1, caretPosition: 3 },
+					'→',
+					// foo<$text a="true" c="true">b[]ar</$text><$text a="true">baz</$text>qux
+					{ selectionAttributes: [ 'a', 'c' ], isGravityOverridden: false, preventDefault: 1, evtStop: 1, caretPosition: 4 }
+				] );
+			} );
+
+			it( 'moving the caret out should take 2 steps', () => {
+				setData( model, 'foo<$text a="true" c="true">b[]ar</$text><$text a="true">baz</$text>qux' );
+
+				testTwoStepCaretMovement( [
+					// foo<$text a="true" c="true">b[]ar</$text><$text a="true">baz</$text>qux
+					{ selectionAttributes: [ 'a', 'c' ], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: 4 },
+					'←',
+					// foo<$text a="true" c="true">[]bar</$text><$text a="true">baz</$text>qux
+					{ selectionAttributes: [ 'a', 'c' ], isGravityOverridden: true, preventDefault: 0, evtStop: 0, caretPosition: 3 },
+					'←',
+					// foo[]<$text a="true" c="true">bar</$text><$text a="true">baz</$text>qux
+					{ selectionAttributes: [], isGravityOverridden: false, preventDefault: 1, evtStop: 1, caretPosition: 3 },
+					'←',
+					// fo[]o<$text a="true" c="true">bar</$text><$text a="true">baz</$text>qux
+					{ selectionAttributes: [], isGravityOverridden: false, preventDefault: 1, evtStop: 1, caretPosition: 2 }
+				] );
+			} );
+		} );
 	} );
 
 	describe( 'mouse', () => {
@@ -854,7 +934,6 @@ describe( 'TwoStepCaretMovement()', () => {
 				// An arrow key pressed. Fire the view event and update the model selection.
 				if ( keyMap[ step ] ) {
 					let preventDefaultCalled;
-
 					fireKeyDownEvent( {
 						keyCode: keyCodes[ keyMap[ step ] ],
 						preventDefault: () => {
@@ -908,9 +987,13 @@ describe( 'TwoStepCaretMovement()', () => {
 				const stepIndex = scenario.indexOf( step );
 				const stepString = `in step #${ stepIndex } ${ JSON.stringify( step ) }`;
 
-				expect( getSelectionAttributesArray( selection ) ).to.have.members(
-					step.selectionAttributes, `#attributes ${ stepString }` );
-				expect( selection, `#isGravityOverridden ${ stepString }` )
+				if ( step.caretPosition !== undefined ) {
+					expect( selection.getFirstPosition(), `in step #${ stepIndex }, selection's first position` )
+						.to.have.deep.property( 'path', [ step.caretPosition ] );
+				}
+				expect( getSelectionAttributesArray( selection ), `in step #${ stepIndex }, selection's gravity` )
+					.to.have.members( step.selectionAttributes, `#attributes ${ stepString }` );
+				expect( selection, `in step #${ stepIndex }, selection's gravity` )
 					.to.have.property( 'isGravityOverridden', step.isGravityOverridden );
 				expect( preventDefaultSpy.callCount ).to.equal( step.preventDefault, `#preventDefault ${ stepString }` );
 				expect( evtStopSpy.callCount ).to.equal( step.evtStop, `#evtStop ${ stepString }` );