Browse Source

Merge branch 'master' into t/1186

Oskar Wróbel 8 years ago
parent
commit
f7e4f7903f

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

@@ -22,7 +22,7 @@
     "@ckeditor/ckeditor5-undo": "^1.0.0-alpha.2",
     "@ckeditor/ckeditor5-widget": "^1.0.0-alpha.2",
     "eslint": "^4.8.0",
-    "eslint-config-ckeditor5": "^1.0.6",
+    "eslint-config-ckeditor5": "^1.0.7",
     "husky": "^0.14.3",
     "lint-staged": "^4.2.3"
   },

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

@@ -18,7 +18,7 @@ import isIterable from '@ckeditor/ckeditor5-utils/src/isiterable';
  *
  * DocumentFragment has own {@link module:engine/model/markercollection~MarkerCollection}. Markers from this collection
  * will be set to the {@link module:engine/model/document~Document#markers document markers} by a
- * {@link module:engine/model/writer~writer.insert} function.
+ * {@linkTODO module:engine/model/writer~writer.insert} function.
  */
 export default class DocumentFragment {
 	/**

+ 2 - 0
packages/ckeditor5-engine/src/model/element.js

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

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

@@ -8,7 +8,7 @@
  */
 
 /**
- * Item is a {@link module:engine/model/node~Node Node} or {@link module:engine/model/textproxy~TextProxy TextProxy}.
+ * Item is a {@link module:engine/model/node~Node} or {@link module:engine/model/textproxy~TextProxy}.
  *
  * @typedef {module:engine/model/node~Node|module:engine/model/textproxy~TextProxy} module:engine/model/item~Item
  */

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

@@ -16,7 +16,7 @@ import NodeList from '../nodelist';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 /**
- * Contains functions used for composing model tree by {@link module:engine/model/operation~Operation operations}.
+ * Contains functions used for composing model tree by {@link module:engine/model/operation/operation~Operation operations}.
  * Those functions are built on top of {@link module:engine/model/node~Node node}, and it's child classes', APIs.
  *
  * @protected

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

@@ -15,10 +15,12 @@ import Node from './node';
  * **Important:** see {@link module:engine/model/node~Node} to read about restrictions using `Text` and `Node` API.
  *
  * **Note:** keep in mind that `Text` instances might indirectly got removed from model tree when model is changed.
- * This happens when {@link module:engine/model/writer~writer model writer} is used to change model and the text node is merged with
+ * This happens when {@linkTODO module:engine/model/writer~writer model writer} is used to change model and the text node is merged with
  * another text node. Then, both text nodes are removed and a new text node is inserted into the model. Because of
  * 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}
  */
 export default class Text extends Node {
 	/**

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

@@ -28,7 +28,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
  * parameter of methods.
  *
  * **Note:** `TextProxy` is a readonly interface. If you want to perform changes on model data represented by a `TextProxy`
- * use {@link module:engine/model/writer~writer model writer API}.
+ * use {@linkTODO module:engine/model/writer~writer model writer API}.
  *
  * **Note:** `TextProxy` instances are created on the fly, basing on the current state of model. Because of this, it is
  * highly unrecommended to store references to `TextProxy` instances. `TextProxy` instances are not refreshed when

+ 3 - 1
packages/ckeditor5-engine/src/view/domconverter.js

@@ -1061,7 +1061,9 @@ export default class DomConverter {
 
 		const direction = getNext ? 'nextNode' : 'previousNode';
 		const document = node.ownerDocument;
-		const treeWalker = document.createTreeWalker( document.childNodes[ 0 ], NodeFilter.SHOW_TEXT );
+		const topmostParent = getAncestors( node )[ 0 ];
+
+		const treeWalker = document.createTreeWalker( topmostParent, NodeFilter.SHOW_TEXT );
 
 		treeWalker.currentNode = node;
 

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

@@ -137,6 +137,8 @@ describe( 'EditingController', () => {
 			editing = new EditingController( model );
 
 			domRoot = document.createElement( 'div' );
+			domRoot.contentEditable = true;
+
 			document.body.appendChild( domRoot );
 			viewRoot = editing.createRoot( domRoot );
 
@@ -217,12 +219,15 @@ describe( 'EditingController', () => {
 					expect( getModelData( model ) ).to.equal(
 						'<paragraph>foo</paragraph>' +
 						'<paragraph></paragraph>' +
-						'<paragraph>b[a]r</paragraph>' );
+						'<paragraph>b[a]r</paragraph>'
+					);
+
 					done();
 				} );
 			} );
 
 			editing.view.isFocused = true;
+			editing.view.render();
 
 			const domSelection = document.getSelection();
 			domSelection.removeAllRanges();

+ 280 - 54
packages/ckeditor5-engine/tests/view/document/jumpoveruielement.js

@@ -6,12 +6,27 @@
 /* globals document */
 
 import ViewDocument from '../../../src/view/document';
+import UIElement from '../../../src/view/uielement';
+import ViewContainerElement from '../../../src/view/containerelement';
+import ViewAttribtueElement from '../../../src/view/attributeelement';
+import ViewText from '../../../src/view/text';
+import ViewRange from '../../../src/view/range';
 import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
 import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';
 import { setData as setViewData } from '../../../src/dev-utils/view';
+import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 
 describe( 'Document', () => {
-	let viewDocument, domRoot, domSelection;
+	let viewDocument, domRoot, domSelection, viewRoot, foo, bar, ui, ui2;
+
+	class MyUIElement extends UIElement {
+		render( domDocument ) {
+			const element = super.render( domDocument );
+			element.innerText = this.contents;
+
+			return element;
+		}
+	}
 
 	beforeEach( () => {
 		domRoot = createElement( document, 'div', {
@@ -20,12 +35,20 @@ describe( 'Document', () => {
 		document.body.appendChild( domRoot );
 
 		viewDocument = new ViewDocument();
-		viewDocument.createRoot( domRoot );
+		viewRoot = viewDocument.createRoot( domRoot );
 
 		domSelection = document.getSelection();
 		domSelection.removeAllRanges();
 
 		viewDocument.isFocused = true;
+
+		foo = new ViewText( 'foo' );
+		bar = new ViewText( 'bar' );
+		ui = new MyUIElement( 'span' );
+		ui.contents = 'xxx';
+
+		ui2 = new MyUIElement( 'span' );
+		ui2.contents = 'yyy';
 	} );
 
 	afterEach( () => {
@@ -34,8 +57,7 @@ describe( 'Document', () => {
 		domRoot.parentElement.removeChild( domRoot );
 	} );
 
-	function prepare( view, options ) {
-		setViewData( viewDocument, view );
+	function renderAndFireKeydownEvent( options ) {
 		viewDocument.render();
 
 		const eventData = Object.assign( { keyCode: keyCodes.arrowright, domTarget: viewDocument.domRoots.get( 'main' ) }, options );
@@ -61,130 +83,334 @@ describe( 'Document', () => {
 	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 );
+				// <container:p>foo<ui:span>xxx</ui:span>{}bar</container:p>
+				const p = new ViewContainerElement( 'p', null, [ foo, ui, bar ] );
+				viewRoot.appendChildren( p );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( bar, 0, bar, 0 ) ] );
+
+				renderAndFireKeydownEvent( { keyCode: keyCodes.arrowleft } );
+
+				testUtils.checkAssertions(
+					() => check( 'bar', 0 ),
+					// Safari renders selection at the end of the text node.
+					() => check( 'xxx', 3 )
+				);
 			} );
 
 			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 );
+				// <container:p>foo[]<ui:span>xxx</ui:span>bar</container:p>
+				const p = new ViewContainerElement( 'p', null, [ foo, ui, bar ] );
+				viewRoot.appendChildren( p );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( p, 1, p, 1 ) ] );
+
+				renderAndFireKeydownEvent();
+
+				testUtils.checkAssertions(
+					// <p>foo<span>xxx</span>[]bar</p>
+					() => check( 'P', 2 ),
+					// Safari renders selection at the end of the text node.
+					// <p>foo<span>xxx{}</span>bar</p>
+					() => check( 'xxx', 3 )
+				);
 			} );
 
 			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 );
+				// <container:p>foo{}<ui:span>xxx</ui:span>bar</container:p>
+				const p = new ViewContainerElement( 'p', null, [ foo, ui, bar ] );
+				viewRoot.appendChildren( p );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( foo, 3, foo, 3 ) ] );
+
+				renderAndFireKeydownEvent();
+
+				testUtils.checkAssertions(
+					// <p>foo<span>xxx</span>[]bar</p>
+					() => check( 'P', 2 ),
+					// Safari renders selection at the end of the text node.
+					// <p>foo<span>xxx{}</span>bar</p>
+					() => check( 'xxx', 3 )
+				);
 			} );
 
 			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 );
+				// <container:p>foo{}<ui:span>xxx</ui:span><ui:span>yyy</ui:span>bar</container:p>'
+				const p = new ViewContainerElement( 'p', null, [ foo, ui, ui2, bar ] );
+				viewRoot.appendChildren( p );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( foo, 3, foo, 3 ) ] );
+
+				renderAndFireKeydownEvent();
+
+				testUtils.checkAssertions(
+					// <p>foo<span>xxx</span><span>yyy</span>[]bar</p>
+					() => check( 'P', 3 ),
+					// Safari renders selection at the end of the text node.
+					// <p>foo<span>xxx</span><span>yyy{}</span>bar</p>
+					() => check( 'yyy', 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 );
+				// <container:p>foo{}<ui:span>xxx</ui:span><ui:span>yyy</ui:span></container:p><container:div></container:div>
+				const p = new ViewContainerElement( 'p', null, [ foo, ui, ui2 ] );
+				const div = new ViewContainerElement( 'div' );
+				viewRoot.appendChildren( p );
+				viewRoot.appendChildren( div );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( foo, 3, foo, 3 ) ] );
+
+				renderAndFireKeydownEvent();
+
+				testUtils.checkAssertions(
+					// <p>foo<span>xxx</span><span>yyy</span>[]</p><div></div>
+					() => check( 'P', 3 ),
+					// Safari renders selection at the end of the text node.
+					// <p>foo<span>xxx</span><span>yyy{}</span></p><div></div>
+					() => check( 'yyy', 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 );
+				// <container:p><attribute:b>foo{}</attribute:b><ui:span>xxx</ui:span>bar</container:p>
+				const b = new ViewAttribtueElement( 'b', null, foo );
+				const p = new ViewContainerElement( 'p', null, [ b, ui, bar ] );
+				viewRoot.appendChildren( p );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( foo, 3, foo, 3 ) ] );
+
+				renderAndFireKeydownEvent();
+
+				testUtils.checkAssertions(
+					// <p><b>foo</b><span>xxx</span>[]bar</p>
+					() => check( 'P', 2 ),
+					// Safari renders selection at the end of the text node.
+					// <p><b>foo</b><span>xxx{}</span>bar</p>
+					() => check( 'xxx', 3 )
+				);
 			} );
 
 			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 );
+				// <container:p><attribute:b>foo[]</attribute:b><ui:span>xxx</ui:span>bar</container:p>
+				const b = new ViewAttribtueElement( 'b', null, foo );
+				const p = new ViewContainerElement( 'p', null, [ b, ui, bar ] );
+				viewRoot.appendChildren( p );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( b, 1, b, 1 ) ] );
+
+				renderAndFireKeydownEvent();
+
+				testUtils.checkAssertions(
+					// <p><b>foo</b><span>xxx</span>[]bar</p>
+					() => check( 'P', 2 ),
+					// Safari renders selection at the end of the text node.
+					// <p><b>foo</b><span>xxx{}</span>bar</p>
+					() => check( 'xxx', 3 )
+				);
 			} );
 
 			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 );
+				// <container:p>
+				// 		<attribute:i>
+				// 			<attribute:b>foo{}</attribute:b>
+				// 		</attribute:i>
+				// 		<ui:span>
+				//			xxx
+				// 		</ui:span>
+				// 		bar
+				// </container:p>
+				const b = new ViewAttribtueElement( 'b', null, foo );
+				const i = new ViewAttribtueElement( 'i', null, b );
+				const p = new ViewContainerElement( 'p', null, [ i, ui, bar ] );
+
+				viewRoot.appendChildren( p );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( foo, 3, foo, 3 ) ] );
+
+				renderAndFireKeydownEvent();
+
+				testUtils.checkAssertions(
+					// <p><i><b>foo</b></i><span>xxx</span>[]bar</p>
+					() => check( 'P', 2 ),
+					// Safari renders selection at the end of the text node.
+					// <p><i><b>foo</b></i><span>xxx{}</span>bar</p>
+					() => check( 'xxx', 3 )
+				);
 			} );
 
 			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>'
+				// <container:p>' +
+				// 		foo{}
+				// 		<attribute:b></attribute:b>
+				// 		<ui:span>xxx</ui:span>
+				// 		<ui:span>yyy</ui:span>
+				// 		<attribute:b></attribute:b>
+				// 		bar
+				// </container:p>
+				const b1 = new ViewAttribtueElement( 'b' );
+				const b2 = new ViewAttribtueElement( 'b' );
+				const p = new ViewContainerElement( 'p', null, [ foo, b1, ui, ui2, b2, bar ] );
+
+				viewRoot.appendChildren( p );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( foo, 3, foo, 3 ) ] );
+
+				renderAndFireKeydownEvent();
+
+				testUtils.checkAssertions(
+					// <p>foo<b></b><span>xxx</span><span>yyy</span>[]bar</p>
+					() => check( 'P', 5 ),
+					// Safari renders selection at the end of the text node.
+					// <p>foo<b></b><span>xxx</span><span>yyy{}</span>bar</p>
+					() => check( 'yyy', 3 )
 				);
-
-				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 }
+				// <container:p>
+				// 		foo{}
+				// 		<attribute:b></attribute:b>
+				// 		<ui:span>xxx</ui:span>
+				// 		<ui:span>yyy</ui:span>
+				// 		<attribute:b></attribute:b>
+				// 		bar
+				// </container:p>
+
+				const b1 = new ViewAttribtueElement( 'b' );
+				const b2 = new ViewAttribtueElement( 'b' );
+				const p = new ViewContainerElement( 'p', null, [ foo, b1, ui, ui2, b2, bar ] );
+
+				viewRoot.appendChildren( p );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( foo, 3, foo, 3 ) ] );
+
+				renderAndFireKeydownEvent( { shiftKey: true } );
+
+				testUtils.checkAssertions(
+					// <p>foo<b></b><span>xxx</span><span>yyy</span><b><b>[]bar</p>
+					() => check( 'P', 5 ),
+					// Safari renders selection at the end of the text node.
+					// <p>foo<b></b><span>xxx</span><span>yyy{}</span><b><b>bar</p>
+					() => check( 'yyy', 3 )
 				);
-
-				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>' );
+				setViewData( viewDocument, '<container:p>fo{}o<ui:span></ui:span>bar</container:p>' );
+				renderAndFireKeydownEvent();
+
 				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>' );
+				setViewData( viewDocument, '<container:p><attribute:b>foo{}</attribute:b>bar</container:p>' );
+				renderAndFireKeydownEvent();
+
 				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>' );
+				setViewData( viewDocument, '<container:p>fo{}<attribute:b>o</attribute:b><ui:span></ui:span>bar</container:p>' );
+				renderAndFireKeydownEvent();
+
 				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>' );
+				setViewData( viewDocument, '<container:p>foo{}</container:p><ui:span></ui:span><container:div>bar</container:div>' );
+				renderAndFireKeydownEvent();
+
 				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>' );
+				setViewData( viewDocument, '<container:div>foo{}<container:p></container:p><ui:span></ui:span></container:div>' );
+				renderAndFireKeydownEvent();
+
 				check( 'foo', 3 );
 			} );
 
 			it( 'do nothing if selection is at the end of last container element', () => {
-				prepare( '<container:p>foo{}</container:p>' );
+				setViewData( viewDocument, '<container:p>foo{}</container:p>' );
+				renderAndFireKeydownEvent();
+
 				check( 'foo', 3 );
 			} );
 		} );
 
 		describe( 'non-collapsed selection', () => {
 			it( 'should do nothing', () => {
-				prepare( '<container:p>f{oo}<ui:span></ui:span>bar</container:p>' );
+				setViewData( viewDocument, '<container:p>f{oo}<ui:span></ui:span>bar</container:p>' );
+				renderAndFireKeydownEvent();
+
 				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 } );
+				setViewData( viewDocument, '<container:p>f{o}o<ui:span></ui:span>bar</container:p>' );
+				renderAndFireKeydownEvent( { 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 );
+				// <container:p>fo{o}<ui:span>xxx</ui:span>bar</container:p>
+				const p = new ViewContainerElement( 'p', null, [ foo, ui, bar ] );
+
+				viewRoot.appendChildren( p );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( foo, 2, foo, 3 ) ] );
+
+				renderAndFireKeydownEvent( { shiftKey: true } );
+
+				testUtils.checkAssertions(
+					// <p>fo{o<span>xxx</span>]bar</p>
+					() => check( 'foo', 2, 'P', 2 ),
+					// Safari renders selection at the end of the previous text node.
+					// <p>fo{o<span>xxx}</span>bar</p>
+					() => check( 'foo', 2, 'xxx', 3 )
+				);
 			} );
 
 			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 }
+				// <container:p>
+				// 		<attribute:i>
+				// 			<attribute:b>fo{o}</attribute:b>
+				// 		</attribute:i>
+				// 		<ui:span>xxx</ui:span>
+				// 		bar
+				// </container:p>
+				const b = new ViewAttribtueElement( 'b', null, foo );
+				const i = new ViewAttribtueElement( 'i', null, b );
+				const p = new ViewContainerElement( 'p', null, [ i, ui, bar ] );
+				viewRoot.appendChildren( p );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( foo, 2, foo, 3 ) ] );
+
+				renderAndFireKeydownEvent( { shiftKey: true } );
+
+				testUtils.checkAssertions(
+					// <p><i><b>fo{o</b></i><span>xxx</span>]bar</p>
+					() => check( 'foo', 2, 'P', 2 ),
+					// Safari renders selection at the end of the previous text node.
+					// <p><i><b>fo{o</b></i><span>xxx}</span>bar</p>
+					() => check( 'foo', 2, 'xxx', 3 )
 				);
-				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 }
+				// <container:p>
+				// 		fo{o}
+				// 		<attribute:b></attribute:b>
+				// 		<ui:span>xxx</ui:span>
+				// 		<ui:span>yyy</ui:span>
+				// 		<attribute:b></attribute:b>
+				// 		bar
+				// </container:p>
+				const b1 = new ViewAttribtueElement( 'b' );
+				const b2 = new ViewAttribtueElement( 'b' );
+				const p = new ViewContainerElement( 'p', null, [ foo, b1, ui, ui2, b2, bar ] );
+				viewRoot.appendChildren( p );
+				viewDocument.selection.setRanges( [ ViewRange.createFromParentsAndOffsets( foo, 2, foo, 3 ) ] );
+
+				renderAndFireKeydownEvent( { shiftKey: true } );
+
+				testUtils.checkAssertions(
+					// <p>fo{o<b></b><span>xxx</span><span>yyy</span><b></b>]bar</p>
+					() => check( 'foo', 2, 'P', 5 ),
+					// Safari renders selection at the end of the previous text node.
+					// <p>fo{o<b></b><span>xxx</span><span>yyy}</span><b></b>bar</p>
+					() => check( 'foo', 2, 'yyy', 3 )
 				);
-
-				check( 'foo', 2, 'P', 5 );
 			} );
 		} );
 

+ 18 - 6
packages/ckeditor5-engine/tests/view/domconverter/domconverter.js

@@ -51,8 +51,11 @@ describe( 'DomConverter', () => {
 		} );
 
 		afterEach( () => {
+			converter.unbindDomElement( domEditable );
 			document.body.removeChild( domEditableParent );
 			viewDocument.destroy();
+
+			document.body.focus();
 		} );
 
 		it( 'should call focus on corresponding DOM editable', () => {
@@ -226,6 +229,10 @@ describe( 'DomConverter', () => {
 			document.body.appendChild( domP );
 		} );
 
+		afterEach( () => {
+			domP.remove();
+		} );
+
 		it( 'should return true for correct dom selection', () => {
 			// <p>INLINE_FILLER{foo}<span></span></p>.
 			const sel1 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
@@ -272,22 +279,27 @@ describe( 'DomConverter', () => {
 			} );
 
 			it( 'if anchor or focus is directly inside dom element that represents view ui element', () => {
+				// Set text indside ui element to put selection there.
+				domUiSpan.innerText = 'xxx';
 				// Tests forward and backward selection.
-				// <p>INLINE_FILLER{foo<span-ui>]<span-container></span></span></p>.
-				const sel1 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH + 3, domUiSpan, 0 );
+				// <p>INLINE_FILLER{foo<span-ui>xxx]<span-container></span></span></p>.
+				const sel1 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH, domUiSpan, 1 );
+
 				expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.false;
 
-				const sel2 = domSelection( domUiSpan, 0, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
+				const sel2 = domSelection( domUiSpan, 1, domFillerTextNode, INLINE_FILLER_LENGTH );
 				expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.false;
 			} );
 
 			it( 'if anchor or focus is inside deep ui element structure (not directly in ui element)', () => {
+				// Set text indside ui element to put selection there.
+				domUiDeepSpan.innerText = 'xxx';
 				// Tests forward and backward selection.
-				// <p>INLINE_FILLER{foo<span-ui><span-container>]</span></span></p>.
-				const sel1 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH + 3, domUiDeepSpan, 0 );
+				// <p>INLINE_FILLER{foo<span-ui><span-container>xxx]</span></span></p>.
+				const sel1 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH, domUiDeepSpan, 1 );
 				expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.false;
 
-				const sel2 = domSelection( domUiDeepSpan, 0, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
+				const sel2 = domSelection( domUiDeepSpan, 1, domFillerTextNode, INLINE_FILLER_LENGTH );
 				expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.false;
 			} );
 		} );

+ 7 - 7
packages/ckeditor5-engine/tests/view/observer/focusobserver.js

@@ -3,7 +3,7 @@
  * For licensing, see LICENSE.md.
  */
 
-/* globals document, window */
+/* globals document */
 import FocusObserver from '../../../src/view/observer/focusobserver';
 import ViewDocument from '../../../src/view/document';
 import ViewRange from '../../../src/view/range';
@@ -142,7 +142,7 @@ describe( 'FocusObserver', () => {
 	} );
 
 	describe( 'integration test', () => {
-		let viewDocument, domRoot, observer, domSelection;
+		let viewDocument, domRoot, observer;
 
 		beforeEach( () => {
 			domRoot = document.createElement( 'div' );
@@ -152,16 +152,14 @@ describe( 'FocusObserver', () => {
 			viewDocument.createRoot( domRoot );
 
 			observer = viewDocument.getObserver( FocusObserver );
-			domSelection = window.getSelection();
 		} );
 
-		it( 'should render document after selectionChange event', done => {
+		it( 'should always render document after selectionChange event', done => {
 			const selectionChangeSpy = sinon.spy();
 			const renderSpy = sinon.spy();
 
 			setData( viewDocument, '<div contenteditable="true">foo bar</div>' );
 			viewDocument.render();
-			const domEditable = domRoot.childNodes[ 0 ];
 
 			viewDocument.on( 'selectionChange', selectionChangeSpy );
 			viewDocument.on( 'render', renderSpy, { priority: 'low' } );
@@ -171,8 +169,10 @@ describe( 'FocusObserver', () => {
 				done();
 			}, { priority: 'low' } );
 
-			observer.onDomEvent( { type: 'focus', target: domEditable } );
-			domSelection.collapse( domEditable, 0 );
+			// Mock selectionchange event after focus event. Render called by focus observer should be fired after
+			// async selection change.
+			viewDocument.fire( 'focus' );
+			viewDocument.fire( 'selectionChange' );
 		} );
 
 		it( 'should render without selectionChange event', done => {

+ 19 - 9
packages/ckeditor5-engine/tests/view/observer/selectionobserver.js

@@ -43,6 +43,7 @@ describe( 'SelectionObserver', () => {
 		domDocument.getSelection().removeAllRanges();
 
 		viewDocument.isFocused = true;
+		domMain.focus();
 
 		selectionObserver.enable();
 
@@ -147,7 +148,7 @@ describe( 'SelectionObserver', () => {
 		viewDocument.on( 'selectionChange', spy );
 
 		setTimeout( () => {
-			sinon.assert.calledOnce( spy );
+			sinon.assert.called( spy );
 			done();
 		}, 70 );
 
@@ -155,8 +156,7 @@ describe( 'SelectionObserver', () => {
 	} );
 
 	it( 'should warn and not enter infinite loop', () => {
-		// Selectionchange event is called twice per `changeDomSelection()` execution.
-		let counter = 35;
+		let counter = 70;
 
 		const viewFoo = viewDocument.getRoot().getChild( 0 ).getChild( 0 );
 		viewDocument.selection.addRange( ViewRange.createFromParentsAndOffsets( viewFoo, 0, viewFoo, 0 ) );
@@ -215,8 +215,6 @@ describe( 'SelectionObserver', () => {
 				clock.restore();
 			} );
 
-		// Selectionchange event is called twice per `changeDomSelection()` execution. We call it 25 times to get
-		// 50 events. Infinite loop counter is reset, so calling this method twice should not show any warning.
 		function doChanges() {
 			return new Promise( resolve => {
 				viewDocument.once( 'selectionChangeDone', () => {
@@ -224,7 +222,7 @@ describe( 'SelectionObserver', () => {
 					resolve();
 				} );
 
-				for ( let i = 0; i < 30; i++ ) {
+				for ( let i = 0; i < 50; i++ ) {
 					changeDomSelection();
 				}
 			} );
@@ -305,6 +303,9 @@ describe( 'SelectionObserver', () => {
 
 	it( 'should re-render view if selections are similar if DOM selection is in incorrect place', done => {
 		const sel = domDocument.getSelection();
+		const domParagraph = domMain.childNodes[ 0 ];
+		const domText = domParagraph.childNodes[ 0 ];
+		const domUI = domParagraph.childNodes[ 1 ];
 
 		// Add rendering on selectionChange event to check this feature.
 		viewDocument.on( 'selectionChange', () => {
@@ -334,12 +335,22 @@ describe( 'SelectionObserver', () => {
 			// 3. Now, collapse selection in similar position, but in UI element.
 			// Current and new selection position are similar in view (but not equal!).
 			// Also add a spy to `viewDocument#render` to see if view will be re-rendered.
-			sel.collapse( domMain.childNodes[ 0 ].childNodes[ 1 ], 0 );
+			sel.collapse( domUI, 0 );
 			sinon.spy( viewDocument, 'render' );
+
+			// Some browsers like Safari won't allow to put selection inside empty ui element.
+			// In that situation selection should stay in correct place.
+			if ( sel.anchorNode !== domUI ) {
+				expect( sel.anchorNode ).to.equal( domText );
+				expect( sel.anchorOffset ).to.equal( 3 );
+				expect( sel.isCollapsed ).to.be.true;
+
+				done();
+			}
 		}, { priority: 'lowest' } );
 
 		// 1. Collapse in a text node, before ui element, and wait for async selectionchange to fire selection change handling.
-		sel.collapse( domMain.childNodes[ 0 ].childNodes[ 0 ], 3 );
+		sel.collapse( domText, 3 );
 	} );
 
 	function changeDomSelection() {
@@ -347,7 +358,6 @@ describe( 'SelectionObserver', () => {
 		const domFoo = domMain.childNodes[ 1 ].childNodes[ 0 ];
 		const offset = domSelection.anchorOffset;
 
-		domSelection.removeAllRanges();
 		domSelection.collapse( domFoo, offset == 2 ? 3 : 2 );
 	}
 } );

+ 75 - 19
packages/ckeditor5-engine/tests/view/renderer.js

@@ -136,6 +136,10 @@ describe( 'Renderer', () => {
 			} );
 		} );
 
+		afterEach( () => {
+			domRoot.remove();
+		} );
+
 		it( 'should update attributes', () => {
 			viewRoot.setAttribute( 'class', 'foo' );
 
@@ -1045,7 +1049,6 @@ describe( 'Renderer', () => {
 			renderer.render();
 
 			// 2. Check the DOM.
-
 			const domP = domRoot.childNodes[ 0 ];
 
 			expect( domP.childNodes.length ).to.equal( 2 );
@@ -1063,6 +1066,7 @@ describe( 'Renderer', () => {
 			expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( INLINE_FILLER_LENGTH );
 			expect( domSelection.getRangeAt( 0 ).collapsed ).to.be.true;
 
+			domSelection.removeAllRanges();
 			// 3. Add text node only to the view: <p><b>x{}</b>foo</p>.
 
 			const viewText = new ViewText( 'x' );
@@ -1077,9 +1081,21 @@ describe( 'Renderer', () => {
 			expect( domB.childNodes[ 0 ].data ).to.equal( INLINE_FILLER + 'x' );
 
 			expect( domSelection.rangeCount ).to.equal( 1 );
-			expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domB.childNodes[ 0 ] );
-			expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( INLINE_FILLER_LENGTH + 1 );
-			expect( domSelection.getRangeAt( 0 ).collapsed ).to.be.true;
+
+			// Depending on the browser selection may end up at the end of the text node or after the text node.
+			const firstRange = domSelection.getRangeAt( 0 );
+
+			const assertSelectionAtEndOfTextNode = () => {
+				expect( firstRange.startOffset ).to.equal( INLINE_FILLER_LENGTH + 1 );
+			};
+
+			const assertSelectionInsideTextNode = () => {
+				expect( firstRange.startContainer ).to.equal( domB );
+				expect( firstRange.startOffset ).to.equal( 1 );
+			};
+
+			testUtils.checkAssertions( assertSelectionAtEndOfTextNode, assertSelectionInsideTextNode );
+			expect( firstRange.collapsed ).to.be.true;
 		} );
 
 		it( 'should handle typing in empty attribute as a text change, render if needed', () => {
@@ -1167,16 +1183,14 @@ describe( 'Renderer', () => {
 		} );
 
 		it( 'should not change selection if there is no editable with selection', () => {
-			const domDiv = createElement( document, 'div', null, 'not editable' );
+			const domDiv = createElement( document, 'div', { contenteditable: true }, 'not editable' );
 			document.body.appendChild( domDiv );
+			domDiv.focus();
 
 			const domSelection = document.getSelection();
 
 			domSelection.removeAllRanges();
-			const domRange = document.createRange();
-			domRange.setStart( domDiv, 0 );
-			domRange.collapse( true );
-			domSelection.addRange( domRange );
+			domSelection.collapse( domDiv, 0 );
 
 			selectionEditable = null;
 
@@ -1188,9 +1202,24 @@ describe( 'Renderer', () => {
 			renderer.render();
 
 			expect( domSelection.rangeCount ).to.equal( 1 );
-			expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domDiv );
-			expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( 0 );
-			expect( domSelection.getRangeAt( 0 ).collapsed ).to.equal( true );
+
+			// Depending on the browser selection may end up before the text node or at the beginning of it.
+			const domRange = domSelection.getRangeAt( 0 );
+
+			const assertSelectionAtEndOfTextNode = () => {
+				expect( domRange.startContainer ).to.equal( domDiv );
+			};
+
+			const assertSelectionInsideTextNode = () => {
+				expect( domRange.startContainer ).to.equal( domDiv.childNodes[ 0 ] );
+			};
+
+			testUtils.checkAssertions( assertSelectionAtEndOfTextNode, assertSelectionInsideTextNode );
+
+			expect( domRange.startOffset ).to.equal( 0 );
+			expect( domRange.collapsed ).to.be.true;
+
+			domDiv.remove();
 		} );
 
 		it( 'should not change selection if there is no focus', () => {
@@ -1215,9 +1244,24 @@ describe( 'Renderer', () => {
 			renderer.render();
 
 			expect( domSelection.rangeCount ).to.equal( 1 );
-			expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domDiv );
-			expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( 0 );
-			expect( domSelection.getRangeAt( 0 ).collapsed ).to.equal( true );
+
+			// Depending on the browser selection may end up before the text node or at the beginning of it.
+			const domSelectionRange = domSelection.getRangeAt( 0 );
+
+			const assertSelectionAtEndOfTextNode = () => {
+				expect( domSelectionRange.startContainer ).to.equal( domDiv );
+			};
+
+			const assertSelectionInsideTextNode = () => {
+				expect( domSelectionRange.startContainer ).to.equal( domDiv.childNodes[ 0 ] );
+			};
+
+			testUtils.checkAssertions( assertSelectionAtEndOfTextNode, assertSelectionInsideTextNode );
+
+			expect( domSelectionRange.startOffset ).to.equal( 0 );
+			expect( domSelectionRange.collapsed ).to.be.true;
+
+			domDiv.remove();
 		} );
 
 		it( 'should not add inline filler after text node', () => {
@@ -1658,10 +1702,22 @@ describe( 'Renderer', () => {
 				renderer.render();
 
 				// Expect that after calling `renderer.render()` the DOM selection was re-rendered (and set at correct position).
-				expect( domSelection.anchorNode ).to.equal( domP );
-				expect( domSelection.anchorOffset ).to.equal( 1 );
-				expect( domSelection.focusNode ).to.equal( domP );
-				expect( domSelection.focusOffset ).to.equal( 1 );
+				// Depending on the browser selection may end up at the end of the text node or after the text node.
+
+				const assertSelectionAtEndOfTextNode = () => {
+					expect( domSelection.anchorNode ).to.equal( domP );
+					expect( domSelection.anchorOffset ).to.equal( 1 );
+				};
+
+				const assertSelectionInsideTextNode = () => {
+					const textNode = domP.childNodes[ 0 ];
+					expect( domSelection.anchorNode ).to.equal( textNode );
+					expect( domSelection.anchorOffset ).to.equal( 3 );
+				};
+
+				testUtils.checkAssertions( assertSelectionAtEndOfTextNode, assertSelectionInsideTextNode );
+
+				expect( domSelection.getRangeAt( 0 ).collapsed ).to.be.true;
 			} );
 
 			it( 'should not render non-collapsed selection it is similar (element start)', () => {