Merge pull request #2440 from ckeditor/t/2411

Changing list type wont throw error for empty list elements
ckeditor · Jan 3, 2019 · 56c10f2 · 56c10f2
2 parents b864b71 + 2e00d1a
commit 56c10f2
Show file tree

Hide file tree

Showing 6 changed files with 639 additions and 4 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -14,6 +14,7 @@ Fixed Issues:
 * [#2650](https://github.com/ckeditor/ckeditor-dev/issues/2650): Fixed: [Table](https://ckeditor.com/cke4/addon/table) dialog validator fails when function `getValue` is defined in global scope.
 * [#2690](https://github.com/ckeditor/ckeditor-dev/issues/2690): Fixed: Decimal characters removed from inside ordered lists when pasting content using [Paste from Word](https://ckeditor.com/cke4/addon/pastefromword) plugin.
 * [#2205](https://github.com/ckeditor/ckeditor-dev/issues/2205): Fixed: It is not possible to add new list items under an item containing block element.
+* [#2411](https://github.com/ckeditor/ckeditor-dev/issues/2411), [#2438](https://github.com/ckeditor/ckeditor-dev/issues/2438) Fixed: Apply numbered list option throw console error for speciffic markup.
 
 Other Changes:
 

diff --git a/plugins/list/plugin.js b/plugins/list/plugin.js
@@ -592,7 +592,8 @@
 
 				while ( ( block = iterator.getNextParagraph() ) ) {
 					// Avoid duplicate blocks get processed across ranges.
-					if ( block.getCustomData( 'list_block' ) )
+					// Avoid processing comments, we don't want to touch it.
+					if ( block.getCustomData( 'list_block' ) || hasCommentsChildOnly( block ) )
 						continue;
 					else
 						CKEDITOR.dom.element.setMarker( database, block, 'list_block', 1 );
@@ -650,11 +651,14 @@
 			while ( listGroups.length > 0 ) {
 				groupObj = listGroups.shift();
 				if ( this.state == CKEDITOR.TRISTATE_OFF ) {
-					if ( listNodeNames[ groupObj.root.getName() ] )
+					if ( isEmptyList( groupObj ) ) {
+						continue;
+					} else if ( listNodeNames[ groupObj.root.getName() ] ) {
 						changeListType.call( this, editor, groupObj, database, listsCreated );
-					else
+					} else {
 						createList.call( this, editor, groupObj, listsCreated );
-				} else if ( this.state == CKEDITOR.TRISTATE_ON && listNodeNames[ groupObj.root.getName() ] ) {
+					}
+				} else if ( this.state == CKEDITOR.TRISTATE_ON && listNodeNames[ groupObj.root.getName() ] && !isEmptyList( groupObj ) ) {
 					removeList.call( this, editor, groupObj, database );
 				}
 			}
@@ -667,6 +671,31 @@
 			CKEDITOR.dom.element.clearAllMarkers( database );
 			selection.selectBookmarks( bookmarks );
 			editor.focus();
+
+			function isEmptyList( groupObj ) {
+				// If list is without any li item, then ignore such element from transformation, because it throws errors in console (#2411, #2438).
+				return listNodeNames[ groupObj.root.getName() ] && !getChildCount( groupObj.root, [ CKEDITOR.NODE_COMMENT ] );
+			}
+
+			function getChildCount( element, excludeTypes ) {
+				return CKEDITOR.tools.array.filter( element.getChildren().toArray(), function( node ) {
+					return CKEDITOR.tools.array.indexOf( excludeTypes, node.type ) === -1;
+				} ).length;
+			}
+
+			function hasCommentsChildOnly( element ) {
+				var ret = true;
+				if ( element.getChildCount() === 0 ) {
+					return false;
+				}
+				element.forEach( function( node ) {
+					if ( node.type !== CKEDITOR.NODE_COMMENT ) {
+						ret = false;
+						return false;
+					}
+				}, null, true );
+				return ret;
+			}
 		},
 
 		refresh: function( editor, path ) {