Sfoglia il codice sorgente

Merge branch 'master' into t/1667

Piotrek Koszuliński 6 anni fa
parent
commit
153245505d

+ 3 - 6
packages/ckeditor5-engine/src/conversion/downcastdispatcher.js

@@ -532,7 +532,6 @@ export default class DowncastDispatcher {
 	 * @param {String} data.attributeKey Attribute key.
 	 * @param {*} data.attributeOldValue Attribute value before the change. This is `null` when selection attribute is converted.
 	 * @param {*} data.attributeNewValue New attribute value.
-	 * @param {module:engine/conversion/modelconsumable~ModelConsumable} consumable Values to consume.
 	 * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi Conversion interface
 	 * to be used by callback, passed in `DowncastDispatcher` constructor.
 	 */
@@ -542,7 +541,6 @@ export default class DowncastDispatcher {
 	 *
 	 * @event selection
 	 * @param {module:engine/model/selection~Selection} selection Selection that is converted.
-	 * @param {module:engine/conversion/modelconsumable~ModelConsumable} consumable Values to consume.
 	 * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi Conversion interface
 	 * to be used by callback, passed in `DowncastDispatcher` constructor.
 	 */
@@ -558,17 +556,17 @@ export default class DowncastDispatcher {
 	 * If the marker range is not collapsed:
 	 *
 	 * * the event is fired for each item in the marker range one by one,
-	 * * consumables object includes each item of the marker range and the consumable value is same as event name.
+	 * * `conversionApi.consumable` includes each item of the marker range and the consumable value is same as event name.
 	 *
 	 * If the marker range is collapsed:
 	 *
 	 * * there is only one event,
-	 * * consumables object includes marker range with event name.
+	 * * `conversionApi.consumable` includes marker range with event name.
 	 *
 	 * If selection inside a marker is converted:
 	 *
 	 * * there is only one event,
-	 * * consumables object includes selection instance with event name.
+	 * * `conversionApi.consumable` includes selection instance with event name.
 	 *
 	 * @event addMarker
 	 * @param {Object} data Additional information about the change.
@@ -578,7 +576,6 @@ export default class DowncastDispatcher {
 	 * the marker range was not collapsed.
 	 * @param {module:engine/model/range~Range} data.markerRange Marker range.
 	 * @param {String} data.markerName Marker name.
-	 * @param {module:engine/conversion/modelconsumable~ModelConsumable} consumable Values to consume.
 	 * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi Conversion interface
 	 * to be used by callback, passed in `DowncastDispatcher` constructor.
 	 */

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

@@ -19,7 +19,7 @@ import isIterable from '@ckeditor/ckeditor5-utils/src/isiterable';
  *
  * **Important**: see {@link module:engine/model/node~Node} to read about restrictions using `Element` and `Node` API.
  *
- * @extends {module:engine/model/node~Node}
+ * @extends module:engine/model/node~Node
  */
 export default class Element extends Node {
 	/**

+ 4 - 4
packages/ckeditor5-engine/src/model/operation/mergeoperation.js

@@ -141,16 +141,16 @@ export default class MergeOperation extends Operation {
 		const targetElement = this.targetPosition.parent;
 
 		// Validate whether merge operation has correct parameters.
-		if ( !sourceElement || !sourceElement.is( 'element' ) || !sourceElement.parent ) {
+		if ( !sourceElement.parent ) {
 			/**
-			 * Merge source position is invalid.
+			 * Merge source position is invalid. The element to be merged must have a parent node.
 			 *
 			 * @error merge-operation-source-position-invalid
 			 */
 			throw new CKEditorError( 'merge-operation-source-position-invalid: Merge source position is invalid.', this );
-		} else if ( !targetElement || !targetElement.is( 'element' ) || !targetElement.parent ) {
+		} else if ( !targetElement.parent ) {
 			/**
-			 * Merge target position is invalid.
+			 * Merge target position is invalid. The element to be merged must have a parent node.
 			 *
 			 * @error merge-operation-target-position-invalid
 			 */

+ 1 - 10
packages/ckeditor5-engine/src/model/operation/moveoperation.js

@@ -123,16 +123,7 @@ export default class MoveOperation extends Operation {
 		// Validate whether move operation has correct parameters.
 		// Validation is pretty complex but move operation is one of the core ways to manipulate the document state.
 		// We expect that many errors might be connected with one of scenarios described below.
-		if ( !sourceElement || !targetElement ) {
-			/**
-			 * Source position or target position is invalid.
-			 *
-			 * @error move-operation-position-invalid
-			 */
-			throw new CKEditorError(
-				'move-operation-position-invalid: Source position or target position is invalid.', this
-			);
-		} else if ( sourceOffset + this.howMany > sourceElement.maxOffset ) {
+		if ( sourceOffset + this.howMany > sourceElement.maxOffset ) {
 			/**
 			 * The nodes which should be moved do not exist.
 			 *

+ 26 - 3
packages/ckeditor5-engine/src/model/position.js

@@ -71,11 +71,11 @@ export default class Position {
 			/**
 			 * Position path must be an array with at least one item.
 			 *
-			 * @error model-position-path-incorrect
+			 * @error model-position-path-incorrect-format
 			 * @param path
 			 */
 			throw new CKEditorError(
-				'model-position-path-incorrect: Position path must be an array with at least one item.',
+				'model-position-path-incorrect-format: Position path must be an array with at least one item.',
 				root,
 				{ path }
 			);
@@ -161,13 +161,36 @@ export default class Position {
 	 * Also it is a good idea to cache `parent` property if it is used frequently in an algorithm (i.e. in a long loop).
 	 *
 	 * @readonly
-	 * @type {module:engine/model/element~Element}
+	 * @type {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment}
 	 */
 	get parent() {
 		let parent = this.root;
 
 		for ( let i = 0; i < this.path.length - 1; i++ ) {
 			parent = parent.getChild( parent.offsetToIndex( this.path[ i ] ) );
+
+			if ( !parent ) {
+				throw new CKEditorError( 'model-position-path-incorrect: The position\'s path is incorrect.', this, { position: this } );
+			}
+		}
+
+		if ( parent.is( 'text' ) ) {
+			/**
+			 * The position's path is incorrect. This means that a position does not point to
+			 * a correct place in the tree and hence, some of its methods and getters cannot work correctly.
+			 *
+			 * **Note**: Unlike DOM and view positions, in the model, the
+			 * {@link module:engine/model/position~Position#parent position's parent} is always an element or a document fragment.
+			 * The last offset in the {@link module:engine/model/position~Position#path position's path} is the point in this element where
+			 * this position points.
+			 *
+			 * Read more about model positions and offsets in
+			 * the {@glink framework/guides/architecture/editing-engine#indexes-and-offsets Editing engine architecture guide}.
+			 *
+			 * @error position-incorrect-path
+			 * @param {module:engine/model/position~Position} position The incorrect position.
+			 */
+			throw new CKEditorError( 'model-position-path-incorrect: The position\'s path is incorrect.', this, { position: this } );
 		}
 
 		return parent;

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

@@ -20,7 +20,7 @@ import Node from './node';
  * this behavior, keeping references to `Text` is not recommended. Instead, consider creating
  * {@link module:engine/model/liveposition~LivePosition live position} placed before the text node.
  *
- * @extends {module:engine/model/node~Node}
+ * @extends module:engine/model/node~Node
  */
 export default class Text extends Node {
 	/**

+ 17 - 11
packages/ckeditor5-engine/src/utils/bindtwostepcarettoattribute.js

@@ -11,12 +11,15 @@ import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
 import priorities from '@ckeditor/ckeditor5-utils/src/priorities';
 
 /**
- * This helper enabled the two-step caret (phantom) movement behavior for the given {@link module:engine/model/model~Model}
+ * This helper enables the two-step caret (phantom) movement behavior for the given {@link module:engine/model/model~Model}
  * attribute on arrow right (<kbd>→</kbd>) and left (<kbd>←</kbd>) key press.
  *
  * Thanks to this (phantom) caret movement the user is able to type before/after as well as at the
  * beginning/end of an attribute.
  *
+ * **Note:** This helper support right–to–left (Arabic, Hebrew, etc.) content by mirroring its behavior
+ * but for the sake of simplicity examples showcase only left–to–right use–cases.
+ *
  * # Forward movement
  *
  * ## "Entering" an attribute:
@@ -78,13 +81,15 @@ import priorities from '@ckeditor/ckeditor5-utils/src/priorities';
  *
  *   		<$text a="true">ba{}r</$text>b{}az
  *
- * @param {module:engine/view/view~View} view View controller 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
+ * @param {Object} options Helper options.
+ * @param {module:engine/view/view~View} options.view View controller instance.
+ * @param {module:engine/model/model~Model} options.model Data model instance.
+ * @param {module:utils/dom/emittermixin~Emitter} options.emitter The emitter to which this behavior should be added
  * (e.g. a plugin instance).
- * @param {String} attribute Attribute for which this behavior will be added.
+ * @param {String} options.attribute Attribute for which this behavior will be added.
+ * @param {module:utils/locale~Locale} options.locale The {@link module:core/editor/editor~Editor#locale} instance.
  */
-export default function bindTwoStepCaretToAttribute( view, model, emitter, attribute ) {
+export default function bindTwoStepCaretToAttribute( { view, model, emitter, attribute, locale } ) {
 	const twoStepCaretHandler = new TwoStepCaretHandler( model, emitter, attribute );
 	const modelSelection = model.document.selection;
 
@@ -120,15 +125,16 @@ export default function bindTwoStepCaretToAttribute( view, model, emitter, attri
 		}
 
 		const position = modelSelection.getFirstPosition();
+		const contentDirection = locale.contentLanguageDirection;
 		let isMovementHandled;
 
-		if ( arrowRightPressed ) {
+		if ( ( contentDirection === 'ltr' && arrowRightPressed ) || ( contentDirection === 'rtl' && arrowLeftPressed ) ) {
 			isMovementHandled = twoStepCaretHandler.handleForwardMovement( position, data );
 		} else {
 			isMovementHandled = twoStepCaretHandler.handleBackwardMovement( position, data );
 		}
 
-		// Stop the keydown event if the two-step arent movement handled it. Avoid collisions
+		// Stop the keydown event if the two-step caret movement handled it. Avoid collisions
 		// with other features which may also take over the caret movement (e.g. Widget).
 		if ( isMovementHandled ) {
 			evt.stop();
@@ -137,13 +143,13 @@ export default function bindTwoStepCaretToAttribute( view, model, emitter, attri
 }
 
 /**
- * This is a private helper–class for {@link module:engine/utils/bindtwostepcarettoattribute}.
+ * This is a protected helper–class for {@link module:engine/utils/bindtwostepcarettoattribute}.
  * 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}.
  *
- * @private
+ * @protected
  */
-class TwoStepCaretHandler {
+export class TwoStepCaretHandler {
 	/*
 	 * Creates two step handler instance.
 	 *

+ 1 - 1
packages/ckeditor5-engine/src/view/observer/fakeselectionobserver.js

@@ -19,7 +19,7 @@ import { debounce } from 'lodash-es';
  * Fires {@link module:engine/view/document~Document#event:selectionChange selectionChange event} simulating natural behaviour of
  * {@link module:engine/view/observer/selectionobserver~SelectionObserver SelectionObserver}.
  *
- * @extends module:engine/view/observer/observer~Observer.Observer
+ * @extends module:engine/view/observer/observer~Observer
  */
 export default class FakeSelectionObserver extends Observer {
 	/**

+ 7 - 1
packages/ckeditor5-engine/tests/manual/tickets/1301/1.js

@@ -21,7 +21,13 @@ ClassicEditor
 	.then( editor => {
 		const bold = editor.plugins.get( Bold );
 
-		bindTwoStepCaretToAttribute( editor.editing.view, editor.model, bold, 'bold' );
+		bindTwoStepCaretToAttribute( {
+			view: editor.editing.view,
+			model: editor.model,
+			emitter: bold,
+			attribute: 'bold',
+			locale: editor.locale
+		} );
 	} )
 	.catch( err => {
 		console.error( err.stack );

+ 21 - 1
packages/ckeditor5-engine/tests/manual/two-step-caret.html

@@ -1,5 +1,25 @@
-<div id="editor">
+<h2>Left-to–right content</h2>
+
+<div id="editor-ltr">
 	<p>Foo <u>bar</u> biz</p>
 	<p>Foo <u>bar</u><i>biz</i> buz?</p>
 	<p>Foo <b>bar</b> biz</p>
 </div>
+
+<h2>Right–to–left content</h2>
+
+<div id="editor-rtl">
+	<p>&nbsp;היא <u>תכונה</u> של</p>
+	<p>&nbsp;זהה <i>לזה</i><u>שיוצג</u> בתוצאה</p>
+	<p>וכדומה. <strong>תכונה</strong> זו מאפיינת</p>
+</div>
+
+<style>
+	u {
+		background: rgba(255,0,0,.3);
+	}
+
+	i {
+		background: rgba(0,255,0,.3);
+	}
+</style>

+ 46 - 3
packages/ckeditor5-engine/tests/manual/two-step-caret.js

@@ -15,7 +15,7 @@ import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic';
 import bindTwoStepCaretToAttribute from '../../src/utils/bindtwostepcarettoattribute';
 
 ClassicEditor
-	.create( document.querySelector( '#editor' ), {
+	.create( document.querySelector( '#editor-ltr' ), {
 		plugins: [ Essentials, Paragraph, Underline, Bold, Italic ],
 		toolbar: [ 'undo', 'redo', '|', 'bold', 'underline', 'italic' ]
 	} )
@@ -23,8 +23,51 @@ ClassicEditor
 		const bold = editor.plugins.get( Italic );
 		const underline = editor.plugins.get( Underline );
 
-		bindTwoStepCaretToAttribute( editor.editing.view, editor.model, bold, 'italic' );
-		bindTwoStepCaretToAttribute( editor.editing.view, editor.model, underline, 'underline' );
+		bindTwoStepCaretToAttribute( {
+			view: editor.editing.view,
+			model: editor.model,
+			emitter: bold,
+			attribute: 'italic',
+			locale: editor.locale
+		} );
+		bindTwoStepCaretToAttribute( {
+			view: editor.editing.view,
+			model: editor.model,
+			emitter: underline,
+			attribute: 'underline',
+			locale: editor.locale
+		} );
+	} )
+	.catch( err => {
+		console.error( err.stack );
+	} );
+
+ClassicEditor
+	.create( document.querySelector( '#editor-rtl' ), {
+		language: {
+			content: 'he'
+		},
+		plugins: [ Essentials, Paragraph, Underline, Bold, Italic ],
+		toolbar: [ 'undo', 'redo', '|', 'bold', 'underline', 'italic' ]
+	} )
+	.then( editor => {
+		const bold = editor.plugins.get( Italic );
+		const underline = editor.plugins.get( Underline );
+
+		bindTwoStepCaretToAttribute( {
+			view: editor.editing.view,
+			model: editor.model,
+			emitter: bold,
+			attribute: 'italic',
+			locale: editor.locale
+		} );
+		bindTwoStepCaretToAttribute( {
+			view: editor.editing.view,
+			model: editor.model,
+			emitter: underline,
+			attribute: 'underline',
+			locale: editor.locale
+		} );
 	} )
 	.catch( err => {
 		console.error( err.stack );

+ 7 - 0
packages/ckeditor5-engine/tests/manual/two-step-caret.md

@@ -47,3 +47,10 @@
 
 ### Not bounded attribute
 Just make sure that two-steps caret movement is disabled for bold text from the third paragraph.
+
+### Right–to–left content
+
+**Tip**: Change the system keyboard to Hebrew before testing.
+
+Two-steps caret movement should also work when the content is right–to–left. Repeat all previous steps keeping in mind that the flow of the text is "reversed".
+

+ 19 - 2
packages/ckeditor5-engine/tests/model/operation/mergeoperation.js

@@ -119,7 +119,7 @@ describe( 'MergeOperation', () => {
 				doc.version
 			);
 
-			expectToThrowCKEditorError( () => operation._validate(), /merge-operation-target-position-invalid/, model );
+			expectToThrowCKEditorError( () => operation._validate(), /model-position-path-incorrect/, model );
 		} );
 
 		it( 'should throw an error if source position is in root', () => {
@@ -139,6 +139,23 @@ describe( 'MergeOperation', () => {
 			expectToThrowCKEditorError( () => operation._validate(), /merge-operation-source-position-invalid/, model );
 		} );
 
+		it( 'should throw an error if target position is in root', () => {
+			const p1 = new Element( 'p1', null, new Text( 'Foo' ) );
+			const p2 = new Element( 'p2', null, new Text( 'bar' ) );
+
+			root._insertChild( 0, [ p1, p2 ] );
+
+			const operation = new MergeOperation(
+				new Position( root, [ 0, 3 ] ),
+				3,
+				new Position( root, [ 0 ] ),
+				gyPos,
+				doc.version
+			);
+
+			expectToThrowCKEditorError( () => operation._validate(), /merge-operation-target-position-invalid/, model );
+		} );
+
 		it( 'should throw an error if target position is invalid', () => {
 			const p1 = new Element( 'p1', null, new Text( 'Foo' ) );
 			const p2 = new Element( 'p2', null, new Text( 'bar' ) );
@@ -153,7 +170,7 @@ describe( 'MergeOperation', () => {
 				doc.version
 			);
 
-			expectToThrowCKEditorError( () => operation._validate(), /merge-operation-source-position-invalid/, model );
+			expectToThrowCKEditorError( () => operation._validate(), /model-position-path-incorrect/, model );
 		} );
 
 		it( 'should throw an error if number of nodes to move is invalid', () => {

+ 1 - 1
packages/ckeditor5-engine/tests/model/operation/moveoperation.js

@@ -171,7 +171,7 @@ describe( 'MoveOperation', () => {
 
 			root._removeChildren( 1 );
 
-			expectToThrowCKEditorError( () => operation._validate(), /move-operation-position-invalid/, model );
+			expectToThrowCKEditorError( () => operation._validate(), /model-position-path-incorrect/, model );
 		} );
 
 		it( 'should throw an error if operation tries to move a range between the beginning and the end of that range', () => {

+ 172 - 85
packages/ckeditor5-engine/tests/model/position.js

@@ -38,7 +38,7 @@ describe( 'Position', () => {
 	//        |- b   Before: [ 1, 1, 0 ] After: [ 1, 1, 1 ]
 	//        |- a   Before: [ 1, 1, 1 ] After: [ 1, 1, 2 ]
 	//        |- r   Before: [ 1, 1, 2 ] After: [ 1, 1, 3 ]
-	before( () => {
+	beforeEach( () => {
 		model = new Model();
 
 		doc = model.document;
@@ -110,11 +110,11 @@ describe( 'Position', () => {
 		it( 'should throw error if given path is incorrect', () => {
 			expectToThrowCKEditorError( () => {
 				new Position( root, {} ); // eslint-disable-line no-new
-			}, /model-position-path-incorrect/, model );
+			}, /model-position-path-incorrect-format/, model );
 
 			expectToThrowCKEditorError( () => {
 				new Position( root, [] ); // eslint-disable-line no-new
-			}, /model-position-path-incorrect/, model );
+			}, /model-position-path-incorrect-format/, model );
 		} );
 
 		it( 'should throw error if given root is invalid', () => {
@@ -283,119 +283,206 @@ describe( 'Position', () => {
 		} );
 	} );
 
-	it( 'should have parent', () => {
-		expect( new Position( root, [ 0 ] ) ).to.have.property( 'parent' ).that.equals( root );
-		expect( new Position( root, [ 1 ] ) ).to.have.property( 'parent' ).that.equals( root );
-		expect( new Position( root, [ 2 ] ) ).to.have.property( 'parent' ).that.equals( root );
+	describe( '#parent', () => {
+		it( 'should have parent', () => {
+			expect( new Position( root, [ 0 ] ) ).to.have.property( 'parent' ).that.equals( root );
+			expect( new Position( root, [ 1 ] ) ).to.have.property( 'parent' ).that.equals( root );
+			expect( new Position( root, [ 2 ] ) ).to.have.property( 'parent' ).that.equals( root );
 
-		expect( new Position( root, [ 0, 0 ] ) ).to.have.property( 'parent' ).that.equals( p );
+			expect( new Position( root, [ 0, 0 ] ) ).to.have.property( 'parent' ).that.equals( p );
 
-		expect( new Position( root, [ 1, 0 ] ) ).to.have.property( 'parent' ).that.equals( ul );
-		expect( new Position( root, [ 1, 1 ] ) ).to.have.property( 'parent' ).that.equals( ul );
-		expect( new Position( root, [ 1, 2 ] ) ).to.have.property( 'parent' ).that.equals( ul );
+			expect( new Position( root, [ 1, 0 ] ) ).to.have.property( 'parent' ).that.equals( ul );
+			expect( new Position( root, [ 1, 1 ] ) ).to.have.property( 'parent' ).that.equals( ul );
+			expect( new Position( root, [ 1, 2 ] ) ).to.have.property( 'parent' ).that.equals( ul );
 
-		expect( new Position( root, [ 1, 0, 0 ] ) ).to.have.property( 'parent' ).that.equals( li1 );
-		expect( new Position( root, [ 1, 0, 1 ] ) ).to.have.property( 'parent' ).that.equals( li1 );
-		expect( new Position( root, [ 1, 0, 2 ] ) ).to.have.property( 'parent' ).that.equals( li1 );
-		expect( new Position( root, [ 1, 0, 3 ] ) ).to.have.property( 'parent' ).that.equals( li1 );
-	} );
+			expect( new Position( root, [ 1, 0, 0 ] ) ).to.have.property( 'parent' ).that.equals( li1 );
+			expect( new Position( root, [ 1, 0, 1 ] ) ).to.have.property( 'parent' ).that.equals( li1 );
+			expect( new Position( root, [ 1, 0, 2 ] ) ).to.have.property( 'parent' ).that.equals( li1 );
+			expect( new Position( root, [ 1, 0, 3 ] ) ).to.have.property( 'parent' ).that.equals( li1 );
+		} );
+
+		it( 'should work with positions rooted in document fragment', () => {
+			const docFrag = new DocumentFragment();
+
+			expect( new Position( docFrag, [ 0 ] ) ).to.have.property( 'parent' ).that.equals( docFrag );
+		} );
 
-	it( 'should have offset', () => {
-		expect( new Position( root, [ 0 ] ) ).to.have.property( 'offset' ).that.equals( 0 );
-		expect( new Position( root, [ 1 ] ) ).to.have.property( 'offset' ).that.equals( 1 );
-		expect( new Position( root, [ 2 ] ) ).to.have.property( 'offset' ).that.equals( 2 );
+		it( 'should throw when path out of bounds', () => {
+			const position = new Position( root, [ 0, 0 ] );
 
-		expect( new Position( root, [ 0, 0 ] ) ).to.have.property( 'offset' ).that.equals( 0 );
+			expect( position ).to.have.property( 'parent' ).that.equals( p );
 
-		expect( new Position( root, [ 1, 0 ] ) ).to.have.property( 'offset' ).that.equals( 0 );
-		expect( new Position( root, [ 1, 1 ] ) ).to.have.property( 'offset' ).that.equals( 1 );
-		expect( new Position( root, [ 1, 2 ] ) ).to.have.property( 'offset' ).that.equals( 2 );
+			root._removeChildren( 0, 2 );
 
-		expect( new Position( root, [ 1, 0, 0 ] ) ).to.have.property( 'offset' ).that.equals( 0 );
-		expect( new Position( root, [ 1, 0, 1 ] ) ).to.have.property( 'offset' ).that.equals( 1 );
-		expect( new Position( root, [ 1, 0, 2 ] ) ).to.have.property( 'offset' ).that.equals( 2 );
-		expect( new Position( root, [ 1, 0, 3 ] ) ).to.have.property( 'offset' ).that.equals( 3 );
+			expectToThrowCKEditorError( () => {
+				position.parent;
+			}, /^model-position-path-incorrect:/, position, { position } );
+		} );
+
+		it( 'should throw when based on a path, the parent would be a text node', () => {
+			// 1,0,0 points at: <p></p><ul><li>^foz</li>...
+			const position = new Position( root, [ 1, 0, 0, 0 ] );
+
+			expectToThrowCKEditorError( () => {
+				position.parent;
+			}, /^model-position-path-incorrect:/, position, { position } );
+		} );
 	} );
 
-	it( 'should have index', () => {
-		expect( new Position( root, [ 0 ] ) ).to.have.property( 'index' ).that.equals( 0 );
-		expect( new Position( root, [ 1 ] ) ).to.have.property( 'index' ).that.equals( 1 );
-		expect( new Position( root, [ 2 ] ) ).to.have.property( 'index' ).that.equals( 2 );
+	describe( '#offset', () => {
+		it( 'should have offset', () => {
+			expect( new Position( root, [ 0 ] ) ).to.have.property( 'offset' ).that.equals( 0 );
+			expect( new Position( root, [ 1 ] ) ).to.have.property( 'offset' ).that.equals( 1 );
+			expect( new Position( root, [ 2 ] ) ).to.have.property( 'offset' ).that.equals( 2 );
 
-		expect( new Position( root, [ 0, 0 ] ) ).to.have.property( 'index' ).that.equals( 0 );
+			expect( new Position( root, [ 0, 0 ] ) ).to.have.property( 'offset' ).that.equals( 0 );
 
-		expect( new Position( root, [ 1, 0 ] ) ).to.have.property( 'index' ).that.equals( 0 );
-		expect( new Position( root, [ 1, 1 ] ) ).to.have.property( 'index' ).that.equals( 1 );
-		expect( new Position( root, [ 1, 2 ] ) ).to.have.property( 'index' ).that.equals( 2 );
+			expect( new Position( root, [ 1, 0 ] ) ).to.have.property( 'offset' ).that.equals( 0 );
+			expect( new Position( root, [ 1, 1 ] ) ).to.have.property( 'offset' ).that.equals( 1 );
+			expect( new Position( root, [ 1, 2 ] ) ).to.have.property( 'offset' ).that.equals( 2 );
 
-		expect( new Position( root, [ 1, 0, 0 ] ) ).to.have.property( 'index' ).that.equals( 0 );
-		expect( new Position( root, [ 1, 0, 1 ] ) ).to.have.property( 'index' ).that.equals( 0 );
-		expect( new Position( root, [ 1, 0, 2 ] ) ).to.have.property( 'index' ).that.equals( 0 );
-		expect( new Position( root, [ 1, 0, 3 ] ) ).to.have.property( 'index' ).that.equals( 1 );
+			expect( new Position( root, [ 1, 0, 0 ] ) ).to.have.property( 'offset' ).that.equals( 0 );
+			expect( new Position( root, [ 1, 0, 1 ] ) ).to.have.property( 'offset' ).that.equals( 1 );
+			expect( new Position( root, [ 1, 0, 2 ] ) ).to.have.property( 'offset' ).that.equals( 2 );
+			expect( new Position( root, [ 1, 0, 3 ] ) ).to.have.property( 'offset' ).that.equals( 3 );
+		} );
 	} );
 
-	it( 'should be able to set offset', () => {
-		const position = new Position( root, [ 1, 0, 2 ] );
-		position.offset = 4;
+	describe( '#index', () => {
+		it( 'should have index', () => {
+			expect( new Position( root, [ 0 ] ) ).to.have.property( 'index' ).that.equals( 0 );
+			expect( new Position( root, [ 1 ] ) ).to.have.property( 'index' ).that.equals( 1 );
+			expect( new Position( root, [ 2 ] ) ).to.have.property( 'index' ).that.equals( 2 );
+
+			expect( new Position( root, [ 0, 0 ] ) ).to.have.property( 'index' ).that.equals( 0 );
+
+			expect( new Position( root, [ 1, 0 ] ) ).to.have.property( 'index' ).that.equals( 0 );
+			expect( new Position( root, [ 1, 1 ] ) ).to.have.property( 'index' ).that.equals( 1 );
+			expect( new Position( root, [ 1, 2 ] ) ).to.have.property( 'index' ).that.equals( 2 );
+
+			expect( new Position( root, [ 1, 0, 0 ] ) ).to.have.property( 'index' ).that.equals( 0 );
+			expect( new Position( root, [ 1, 0, 1 ] ) ).to.have.property( 'index' ).that.equals( 0 );
+			expect( new Position( root, [ 1, 0, 2 ] ) ).to.have.property( 'index' ).that.equals( 0 );
+			expect( new Position( root, [ 1, 0, 3 ] ) ).to.have.property( 'index' ).that.equals( 1 );
+		} );
+
+		it( 'should be able to set offset', () => {
+			const position = new Position( root, [ 1, 0, 2 ] );
+			position.offset = 4;
 
-		expect( position.offset ).to.equal( 4 );
-		expect( position.path ).to.deep.equal( [ 1, 0, 4 ] );
+			expect( position.offset ).to.equal( 4 );
+			expect( position.path ).to.deep.equal( [ 1, 0, 4 ] );
+		} );
+
+		it( 'should throw when path out of bounds', () => {
+			const position = new Position( root, [ 0, 0 ] );
+
+			expect( position ).to.have.property( 'index' ).that.equals( 0 );
+
+			root._removeChildren( 0, 2 );
+
+			expectToThrowCKEditorError( () => {
+				position.index;
+			}, /^model-position-path-incorrect:/, position, { position } );
+		} );
 	} );
 
-	it( 'should have nodeBefore if it is not inside a text node', () => {
-		expect( new Position( root, [ 0 ] ).nodeBefore ).to.be.null;
-		expect( new Position( root, [ 1 ] ).nodeBefore ).to.equal( p );
-		expect( new Position( root, [ 2 ] ).nodeBefore ).to.equal( ul );
+	describe( '#nodeBefore', () => {
+		it( 'should have nodeBefore if it is not inside a text node', () => {
+			expect( new Position( root, [ 0 ] ).nodeBefore ).to.be.null;
+			expect( new Position( root, [ 1 ] ).nodeBefore ).to.equal( p );
+			expect( new Position( root, [ 2 ] ).nodeBefore ).to.equal( ul );
+
+			expect( new Position( root, [ 0, 0 ] ).nodeBefore ).to.null;
+
+			expect( new Position( root, [ 1, 0 ] ).nodeBefore ).to.be.null;
+			expect( new Position( root, [ 1, 1 ] ).nodeBefore ).to.equal( li1 );
+			expect( new Position( root, [ 1, 2 ] ).nodeBefore ).to.equal( li2 );
+
+			expect( new Position( root, [ 1, 0, 0 ] ).nodeBefore ).to.be.null;
+			expect( new Position( root, [ 1, 0, 1 ] ).nodeBefore ).to.be.null;
+			expect( new Position( root, [ 1, 0, 2 ] ).nodeBefore ).to.be.null;
+			expect( new Position( root, [ 1, 0, 3 ] ).nodeBefore.data ).to.equal( 'foz' );
+		} );
+
+		it( 'should throw when path out of bounds', () => {
+			const position = new Position( root, [ 1, 1 ] );
 
-		expect( new Position( root, [ 0, 0 ] ).nodeBefore ).to.null;
+			expect( position ).to.have.property( 'nodeBefore' ).that.equals( li1 );
 
-		expect( new Position( root, [ 1, 0 ] ).nodeBefore ).to.be.null;
-		expect( new Position( root, [ 1, 1 ] ).nodeBefore ).to.equal( li1 );
-		expect( new Position( root, [ 1, 2 ] ).nodeBefore ).to.equal( li2 );
+			root._removeChildren( 0, 2 );
 
-		expect( new Position( root, [ 1, 0, 0 ] ).nodeBefore ).to.be.null;
-		expect( new Position( root, [ 1, 0, 1 ] ).nodeBefore ).to.be.null;
-		expect( new Position( root, [ 1, 0, 2 ] ).nodeBefore ).to.be.null;
-		expect( new Position( root, [ 1, 0, 3 ] ).nodeBefore.data ).to.equal( 'foz' );
+			expectToThrowCKEditorError( () => {
+				position.nodeBefore;
+			}, /^model-nodelist-offset-out-of-bounds:/, position );
+		} );
 	} );
 
-	it( 'should have nodeAfter if it is not inside a text node', () => {
-		expect( new Position( root, [ 0 ] ).nodeAfter ).to.equal( p );
-		expect( new Position( root, [ 1 ] ).nodeAfter ).to.equal( ul );
-		expect( new Position( root, [ 2 ] ).nodeAfter ).to.be.null;
+	describe( '#nodeAfter', () => {
+		it( 'should have nodeAfter if it is not inside a text node', () => {
+			expect( new Position( root, [ 0 ] ).nodeAfter ).to.equal( p );
+			expect( new Position( root, [ 1 ] ).nodeAfter ).to.equal( ul );
+			expect( new Position( root, [ 2 ] ).nodeAfter ).to.be.null;
+
+			expect( new Position( root, [ 0, 0 ] ).nodeAfter ).to.be.null;
+
+			expect( new Position( root, [ 1, 0 ] ).nodeAfter ).to.equal( li1 );
+			expect( new Position( root, [ 1, 1 ] ).nodeAfter ).to.equal( li2 );
+			expect( new Position( root, [ 1, 2 ] ).nodeAfter ).to.be.null;
+
+			expect( new Position( root, [ 1, 0, 0 ] ).nodeAfter.data ).to.equal( 'foz' );
+			expect( new Position( root, [ 1, 0, 1 ] ).nodeAfter ).to.be.null;
+			expect( new Position( root, [ 1, 0, 2 ] ).nodeAfter ).to.be.null;
+			expect( new Position( root, [ 1, 0, 3 ] ).nodeAfter ).to.be.null;
+		} );
+
+		it( 'should throw when path out of bounds', () => {
+			const position = new Position( root, [ 1, 1 ] );
 
-		expect( new Position( root, [ 0, 0 ] ).nodeAfter ).to.be.null;
+			expect( position ).to.have.property( 'nodeAfter' ).that.equals( li2 );
 
-		expect( new Position( root, [ 1, 0 ] ).nodeAfter ).to.equal( li1 );
-		expect( new Position( root, [ 1, 1 ] ).nodeAfter ).to.equal( li2 );
-		expect( new Position( root, [ 1, 2 ] ).nodeAfter ).to.be.null;
+			root._removeChildren( 0, 2 );
 
-		expect( new Position( root, [ 1, 0, 0 ] ).nodeAfter.data ).to.equal( 'foz' );
-		expect( new Position( root, [ 1, 0, 1 ] ).nodeAfter ).to.be.null;
-		expect( new Position( root, [ 1, 0, 2 ] ).nodeAfter ).to.be.null;
-		expect( new Position( root, [ 1, 0, 3 ] ).nodeAfter ).to.be.null;
+			expectToThrowCKEditorError( () => {
+				position.nodeAfter;
+			}, /^model-nodelist-offset-out-of-bounds:/, position );
+		} );
 	} );
 
-	it( 'should have a text node property if it is in text node', () => {
-		expect( new Position( root, [ 0 ] ).textNode ).to.be.null;
-		expect( new Position( root, [ 1 ] ).textNode ).to.be.null;
-		expect( new Position( root, [ 2 ] ).textNode ).to.be.null;
+	describe( '#textNode', () => {
+		it( 'should have a text node property if it is in text node', () => {
+			expect( new Position( root, [ 0 ] ).textNode ).to.be.null;
+			expect( new Position( root, [ 1 ] ).textNode ).to.be.null;
+			expect( new Position( root, [ 2 ] ).textNode ).to.be.null;
+
+			expect( new Position( root, [ 0, 0 ] ).textNode ).to.be.null;
 
-		expect( new Position( root, [ 0, 0 ] ).textNode ).to.be.null;
+			expect( new Position( root, [ 1, 0 ] ).textNode ).to.be.null;
+			expect( new Position( root, [ 1, 1 ] ).textNode ).to.be.null;
+			expect( new Position( root, [ 1, 2 ] ).textNode ).to.be.null;
 
-		expect( new Position( root, [ 1, 0 ] ).textNode ).to.be.null;
-		expect( new Position( root, [ 1, 1 ] ).textNode ).to.be.null;
-		expect( new Position( root, [ 1, 2 ] ).textNode ).to.be.null;
+			expect( new Position( root, [ 1, 0, 0 ] ).textNode ).to.be.null;
+			expect( new Position( root, [ 1, 0, 1 ] ).textNode ).to.equal( foz );
+			expect( new Position( root, [ 1, 0, 2 ] ).textNode ).to.equal( foz );
+			expect( new Position( root, [ 1, 0, 3 ] ).textNode ).to.be.null;
 
-		expect( new Position( root, [ 1, 0, 0 ] ).textNode ).to.be.null;
-		expect( new Position( root, [ 1, 0, 1 ] ).textNode ).to.equal( foz );
-		expect( new Position( root, [ 1, 0, 2 ] ).textNode ).to.equal( foz );
-		expect( new Position( root, [ 1, 0, 3 ] ).textNode ).to.be.null;
+			expect( new Position( root, [ 1, 1, 0 ] ).textNode ).to.be.null;
+			expect( new Position( root, [ 1, 1, 1 ] ).textNode ).to.equal( bar );
+			expect( new Position( root, [ 1, 1, 2 ] ).textNode ).to.equal( bar );
+			expect( new Position( root, [ 1, 1, 3 ] ).textNode ).to.be.null;
+		} );
+
+		it( 'should throw when path out of bounds', () => {
+			const position = new Position( root, [ 1, 0, 1 ] );
+
+			expect( position ).to.have.property( 'textNode' ).that.equals( foz );
 
-		expect( new Position( root, [ 1, 1, 0 ] ).textNode ).to.be.null;
-		expect( new Position( root, [ 1, 1, 1 ] ).textNode ).to.equal( bar );
-		expect( new Position( root, [ 1, 1, 2 ] ).textNode ).to.equal( bar );
-		expect( new Position( root, [ 1, 1, 3 ] ).textNode ).to.be.null;
+			root._removeChildren( 0, 2 );
+
+			expectToThrowCKEditorError( () => {
+				position.textNode;
+			}, /^model-nodelist-offset-out-of-bounds:/, position );
+		} );
 	} );
 
 	it( 'should have proper parent path', () => {

+ 107 - 4
packages/ckeditor5-engine/tests/utils/bindtwostepcarettoattribute.js

@@ -9,15 +9,18 @@ import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtest
 import DomEmitterMixin from '@ckeditor/ckeditor5-utils/src/dom/emittermixin';
 import DomEventData from '../../src/view/observer/domeventdata';
 import EventInfo from '@ckeditor/ckeditor5-utils/src/eventinfo';
-import bindTwoStepCaretToAttribute from '../../src/utils/bindtwostepcarettoattribute';
+import bindTwoStepCaretToAttribute, { TwoStepCaretHandler } from '../../src/utils/bindtwostepcarettoattribute';
 import Position from '../../src/model/position';
+import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
 import { setData } from '../../src/dev-utils/model';
 
 describe( 'bindTwoStepCaretToAttribute()', () => {
-	let editor, model, emitter, selection, view;
+	let editor, model, emitter, selection, view, locale;
 	let preventDefaultSpy, evtStopSpy;
 
+	testUtils.createSinonSandbox();
+
 	beforeEach( () => {
 		emitter = Object.create( DomEmitterMixin );
 
@@ -26,6 +29,7 @@ describe( 'bindTwoStepCaretToAttribute()', () => {
 			model = editor.model;
 			selection = model.document.selection;
 			view = editor.editing.view;
+			locale = editor.locale;
 
 			preventDefaultSpy = sinon.spy();
 			evtStopSpy = sinon.spy();
@@ -41,7 +45,13 @@ describe( 'bindTwoStepCaretToAttribute()', () => {
 			editor.conversion.for( 'upcast' ).elementToAttribute( { view: 'c', model: 'c' } );
 			editor.conversion.elementToElement( { model: 'paragraph', view: 'p' } );
 
-			bindTwoStepCaretToAttribute( editor.editing.view, editor.model, emitter, 'a' );
+			bindTwoStepCaretToAttribute( {
+				view: editor.editing.view,
+				model: editor.model,
+				emitter,
+				attribute: 'a',
+				locale
+			} );
 		} );
 	} );
 
@@ -550,7 +560,13 @@ describe( 'bindTwoStepCaretToAttribute()', () => {
 
 	describe( 'multiple attributes', () => {
 		beforeEach( () => {
-			bindTwoStepCaretToAttribute( editor.editing.view, editor.model, emitter, 'c' );
+			bindTwoStepCaretToAttribute( {
+				view: editor.editing.view,
+				model: editor.model,
+				emitter,
+				attribute: 'c',
+				locale
+			} );
 		} );
 
 		it( 'should work with the two-step caret movement (moving right)', () => {
@@ -743,6 +759,93 @@ describe( 'bindTwoStepCaretToAttribute()', () => {
 		expect( getSelectionAttributesArray( selection ) ).to.have.members( [] );
 	} );
 
+	describe( 'left–to–right and right–to–left content', () => {
+		it( 'should call methods associated with the keys (LTR content direction)', () => {
+			const forwardStub = testUtils.sinon.stub( TwoStepCaretHandler.prototype, 'handleForwardMovement' );
+			const backwardStub = testUtils.sinon.stub( TwoStepCaretHandler.prototype, 'handleBackwardMovement' );
+
+			setData( model, '<$text>foo[]</$text><$text a="true">bar</$text>' );
+
+			fireKeyDownEvent( {
+				keyCode: keyCodes.arrowright
+			} );
+
+			sinon.assert.calledOnce( forwardStub );
+			sinon.assert.notCalled( backwardStub );
+
+			setData( model, '<$text>foo</$text><$text a="true">[]bar</$text>' );
+
+			fireKeyDownEvent( {
+				keyCode: keyCodes.arrowleft
+			} );
+
+			sinon.assert.calledOnce( backwardStub );
+			sinon.assert.calledOnce( forwardStub );
+		} );
+
+		it( 'should use the opposite helper methods (RTL content direction)', () => {
+			const forwardStub = testUtils.sinon.stub( TwoStepCaretHandler.prototype, 'handleForwardMovement' );
+			const backwardStub = testUtils.sinon.stub( TwoStepCaretHandler.prototype, 'handleBackwardMovement' );
+			const emitter = Object.create( DomEmitterMixin );
+
+			let model;
+
+			return VirtualTestEditor
+				.create( {
+					language: {
+						content: 'ar'
+					}
+				} )
+				.then( newEditor => {
+					model = newEditor.model;
+					selection = model.document.selection;
+					view = newEditor.editing.view;
+
+					newEditor.model.schema.extend( '$text', {
+						allowAttributes: [ 'a', 'b', 'c' ],
+						allowIn: '$root'
+					} );
+
+					model.schema.register( 'paragraph', { inheritAllFrom: '$block' } );
+					newEditor.conversion.for( 'upcast' ).elementToAttribute( { view: 'a', model: 'a' } );
+					newEditor.conversion.for( 'upcast' ).elementToAttribute( { view: 'b', model: 'b' } );
+					newEditor.conversion.for( 'upcast' ).elementToAttribute( { view: 'c', model: 'c' } );
+					newEditor.conversion.elementToElement( { model: 'paragraph', view: 'p' } );
+
+					bindTwoStepCaretToAttribute( {
+						view: newEditor.editing.view,
+						model: newEditor.model,
+						emitter,
+						attribute: 'a',
+						locale: newEditor.locale
+					} );
+
+					return newEditor;
+				} )
+				.then( newEditor => {
+					setData( model, '<$text>foo[]</$text><$text a="true">bar</$text>' );
+
+					fireKeyDownEvent( {
+						keyCode: keyCodes.arrowleft
+					} );
+
+					sinon.assert.calledOnce( forwardStub );
+					sinon.assert.notCalled( backwardStub );
+
+					setData( model, '<$text>foo</$text><$text a="true">[]bar</$text>' );
+
+					fireKeyDownEvent( {
+						keyCode: keyCodes.arrowright
+					} );
+
+					sinon.assert.calledOnce( backwardStub );
+					sinon.assert.calledOnce( forwardStub );
+
+					return newEditor.destroy();
+				} );
+		} );
+	} );
+
 	const keyMap = {
 		'→': 'arrowright',
 		'←': 'arrowleft'