Merge pull request #451 from cksource/t/17028

[#17028] Tabbing through inline editor causes JavaScript error.

fixes #424

CHANGES.md

@@ -15,6 +15,7 @@ Fixed Issues:
 * [#450](https://github.com/ckeditor/ckeditor-dev/issues/450): Fixed: [`CKEDITOR.filter`](http://docs.ckeditor.com/#!/api/CKEDITOR.filter) incorrectly transforms `margin` CSS property.
 * [#478](https://github.com/ckeditor/ckeditor-dev/issues/478): Fixed: [Chrome] Error thrown by [Enter Key](http://ckeditor.com/addon/enterkey) plugin when pressing enter with no selection.
 * [#417](https://github.com/ckeditor/ckeditor-dev/issues/417): Fixed: [Table Resize](http://ckeditor.com/addon/tableresize) plugin throws an error when used with a table with only header or footer rows.
+* [#424](https://github.com/ckeditor/ckeditor-dev/issues/424): Fixed: Error thrown by [Tab Key Handling](http://ckeditor.com/addon/tab) and [Indent List](http://ckeditor.com/addon/indentlist) when pressing `Tab` with no selection in inline editor.
 
 ## CKEditor 4.7
 

core/dom/elementpath.js

@@ -57,6 +57,11 @@
 		// Backward compact.
 		root = root || startNode.getDocument().getBody();
 
+		// Assign root value if startNode is null (#424)(https://dev.ckeditor.com/ticket/17028).
+		if ( !e ) {
+			e = root;
+		}
+
 		do {
 			if ( e.type == CKEDITOR.NODE_ELEMENT ) {
 				elements.push( e );

core/focusmanager.js

@@ -152,18 +152,9 @@
 			}
 
 			function doBlur() {
-				var editor = this._.editor;
-
 				if ( this.hasFocus ) {
 					this.hasFocus = false;
 
-					// Blink browsers leave selection in `[contenteditable=true]`
-					// when it's blurred and it's necessary to remove it manually for inline editor. (http://dev.ckeditor.com/ticket/13446)
-					// It seems to be related to https://bugs.chromium.org/p/chromium/issues/detail?id=433303.
-					if ( CKEDITOR.env.chrome && editor.elementMode == CKEDITOR.ELEMENT_MODE_INLINE ) {
-						editor.window.$.getSelection().removeAllRanges();
-					}
-
 					var ct = this._.editor.container;
 					ct && ct.removeClass( 'cke_focus' );
 					this._.editor.fire( 'blur' );

plugins/indentlist/plugin.js

@@ -35,17 +35,24 @@
 				// Indent and outdent lists with TAB/SHIFT+TAB key. Indenting can
 				// be done for any list item that isn't the first child of the parent.
 				editor.on( 'key', function( evt ) {
+					var path = editor.elementPath();
+
 					if ( editor.mode != 'wysiwyg' )
 						return;
 
 					if ( evt.data.keyCode == this.indentKey ) {
-						var list = this.getContext( editor.elementPath() );
+						// Prevent of getting context of empty path (#424)(https://dev.ckeditor.com/ticket/17028).
+						if ( !path ) {
+							return;
+						}
+
+						var list = this.getContext( path );
 
 						if ( list ) {
 							// Don't indent if in first list item of the parent.
 							// Outdent, however, can always be done to collapse
 							// the list into a paragraph (div).
-							if ( this.isIndent && CKEDITOR.plugins.indentList.firstItemInPath( this.context, editor.elementPath(), list ) )
+							if ( this.isIndent && CKEDITOR.plugins.indentList.firstItemInPath( this.context, path, list ) )
 								return;
 
 							// Exec related global indentation command. Global
@@ -109,7 +116,8 @@
 	function indentList( editor ) {
 		var that = this,
 			database = this.database,
-			context = this.context;
+			context = this.context,
+			range;
 
 		function indent( listNode ) {
 			// Our starting and ending points of the range might be inside some blocks under a list item...
@@ -226,8 +234,7 @@
 
 		var selection = editor.getSelection(),
 			ranges = selection && selection.getRanges(),
-			iterator = ranges.createIterator(),
-			range;
+			iterator = ranges.createIterator();
 
 		while ( ( range = iterator.getNextRange() ) ) {
 			var nearestListBlock = range.getCommonAncestor();

tests/core/focusmanager/editor.js

@@ -58,7 +58,7 @@ bender.test( {
 		}, 100 );
 	},
 
-	// http://dev.ckeditor.com/ticket/11647
+	// https://dev.ckeditor.com/ticket/11647
 	'test inheriting the initial focus': function() {
 		var el = CKEDITOR.document.createElement( 'div' );
 		CKEDITOR.document.getBody().append( el );
@@ -91,53 +91,6 @@ bender.test( {
 		} );
 
 		wait();
-	},
-
-	// http://dev.ckeditor.com/ticket/13446
-	'test cleaning up selection on blur (inline editor)': function() {
-		if ( !CKEDITOR.env.chrome ) {
-			assert.ignore();
-		}
-
-		var el = CKEDITOR.document.createElement( 'div' );
-		CKEDITOR.document.getBody().append( el );
-		el.setAttribute( 'contenteditable', true );
-
-		var editor = CKEDITOR.inline( el );
-
-		editor.on( 'instanceReady', function() {
-			resume( function() {
-				editor.on( 'blur', function() {
-					resume( function() {
-						assert.areSame( 'None', editor.getSelection().getNative().type );
-					} );
-				} );
-
-				CKEDITOR.document.getById( 'focusable' ).focus();
-				wait();
-			} );
-		} );
-
-		el.focus();
-		wait();
-	},
-
-	// http://dev.ckeditor.com/ticket/13446
-	'test preserving selection on blur (classic editor)': function() {
-		if ( !CKEDITOR.env.chrome ) {
-			assert.ignore();
-		}
-
-		var editor = this.editor;
-
-		editor.focus();
-
-		bender.tools.focus( CKEDITOR.document.getById( 'focusable' ), function() {
-			// Chrome doesn't reset selection after blurring [contenteditable=true],
-			// however is not troublesome for classic editor (because the focus is moved
-			// outside the whole editor's window), so it should be left untouched there.
-			assert.areSame( 'Caret', editor.getSelection().getNative().type );
-		} );
 	}
 
 } );

tests/plugins/indentlist/_assets/styles.css

@@ -0,0 +1,5 @@
+.inline-editor {
+	background-color: #ccffcc;
+	margin: 20px;
+	padding: 10px;
+}

tests/plugins/indentlist/manual/taberrorininlineeditor.html

@@ -0,0 +1,40 @@
+<head>
+	<link rel="stylesheet" href="../_assets/styles.css">
+</head>
+<body>
+	<script>
+		if ( !CKEDITOR.env.chrome ) {
+			bender.ignore();
+		}
+	</script>
+	<div class="inline-editor" id="inline1" contenteditable="true">
+		<p>Hello world</p>
+		<ul>
+			<li>One</li>
+			<li>Two</li>
+			<li>Three</li>
+		</ul>
+	</div>
+	<div class="inline-editor" id="inline2" contenteditable="true">
+		<p>Hello world</p>
+		<ul>
+			<li>One</li>
+			<li>Two</li>
+			<li>Three</li>
+		</ul>
+	</div>
+	<div class="inline-editor" id="inline3" contenteditable="true">
+		<p>Hello world</p>
+		<ul>
+			<li>One</li>
+			<li>Two</li>
+			<li>Three</li>
+		</ul>
+	</div>
+	<script>
+		CKEDITOR.inline( 'inline1' );
+		CKEDITOR.inline( 'inline2' );
+		CKEDITOR.inline( 'inline3' );
+	</script>
+</body>
+

tests/plugins/indentlist/manual/taberrorininlineeditor.md

@@ -0,0 +1,12 @@
+@bender-tags: 4.7.1, 424, bug
+@bender-ui: collapsed
+@bender-ckeditor-plugins: floatingspace, toolbar, list, indentlist
+
+----
+
+1. Open console.
+1. Click into 1st `Hello world`.
+1. Press `TAB` few times to cycle through other editors.
+1. There should be no error in the console.
+
+**Unexpected:** Error with status: `Uncaught TypeError: Cannot read property 'contains' of null`

tests/plugins/indentlist/taberror.js

@@ -0,0 +1,31 @@
+/* bender-tags: editor,unit */
+/* bender-ckeditor-plugins: toolbar,indentlist */
+/* bender-ui: collapsed */
+
+( function() {
+	'use strict';
+
+	bender.editor = {
+		creator: 'inline'
+	};
+
+	// #424 - https://dev.ckeditor.com/ticket/17028
+	bender.test( {
+		'test tab in inline editor': function() {
+			var editor = this.editor;
+
+			editor.focus();
+			var sel = editor.getSelection();
+			sel.removeAllRanges();
+			editor.fire( 'key', {
+				domEvent: {
+					getKey: function() {
+						return 9;
+					}
+				},
+				keyCode: 9
+			} );
+			assert.pass();
+		}
+	} );
+} )();

tests/plugins/tab/_assets/styles.css

@@ -0,0 +1,5 @@
+.inline-editor {
+	background-color: #ccffcc;
+	margin: 20px;
+	padding: 10px;
+}

tests/plugins/tab/errorinlineeditor.js

@@ -0,0 +1,31 @@
+/* bender-tags: editor,unit */
+/* bender-ckeditor-plugins: toolbar,tab */
+/* bender-ui: collapsed */
+
+( function() {
+	'use strict';
+
+	bender.editor = {
+		creator: 'inline'
+	};
+
+	// #424 - https://dev.ckeditor.com/ticket/17028
+	bender.test( {
+		'test tab in inline editor': function() {
+			var editor = this.editor;
+
+			editor.focus();
+			var sel = editor.getSelection();
+			sel.removeAllRanges();
+			editor.fire( 'key', {
+				domEvent: {
+					getKey: function() {
+						return 9;
+					}
+				},
+				keyCode: 9
+			} );
+			assert.pass();
+		}
+	} );
+} )();

tests/plugins/tab/manual/taberrorininlineeditor.html

@@ -0,0 +1,40 @@
+<head>
+	<link rel="stylesheet" href="../_assets/styles.css">
+</head>
+<body>
+	<script>
+		if ( !CKEDITOR.env.chrome ) {
+			bender.ignore();
+		}
+	</script>
+	<div class="inline-editor" id="inline1" contenteditable="true">
+		<p>Hello world</p>
+		<ul>
+			<li>One</li>
+			<li>Two</li>
+			<li>Three</li>
+		</ul>
+	</div>
+	<div class="inline-editor" id="inline2" contenteditable="true">
+		<p>Hello world</p>
+		<ul>
+			<li>One</li>
+			<li>Two</li>
+			<li>Three</li>
+		</ul>
+	</div>
+	<div class="inline-editor" id="inline3" contenteditable="true">
+		<p>Hello world</p>
+		<ul>
+			<li>One</li>
+			<li>Two</li>
+			<li>Three</li>
+		</ul>
+	</div>
+	<script>
+		CKEDITOR.inline( 'inline1' );
+		CKEDITOR.inline( 'inline2' );
+		CKEDITOR.inline( 'inline3' );
+	</script>
+</body>
+

tests/plugins/tab/manual/taberrorininlineeditor.md

@@ -0,0 +1,12 @@
+@bender-tags: 4.7.1, 424, bug
+@bender-ui: collapsed
+@bender-ckeditor-plugins: floatingspace, toolbar, list, tab
+
+----
+
+1. Open console.
+1. Click into 1st `Hello world`.
+1. Press `TAB` few times to cycle through other editors.
+1. There should be no error in the console.
+
+**Unexpected:** Error with status: `Uncaught TypeError: Cannot read property 'contains' of null`