8
0
Просмотр исходного кода

Merge branch 'master' into t/1055

# Conflicts:
#	tests/model/delta/transform/removedelta.js
#	tests/model/delta/transform/splitdelta.js
Szymon Cofalik 8 лет назад
Родитель
Сommit
2a1492bbf8

+ 1 - 6
packages/ckeditor5-engine/src/controller/deletecontent.js

@@ -197,13 +197,8 @@ function replaceEntireContentWithParagraph( batch, selection ) {
 // * whether the paragraph is allowed in schema in the common ancestor.
 // * whether the paragraph is allowed in schema in the common ancestor.
 function shouldEntireContentBeReplacedWithParagraph( schema, selection ) {
 function shouldEntireContentBeReplacedWithParagraph( schema, selection ) {
 	const limitElement = schema.getLimitElement( selection );
 	const limitElement = schema.getLimitElement( selection );
-	const limitStartPosition = Position.createAt( limitElement );
-	const limitEndPosition = Position.createAt( limitElement, 'end' );
 
 
-	if (
-		!limitStartPosition.isTouching( selection.getFirstPosition() ) ||
-		!limitEndPosition.isTouching( selection.getLastPosition() )
-	) {
+	if ( !selection.isEntireContentSelected( limitElement ) ) {
 		return false;
 		return false;
 	}
 	}
 
 

+ 11 - 1
packages/ckeditor5-engine/src/conversion/model-to-view-converters.js

@@ -6,6 +6,7 @@
 import ViewElement from '../view/element';
 import ViewElement from '../view/element';
 import ViewText from '../view/text';
 import ViewText from '../view/text';
 import ViewRange from '../view/range';
 import ViewRange from '../view/range';
+import ViewPosition from '../view/position';
 import ViewTreeWalker from '../view/treewalker';
 import ViewTreeWalker from '../view/treewalker';
 import viewWriter from '../view/writer';
 import viewWriter from '../view/writer';
 
 
@@ -441,13 +442,22 @@ export function remove() {
 		// end of that range is incorrect.
 		// end of that range is incorrect.
 		// Instead we will use `data.sourcePosition` as this is the last correct model position and
 		// Instead we will use `data.sourcePosition` as this is the last correct model position and
 		// it is a position before the removed item. Then, we will calculate view range to remove "manually".
 		// it is a position before the removed item. Then, we will calculate view range to remove "manually".
-		const viewPosition = conversionApi.mapper.toViewPosition( data.sourcePosition );
+		let viewPosition = conversionApi.mapper.toViewPosition( data.sourcePosition );
 		let viewRange;
 		let viewRange;
 
 
 		if ( data.item.is( 'element' ) ) {
 		if ( data.item.is( 'element' ) ) {
 			// Note: in remove conversion we cannot use model-to-view element mapping because `data.item` may be
 			// Note: in remove conversion we cannot use model-to-view element mapping because `data.item` may be
 			// already mapped to another element (this happens when move change is converted).
 			// already mapped to another element (this happens when move change is converted).
 			// In this case however, `viewPosition` is the position before view element that corresponds to removed model element.
 			// In this case however, `viewPosition` is the position before view element that corresponds to removed model element.
+			//
+			// First, fix the position. Traverse the tree forward until the container element is found. The `viewPosition`
+			// may be before a ui element, before attribute element or at the end of text element.
+			viewPosition = viewPosition.getLastMatchingPosition( value => !value.item.is( 'containerElement' ) );
+
+			if ( viewPosition.parent.is( 'text' ) && viewPosition.isAtEnd ) {
+				viewPosition = ViewPosition.createAfter( viewPosition.parent );
+			}
+
 			viewRange = ViewRange.createOn( viewPosition.nodeAfter );
 			viewRange = ViewRange.createOn( viewPosition.nodeAfter );
 		} else {
 		} else {
 			// If removed item is a text node, we need to traverse view tree to find the view range to remove.
 			// If removed item is a text node, we need to traverse view tree to find the view range to remove.

+ 16 - 3
packages/ckeditor5-engine/src/model/delta/basic-transformations.js

@@ -407,7 +407,13 @@ addTransformationCase( SplitDelta, RenameDelta, ( a, b, context ) => {
 // Add special case for RemoveDelta x SplitDelta transformation.
 // Add special case for RemoveDelta x SplitDelta transformation.
 addTransformationCase( RemoveDelta, SplitDelta, ( a, b, context ) => {
 addTransformationCase( RemoveDelta, SplitDelta, ( a, b, context ) => {
 	const deltas = defaultTransform( a, b, context );
 	const deltas = defaultTransform( a, b, context );
-	const insertPosition = b._cloneOperation.position;
+	// The "clone operation" may be InsertOperation, ReinsertOperation, MoveOperation or NoOperation.
+	const insertPosition = b._cloneOperation.position || b._cloneOperation.targetPosition;
+
+	// NoOperation.
+	if ( !insertPosition ) {
+		return defaultTransform( a, b, context );
+	}
 
 
 	const undoMode = context.aWasUndone || context.bWasUndone;
 	const undoMode = context.aWasUndone || context.bWasUndone;
 
 
@@ -444,9 +450,16 @@ addTransformationCase( SplitDelta, RemoveDelta, ( a, b, context ) => {
 	// This case is very trickily solved.
 	// This case is very trickily solved.
 	// Instead of fixing `a` delta, we change `b` delta for a while and fire default transformation with fixed `b` delta.
 	// Instead of fixing `a` delta, we change `b` delta for a while and fire default transformation with fixed `b` delta.
 	// Thanks to that fixing `a` delta will be differently (correctly) transformed.
 	// Thanks to that fixing `a` delta will be differently (correctly) transformed.
-	b = b.clone();
+	//
+	// The "clone operation" may be InsertOperation, ReinsertOperation, MoveOperation or NoOperation.
+	const insertPosition = a._cloneOperation.position || a._cloneOperation.targetPosition;
 
 
-	const insertPosition = a._cloneOperation.position;
+	// NoOperation.
+	if ( !insertPosition ) {
+		return defaultTransform( a, b, context );
+	}
+
+	b = b.clone();
 	const operation = b._moveOperation;
 	const operation = b._moveOperation;
 	const rangeEnd = operation.sourcePosition.getShiftedBy( operation.howMany );
 	const rangeEnd = operation.sourcePosition.getShiftedBy( operation.howMany );
 
 

+ 19 - 0
packages/ckeditor5-engine/src/model/selection.js

@@ -624,6 +624,25 @@ export default class Selection {
 		}
 		}
 	}
 	}
 
 
+	/**
+	 * Checks whether the selection contains the entire content of the given element. This means that selection must start
+	 * at a position {@link module:engine/model/position~Position#isTouching touching} the element's start and ends at position
+	 * touching the element's end.
+	 *
+	 * By default, this method will check whether the entire content of the selection's current root is selected.
+	 * Useful to check if e.g. the user has just pressed <kbd>Ctrl</kbd> + <kbd>A</kbd>.
+	 *
+	 * @param {module:engine/model/element~Element} [element=this.anchor.root]
+	 * @returns {Boolean}
+	 */
+	isEntireContentSelected( element = this.anchor.root ) {
+		const limitStartPosition = Position.createAt( element );
+		const limitEndPosition = Position.createAt( element, 'end' );
+
+		return limitStartPosition.isTouching( this.getFirstPosition() ) &&
+			limitEndPosition.isTouching( this.getLastPosition() );
+	}
+
 	/**
 	/**
 	 * Creates and returns an instance of `Selection` that is a clone of given selection, meaning that it has same
 	 * Creates and returns an instance of `Selection` that is a clone of given selection, meaning that it has same
 	 * ranges and same direction as this selection.
 	 * ranges and same direction as this selection.

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

@@ -21,6 +21,7 @@ import KeyObserver from './observer/keyobserver';
 import FakeSelectionObserver from './observer/fakeselectionobserver';
 import FakeSelectionObserver from './observer/fakeselectionobserver';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
 import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
 import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
+import { scrollViewportToShowTarget } from '@ckeditor/ckeditor5-utils/src/dom/scroll';
 
 
 /**
 /**
  * Document class creates an abstract layer over the content editable area.
  * Document class creates an abstract layer over the content editable area.
@@ -301,6 +302,21 @@ export default class Document {
 		}
 		}
 	}
 	}
 
 
+	/**
+	 * Scrolls the page viewport and {@link #domRoots} with their ancestors to reveal the
+	 * caret, if not already visible to the user.
+	 */
+	scrollToTheSelection() {
+		const range = this.selection.getFirstRange();
+
+		if ( range ) {
+			scrollViewportToShowTarget( {
+				target: this.domConverter.viewRangeToDom( range ),
+				viewportOffset: 20
+			} );
+		}
+	}
+
 	/**
 	/**
 	 * Disables all added observers.
 	 * Disables all added observers.
 	 */
 	 */

+ 61 - 4
packages/ckeditor5-engine/tests/conversion/model-to-view-converters.js

@@ -1028,8 +1028,8 @@ describe( 'model-to-view-converters', () => {
 		} );
 		} );
 
 
 		it( 'should not unbind element that has not been moved to graveyard', () => {
 		it( 'should not unbind element that has not been moved to graveyard', () => {
-			const modelElement = new ModelElement( 'a' );
-			const viewElement = new ViewElement( 'a' );
+			const modelElement = new ModelElement( 'paragraph' );
+			const viewElement = new ViewContainerElement( 'p' );
 
 
 			modelRoot.appendChildren( [ modelElement, new ModelText( 'b' ) ] );
 			modelRoot.appendChildren( [ modelElement, new ModelText( 'b' ) ] );
 			viewRoot.appendChildren( [ viewElement, new ViewText( 'b' ) ] );
 			viewRoot.appendChildren( [ viewElement, new ViewText( 'b' ) ] );
@@ -1056,8 +1056,8 @@ describe( 'model-to-view-converters', () => {
 		} );
 		} );
 
 
 		it( 'should unbind elements if model element was moved to graveyard', () => {
 		it( 'should unbind elements if model element was moved to graveyard', () => {
-			const modelElement = new ModelElement( 'a' );
-			const viewElement = new ViewElement( 'a' );
+			const modelElement = new ModelElement( 'paragraph' );
+			const viewElement = new ViewContainerElement( 'p' );
 
 
 			modelRoot.appendChildren( [ modelElement, new ModelText( 'b' ) ] );
 			modelRoot.appendChildren( [ modelElement, new ModelText( 'b' ) ] );
 			viewRoot.appendChildren( [ viewElement, new ViewText( 'b' ) ] );
 			viewRoot.appendChildren( [ viewElement, new ViewText( 'b' ) ] );
@@ -1125,5 +1125,62 @@ describe( 'model-to-view-converters', () => {
 			expect( mapper.toModelElement( viewWElement ) ).to.be.undefined;
 			expect( mapper.toModelElement( viewWElement ) ).to.be.undefined;
 			expect( mapper.toViewElement( modelWElement ) ).to.be.undefined;
 			expect( mapper.toViewElement( modelWElement ) ).to.be.undefined;
 		} );
 		} );
+
+		it( 'should work correctly if container element after ui element is removed', () => {
+			const modelP1 = new ModelElement( 'paragraph' );
+			const modelP2 = new ModelElement( 'paragraph' );
+
+			const viewP1 = new ViewContainerElement( 'p' );
+			const viewUi1 = new ViewUIElement( 'span' );
+			const viewUi2 = new ViewUIElement( 'span' );
+			const viewP2 = new ViewContainerElement( 'p' );
+
+			modelRoot.appendChildren( [ modelP1, modelP2 ] );
+			viewRoot.appendChildren( [ viewP1, viewUi1, viewUi2, viewP2 ] );
+
+			mapper.bindElements( modelP1, viewP1 );
+			mapper.bindElements( modelP2, viewP2 );
+
+			dispatcher.on( 'remove', remove() );
+
+			modelWriter.move(
+				ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 2 ),
+				ModelPosition.createAt( modelDoc.graveyard, 'end' )
+			);
+
+			dispatcher.convertRemove(
+				ModelPosition.createFromParentAndOffset( modelRoot, 1 ),
+				ModelRange.createFromParentsAndOffsets( modelDoc.graveyard, 0, modelDoc.graveyard, 1 )
+			);
+
+			expect( viewToString( viewRoot ) ).to.equal( '<div><p></p><span></span><span></span></div>' );
+		} );
+
+		it( 'should work correctly if container element after text node is removed', () => {
+			const modelText = new ModelText( 'foo' );
+			const modelP = new ModelElement( 'paragraph' );
+
+			const viewText = new ViewText( 'foo' );
+			const viewP = new ViewContainerElement( 'p' );
+
+			modelRoot.appendChildren( [ modelText, modelP ] );
+			viewRoot.appendChildren( [ viewText, viewP ] );
+
+			mapper.bindElements( modelP, viewP );
+
+			dispatcher.on( 'remove', remove() );
+
+			modelWriter.move(
+				ModelRange.createFromParentsAndOffsets( modelRoot, 3, modelRoot, 4 ),
+				ModelPosition.createAt( modelDoc.graveyard, 'end' )
+			);
+
+			dispatcher.convertRemove(
+				ModelPosition.createFromParentAndOffset( modelRoot, 3 ),
+				ModelRange.createFromParentsAndOffsets( modelDoc.graveyard, 0, modelDoc.graveyard, 1 )
+			);
+
+			expect( viewToString( viewRoot ) ).to.equal( '<div>foo</div>' );
+		} );
 	} );
 	} );
 } );
 } );

+ 24 - 0
packages/ckeditor5-engine/tests/model/delta/transform/removedelta.js

@@ -213,6 +213,30 @@ describe( 'transform', () => {
 					]
 					]
 				} );
 				} );
 			} );
 			} );
+
+			it( 'should not throw if clone operation is NoOperation and use default transformation in that case', () => {
+				const noOpSplitDelta = new SplitDelta();
+				noOpSplitDelta.addOperation( new NoOperation( 0 ) );
+				noOpSplitDelta.addOperation( new MoveOperation( new Position( root, [ 1, 2 ] ), 3, new Position( root, [ 2, 0 ] ), 1 ) );
+
+				const removeDelta = getRemoveDelta( new Position( root, [ 3 ] ), 1, 0 );
+
+				const transformed = transform( removeDelta, noOpSplitDelta, context );
+
+				expect( transformed.length ).to.equal( 1 );
+
+				expectDelta( transformed[ 0 ], {
+					type: RemoveDelta,
+					operations: [
+						{
+							type: RemoveOperation,
+							sourcePosition: new Position( root, [ 3 ] ),
+							howMany: 1,
+							baseVersion: 2
+						}
+					]
+				} );
+			} );
 		} );
 		} );
 	} );
 	} );
 } );
 } );

+ 29 - 0
packages/ckeditor5-engine/tests/model/delta/transform/splitdelta.js

@@ -845,6 +845,35 @@ describe( 'transform', () => {
 					]
 					]
 				} );
 				} );
 			} );
 			} );
+
+			it( 'should not throw if clone operation is NoOperation and use default transformation in that case', () => {
+				const noOpSplitDelta = new SplitDelta();
+				noOpSplitDelta.addOperation( new NoOperation( 0 ) );
+				noOpSplitDelta.addOperation( new MoveOperation( new Position( root, [ 1, 2 ] ), 3, new Position( root, [ 2, 0 ] ), 1 ) );
+
+				const removeDelta = getRemoveDelta( new Position( root, [ 0 ] ), 1, 0 );
+
+				const transformed = transform( noOpSplitDelta, removeDelta, context );
+
+				expect( transformed.length ).to.equal( 1 );
+
+				expectDelta( transformed[ 0 ], {
+					type: SplitDelta,
+					operations: [
+						{
+							type: NoOperation,
+							baseVersion: 1
+						},
+						{
+							type: MoveOperation,
+							sourcePosition: new Position( root, [ 0, 2 ] ),
+							howMany: 3,
+							targetPosition: new Position( root, [ 1, 0 ] ),
+							baseVersion: 2
+						}
+					]
+				} );
+			} );
 		} );
 		} );
 	} );
 	} );
 } );
 } );

+ 58 - 0
packages/ckeditor5-engine/tests/model/selection.js

@@ -1237,4 +1237,62 @@ describe( 'Selection', () => {
 			} );
 			} );
 		} );
 		} );
 	} );
 	} );
+
+	describe( 'isEntireContentSelected()', () => {
+		beforeEach( () => {
+			doc.schema.registerItem( 'p', '$block' );
+			doc.schema.allow( { name: 'p', inside: '$root' } );
+		} );
+
+		it( 'returns true if the entire content in $root is selected', () => {
+			setData( doc, '<p>[Foo</p><p>Bom</p><p>Bar]</p>' );
+
+			expect( doc.selection.isEntireContentSelected() ).to.equal( true );
+		} );
+
+		it( 'returns false when only a fragment of the content in $root is selected', () => {
+			setData( doc, '<p>Fo[o</p><p>Bom</p><p>Bar]</p>' );
+
+			expect( doc.selection.isEntireContentSelected() ).to.equal( false );
+		} );
+
+		it( 'returns true if the entire content in specified element is selected', () => {
+			setData( doc, '<p>Foo</p><p>[Bom]</p><p>Bar</p>' );
+
+			const root = doc.getRoot();
+			const secondParagraph = root.getNodeByPath( [ 1 ] );
+
+			expect( doc.selection.isEntireContentSelected( secondParagraph ) ).to.equal( true );
+		} );
+
+		it( 'returns false if the entire content in specified element is not selected', () => {
+			setData( doc, '<p>Foo</p><p>[Bom</p><p>B]ar</p>' );
+
+			const root = doc.getRoot();
+			const secondParagraph = root.getNodeByPath( [ 1 ] );
+
+			expect( doc.selection.isEntireContentSelected( secondParagraph ) ).to.equal( false );
+		} );
+
+		it( 'returns false when the entire content except an empty element is selected', () => {
+			doc.schema.registerItem( 'img', '$inline' );
+			doc.schema.allow( { name: 'img', inside: 'p' } );
+
+			setData( doc, '<p><img></img>[Foo]</p>' );
+
+			expect( doc.selection.isEntireContentSelected() ).to.equal( false );
+		} );
+
+		it( 'returns true if the content is empty', () => {
+			setData( doc, '[]' );
+
+			expect( doc.selection.isEntireContentSelected() ).to.equal( true );
+		} );
+
+		it( 'returns false if empty selection is at the end of non-empty content', () => {
+			setData( doc, '<p>Foo bar bom.</p>[]' );
+
+			expect( doc.selection.isEntireContentSelected() ).to.equal( false );
+		} );
+	} );
 } );
 } );

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

@@ -19,6 +19,7 @@ import DomConverter from '../../../src/view/domconverter';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import count from '@ckeditor/ckeditor5-utils/src/count';
 import count from '@ckeditor/ckeditor5-utils/src/count';
 import log from '@ckeditor/ckeditor5-utils/src/log';
 import log from '@ckeditor/ckeditor5-utils/src/log';
+import global from '@ckeditor/ckeditor5-utils/src/dom/global';
 
 
 testUtils.createSinonSandbox();
 testUtils.createSinonSandbox();
 
 
@@ -323,6 +324,26 @@ describe( 'Document', () => {
 		} );
 		} );
 	} );
 	} );
 
 
+	describe( 'scrollToTheSelection()', () => {
+		it( 'does nothing when there are no ranges in the selection', () => {
+			const stub = testUtils.sinon.stub( global.window, 'scrollTo' );
+
+			viewDocument.scrollToTheSelection();
+			sinon.assert.notCalled( stub );
+		} );
+
+		it( 'scrolls to the first range in selection with an offset', () => {
+			const stub = testUtils.sinon.stub( global.window, 'scrollTo' );
+			const root = viewDocument.createRoot( document.createElement( 'div' ) );
+			const range = ViewRange.createIn( root );
+
+			viewDocument.selection.addRange( range );
+
+			viewDocument.scrollToTheSelection();
+			sinon.assert.calledWithMatch( stub, sinon.match.number, sinon.match.number );
+		} );
+	} );
+
 	describe( 'disableObservers()', () => {
 	describe( 'disableObservers()', () => {
 		it( 'should disable observers', () => {
 		it( 'should disable observers', () => {
 			const addedObserverMock = viewDocument.addObserver( ObserverMock );
 			const addedObserverMock = viewDocument.addObserver( ObserverMock );

+ 148 - 81
packages/ckeditor5-engine/tests/view/document/jumpoveruielement.js

@@ -8,10 +8,10 @@
 import ViewDocument from '../../../src/view/document';
 import ViewDocument from '../../../src/view/document';
 import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
 import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
 import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';
 import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';
-import { setData } from '../../../src/dev-utils/view';
+import { setData as setViewData } from '../../../src/dev-utils/view';
 
 
 describe( 'Document', () => {
 describe( 'Document', () => {
-	let viewDocument, domRoot;
+	let viewDocument, domRoot, domSelection;
 
 
 	beforeEach( () => {
 	beforeEach( () => {
 		domRoot = createElement( document, 'div', {
 		domRoot = createElement( document, 'div', {
@@ -22,7 +22,8 @@ describe( 'Document', () => {
 		viewDocument = new ViewDocument();
 		viewDocument = new ViewDocument();
 		viewDocument.createRoot( domRoot );
 		viewDocument.createRoot( domRoot );
 
 
-		document.getSelection().removeAllRanges();
+		domSelection = document.getSelection();
+		domSelection.removeAllRanges();
 
 
 		viewDocument.isFocused = true;
 		viewDocument.isFocused = true;
 	} );
 	} );
@@ -33,92 +34,158 @@ describe( 'Document', () => {
 		domRoot.parentElement.removeChild( domRoot );
 		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();
+	function prepare( view, options ) {
+		setViewData( viewDocument, view );
+		viewDocument.render();
 
 
-			viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowleft, domTarget: viewDocument.domRoots.get( 'main' ) } );
+		const eventData = Object.assign( { keyCode: keyCodes.arrowright, domTarget: viewDocument.domRoots.get( 'main' ) }, options );
+		viewDocument.fire( 'keydown', eventData );
+	}
 
 
-			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();
+	function check( anchorNode, anchorOffset, focusNode, focusOffset ) {
+		const anchor = domSelection.anchorNode.data ? domSelection.anchorNode.data : domSelection.anchorNode.nodeName.toUpperCase();
 
 
-			viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowright, domTarget: viewDocument.domRoots.get( 'main' ) } );
-
-			const domSelection = document.getSelection();
+		expect( anchor, 'anchorNode' ).to.equal( anchorNode );
+		expect( domSelection.anchorOffset, 'anchorOffset' ).to.equal( anchorOffset );
 
 
-			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;
-		} );
+		if ( focusNode ) {
+			const focus = domSelection.focusNode.data ? domSelection.focusNode.data : domSelection.focusNode.nodeName.toUpperCase();
 
 
-		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();
+			expect( focus, 'focusNode' ).to.equal( focusNode );
+			expect( domSelection.focusOffset, 'focusOffset' ).to.equal( focusOffset );
+		} else {
+			expect( domSelection.isCollapsed, 'isCollapsed' ).to.be.true;
+		}
+	}
 
 
-			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;
+	describe( 'jump over ui element handler', () => {
+		describe( 'collapsed selection', () => {
+			it( 'do nothing when another key is pressed', () => {
+				prepare( '<container:p>foo<ui:span></ui:span>{}bar</container:p>', { keyCode: keyCodes.arrowleft } );
+				check( 'bar', 0 );
+			} );
+
+			it( 'jump over ui element when right arrow is pressed before ui element - directly before ui element', () => {
+				prepare( '<container:p>foo[]<ui:span></ui:span>bar</container:p>' );
+				check( 'P', 2 );
+			} );
+
+			it( 'jump over ui element when right arrow is pressed before ui element - not directly before ui element', () => {
+				prepare( '<container:p>foo{}<ui:span></ui:span>bar</container:p>' );
+				check( 'P', 2 );
+			} );
+
+			it( 'jump over multiple ui elements when right arrow is pressed before ui element', () => {
+				prepare( '<container:p>foo{}<ui:span></ui:span><ui:span></ui:span>bar</container:p>' );
+				check( 'P', 3 );
+			} );
+
+			it( 'jump over ui elements at the end of container element', () => {
+				prepare( '<container:p>foo{}<ui:span></ui:span><ui:span></ui:span></container:p><container:div></container:div>' );
+				check( 'P', 3 );
+			} );
+
+			it( 'jump over ui element if selection is in attribute element - case 1', () => {
+				prepare( '<container:p><attribute:b>foo{}</attribute:b><ui:span></ui:span>bar</container:p>' );
+				check( 'P', 2 );
+			} );
+
+			it( 'jump over ui element if selection is in attribute element - case 2', () => {
+				prepare( '<container:p><attribute:b>foo{}</attribute:b><ui:span></ui:span>bar</container:p>' );
+				check( 'P', 2 );
+			} );
+
+			it( 'jump over ui element if selection is in multiple attribute elements', () => {
+				prepare( '<container:p><attribute:i><attribute:b>foo{}</attribute:b></attribute:i><ui:span></ui:span>bar</container:p>' );
+				check( 'P', 2 );
+			} );
+
+			it( 'jump over empty attribute elements and ui elements', () => {
+				prepare(
+					'<container:p>' +
+						'foo{}<attribute:b></attribute:b><ui:span></ui:span><ui:span></ui:span><attribute:b></attribute:b>bar' +
+					'</container:p>'
+				);
+
+				check( 'P', 5 );
+			} );
+
+			it( 'jump over empty attribute elements and ui elements if shift key is pressed', () => {
+				prepare(
+					'<container:p>' +
+						'foo{}<attribute:b></attribute:b><ui:span></ui:span><ui:span></ui:span><attribute:b></attribute:b>bar' +
+					'</container:p>',
+					{ shiftKey: true }
+				);
+
+				check( 'P', 5 );
+			} );
+
+			it( 'do nothing if selection is not directly before ui element', () => {
+				prepare( '<container:p>fo{}o<ui:span></ui:span>bar</container:p>' );
+				check( 'foo', 2 );
+			} );
+
+			it( 'do nothing if selection is in attribute element but not before ui element', () => {
+				prepare( '<container:p><attribute:b>foo{}</attribute:b>bar</container:p>' );
+				check( 'foo', 3 );
+			} );
+
+			it( 'do nothing if selection is before non-empty attribute element', () => {
+				prepare( '<container:p>fo{}<attribute:b>o</attribute:b><ui:span></ui:span>bar</container:p>' );
+				check( 'fo', 2 );
+			} );
+
+			it( 'do nothing if selection is before container element - case 1', () => {
+				prepare( '<container:p>foo{}</container:p><ui:span></ui:span><container:div>bar</container:div>' );
+				check( 'foo', 3 );
+			} );
+
+			it( 'do nothing if selection is before container element - case 2', () => {
+				prepare( '<container:div>foo{}<container:p></container:p><ui:span></ui:span></container:div>' );
+				check( 'foo', 3 );
+			} );
+
+			it( 'do nothing if selection is at the end of last container element', () => {
+				prepare( '<container:p>foo{}</container:p>' );
+				check( 'foo', 3 );
+			} );
 		} );
 		} );
 
 
-		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;
+		describe( 'non-collapsed selection', () => {
+			it( 'should do nothing', () => {
+				prepare( '<container:p>f{oo}<ui:span></ui:span>bar</container:p>' );
+				check( 'foo', 1, 'foo', 3 );
+			} );
+
+			it( 'should do nothing if selection is not before ui element - shift key pressed', () => {
+				prepare( '<container:p>f{o}o<ui:span></ui:span>bar</container:p>', { shiftKey: true } );
+				check( 'foo', 1, 'foo', 2 );
+			} );
+
+			it( 'jump over ui element if shift key is pressed', () => {
+				prepare( '<container:p>fo{o}<ui:span></ui:span>bar</container:p>', { shiftKey: true } );
+				check( 'foo', 2, 'P', 2 );
+			} );
+
+			it( 'jump over ui element if selection is in multiple attribute elements', () => {
+				prepare(
+					'<container:p><attribute:i><attribute:b>fo{o}</attribute:b></attribute:i><ui:span></ui:span>bar</container:p>',
+					{ shiftKey: true }
+				);
+				check( 'foo', 2, 'P', 2 );
+			} );
+
+			it( 'jump over empty attribute elements and ui elements if shift key is pressed', () => {
+				prepare(
+					'<container:p>' +
+						'fo{o}<attribute:b></attribute:b><ui:span></ui:span><ui:span></ui:span><attribute:b></attribute:b>bar' +
+					'</container:p>',
+					{ shiftKey: true }
+				);
+
+				check( 'foo', 2, 'P', 5 );
+			} );
 		} );
 		} );
 
 
 		it( 'should do nothing if dom position cannot be converted to view position', () => {
 		it( 'should do nothing if dom position cannot be converted to view position', () => {

+ 27 - 2
packages/ckeditor5-engine/tests/view/manual/uielement.js

@@ -14,8 +14,10 @@ import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
 import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic';
 import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic';
 import Undo from '@ckeditor/ckeditor5-undo/src/undo';
 import Undo from '@ckeditor/ckeditor5-undo/src/undo';
 import UIElement from '../../../src/view/uielement';
 import UIElement from '../../../src/view/uielement';
+import Position from '../../../src/view/position';
+import writer from '../../../src/view/writer';
 
 
-class MyUIElement extends UIElement {
+class EndingUIElement extends UIElement {
 	render( domDocument ) {
 	render( domDocument ) {
 		const root = super.render( domDocument );
 		const root = super.render( domDocument );
 
 
@@ -26,6 +28,17 @@ class MyUIElement extends UIElement {
 	}
 	}
 }
 }
 
 
+class MiddleUIElement extends UIElement {
+	render( domDocument ) {
+		const root = super.render( domDocument );
+
+		root.classList.add( 'ui-element' );
+		root.innerHTML = 'X';
+
+		return root;
+	}
+}
+
 class UIElementTestPlugin extends Plugin {
 class UIElementTestPlugin extends Plugin {
 	init() {
 	init() {
 		const editor = this.editor;
 		const editor = this.editor;
@@ -34,7 +47,7 @@ class UIElementTestPlugin extends Plugin {
 		// Add some UIElement to each paragraph.
 		// Add some UIElement to each paragraph.
 		editing.modelToView.on( 'insert:paragraph', ( evt, data, consumable, conversionApi ) => {
 		editing.modelToView.on( 'insert:paragraph', ( evt, data, consumable, conversionApi ) => {
 			const viewP = conversionApi.mapper.toViewElement( data.item );
 			const viewP = conversionApi.mapper.toViewElement( data.item );
-			viewP.appendChildren( new MyUIElement( 'span' ) );
+			viewP.appendChildren( new EndingUIElement( 'span' ) );
 		}, { priority: 'lowest' } );
 		}, { priority: 'lowest' } );
 	}
 	}
 }
 }
@@ -46,6 +59,18 @@ ClassicEditor
 	} )
 	} )
 	.then( editor => {
 	.then( editor => {
 		window.editor = editor;
 		window.editor = editor;
+
+		// Add some UI elements.
+		const viewRoot = editor.editing.view.getRoot();
+		const viewText1 = viewRoot.getChild( 0 ).getChild( 0 );
+		const viewText2 = viewRoot.getChild( 1 ).getChild( 0 );
+
+		writer.insert( new Position( viewText1, 20 ), new MiddleUIElement( 'span' ) );
+		writer.insert( new Position( viewText1, 20 ), new MiddleUIElement( 'span' ) );
+		writer.insert( new Position( viewText2, 0 ), new MiddleUIElement( 'span' ) );
+		writer.insert( new Position( viewText2, 6 ), new MiddleUIElement( 'span' ) );
+
+		editor.editing.view.render();
 	} )
 	} )
 	.catch( err => {
 	.catch( err => {
 		console.error( err.stack );
 		console.error( err.stack );