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

Merge pull request #8075 from ckeditor/i/8026

Fix (list): List styles plugin will no longer cause the editor to crash when indenting a list item that is the last element in the editor. Closes #8072.

Fix (list): Undo will restore the proper value of the list-style-type attribute in the View element after undoing merging different lists. Closes #7930.

Fix (list): Fixed a bug that prevented using the same value of the List style feature for nested lists. Closes #8081.

Internal (list): The mechanism that merges two lists and inherits their list styles will not crash while deleting content (lists with different values of the listIndent attribute) in a nested list. Closes #8073.

Internal (list): The listStyle attribute will be inherited when deleting nested lists. The previous implementation added in #7879 handled lists with the listIndent=0. See #8073.
Marek Lewandowski 5 лет назад
Родитель
Сommit
3e6ea99fe2

+ 32 - 12
packages/ckeditor5-list/src/liststyleediting.js

@@ -141,11 +141,29 @@ export default class ListStyleEditing extends Plugin {
 				return;
 			}
 
+			// Find the outermost list item based on the `listIndent` attribute. We can't assume that `listIndent=0`
+			// because the selection can be hooked in nested lists.
+			//
+			// <listItem listIndent="0" listType="bulleted" listStyle="square">UL List item 1</listItem>
+			// <listItem listIndent="1" listType="bulleted" listStyle="square">UL List [item 1.1</listItem>
+			// <listItem listIndent="0" listType="bulleted" listStyle="circle">[]UL List item 1.</listItem>
+			// <listItem listIndent="1" listType="bulleted" listStyle="circle">UL List ]item 1.1</listItem>
+			//
+			// After deleting the content, we would like to inherit the "square" attribute for the last element:
+			//
+			// <listItem listIndent="0" listType="bulleted" listStyle="square">UL List item 1</listItem>
+			// <listItem listIndent="1" listType="bulleted" listStyle="square">UL List []item 1.1</listItem>
 			const mostOuterItemList = getSiblingListItem( firstPosition.parent, {
 				sameIndent: true,
-				listIndent: 0
+				listIndent: nextSibling.getAttribute( 'listIndent' )
 			} );
 
+			// The outermost list item may not exist while removing elements between lists with different value
+			// of the `listIndent` attribute. In such a case we don't want to update anything. See: #8073.
+			if ( !mostOuterItemList ) {
+				return;
+			}
+
 			if ( mostOuterItemList.getAttribute( 'listType' ) === nextSibling.getAttribute( 'listType' ) ) {
 				firstMostOuterItem = mostOuterItemList;
 			}
@@ -167,7 +185,7 @@ export default class ListStyleEditing extends Plugin {
 				// <listItem listIndent="0" listType="bulleted" listStyle="circle">UL List item 2</listItem>
 				const secondListMostOuterItem = getSiblingListItem( firstMostOuterItem.nextSibling, {
 					sameIndent: true,
-					listIndent: 0,
+					listIndent: firstMostOuterItem.getAttribute( 'listIndent' ),
 					direction: 'forward'
 				} );
 
@@ -211,7 +229,6 @@ function downcastListStyleAttribute() {
 		dispatcher.on( 'attribute:listStyle:listItem', ( evt, data, conversionApi ) => {
 			const viewWriter = conversionApi.writer;
 			const currentElement = data.item;
-			const listStyle = data.attributeNewValue;
 
 			const previousElement = getSiblingListItem( currentElement.previousSibling, {
 				sameIndent: true,
@@ -221,25 +238,23 @@ function downcastListStyleAttribute() {
 
 			const viewItem = conversionApi.mapper.toViewElement( currentElement );
 
-			// Single item list.
-			if ( !previousElement ) {
-				setListStyle( viewWriter, listStyle, viewItem.parent );
-			} else if ( !areRepresentingSameList( previousElement, currentElement ) ) {
+			// A case when elements represent different lists. We need to separate their container.
+			if ( !areRepresentingSameList( currentElement, previousElement ) ) {
 				viewWriter.breakContainer( viewWriter.createPositionBefore( viewItem ) );
-				viewWriter.breakContainer( viewWriter.createPositionAfter( viewItem ) );
-
-				setListStyle( viewWriter, listStyle, viewItem.parent );
 			}
+
+			setListStyle( viewWriter, data.attributeNewValue, viewItem.parent );
 		}, { priority: 'low' } );
 	};
 
 	// Checks whether specified list items belong to the same list.
 	//
 	// @param {module:engine/model/element~Element} listItem1 The first list item to check.
-	// @param {module:engine/model/element~Element} listItem2 The second list item to check.
+	// @param {module:engine/model/element~Element|null} listItem2 The second list item to check.
 	// @returns {Boolean}
 	function areRepresentingSameList( listItem1, listItem2 ) {
-		return listItem1.getAttribute( 'listType' ) === listItem2.getAttribute( 'listType' ) &&
+		return listItem2 &&
+			listItem1.getAttribute( 'listType' ) === listItem2.getAttribute( 'listType' ) &&
 			listItem1.getAttribute( 'listIndent' ) === listItem2.getAttribute( 'listIndent' ) &&
 			listItem1.getAttribute( 'listStyle' ) === listItem2.getAttribute( 'listStyle' );
 	}
@@ -459,6 +474,11 @@ function fixListStyleAttributeOnListItemElements( editor ) {
 				// ■ Paragraph[]  // <-- The inserted item.
 				while ( existingListItem.is( 'element', 'listItem' ) && existingListItem.getAttribute( 'listIndent' ) !== indent ) {
 					existingListItem = existingListItem.previousSibling;
+
+					// If the item does not exist, most probably there is no other content in the editor. See: #8072.
+					if ( !existingListItem ) {
+						break;
+					}
 				}
 			}
 		}

+ 184 - 4
packages/ckeditor5-list/tests/liststyleediting.js

@@ -384,6 +384,41 @@ describe( 'ListStyleEditing', () => {
 					'</ul>'
 				);
 			} );
+
+			// See: #8081.
+			it( 'should convert properly nested list styles', () => {
+				// ■ Level 0
+				//     ▶ Level 0.1
+				//         ○ Level 0.1.1
+				//     ▶ Level 0.2
+				//         ○ Level 0.2.1
+				setModelData( model,
+					'<listItem listIndent="0" listType="bulleted" listStyle="default">Level 0</listItem>' +
+					'<listItem listIndent="1" listType="bulleted" listStyle="default">Level 0.1</listItem>' +
+					'<listItem listIndent="2" listType="bulleted" listStyle="circle">Level 0.1.1</listItem>' +
+					'<listItem listIndent="1" listType="bulleted" listStyle="default">Level 0.2</listItem>' +
+					'<listItem listIndent="2" listType="bulleted" listStyle="circle">Level 0.2.1</listItem>'
+				);
+
+				expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
+					'<ul>' +
+						'<li>Level 0' +
+							'<ul>' +
+								'<li>Level 0.1' +
+									'<ul style="list-style-type:circle">' +
+										'<li>Level 0.1.1</li>' +
+									'</ul>' +
+								'</li>' +
+								'<li>Level 0.2' +
+									'<ul style="list-style-type:circle">' +
+										'<li>Level 0.2.1</li>' +
+									'</ul>' +
+								'</li>' +
+							'</ul>' +
+						'</li>' +
+					'</ul>'
+				);
+			} );
 		} );
 	} );
 
@@ -725,6 +760,21 @@ describe( 'ListStyleEditing', () => {
 					'<listItem listIndent="1" listStyle="decimal" listType="numbered">3.[]</listItem>'
 				);
 			} );
+
+			// See: #8072.
+			it( 'should not throw when indenting a list without any other content in the editor', () => {
+				setModelData( model,
+					'<listItem listIndent="0" listStyle="default" listType="bulleted">Foo</listItem>' +
+					'<listItem listIndent="0" listStyle="default" listType="bulleted">[]</listItem>'
+				);
+
+				editor.execute( 'indentList' );
+
+				expect( getModelData( model ) ).to.equal(
+					'<listItem listIndent="0" listStyle="default" listType="bulleted">Foo</listItem>' +
+					'<listItem listIndent="1" listStyle="default" listType="bulleted">[]</listItem>'
+				);
+			} );
 		} );
 
 		describe( 'outdenting lists', () => {
@@ -923,6 +973,97 @@ describe( 'ListStyleEditing', () => {
 			} );
 		} );
 
+		describe( 'delete + undo', () => {
+			let editor, model, view;
+
+			beforeEach( () => {
+				return VirtualTestEditor
+					.create( {
+						plugins: [ Paragraph, ListStyleEditing, Typing, UndoEditing ]
+					} )
+					.then( newEditor => {
+						editor = newEditor;
+						model = editor.model;
+						view = editor.editing.view;
+					} );
+			} );
+
+			afterEach( () => {
+				return editor.destroy();
+			} );
+
+			// See: #7930.
+			it( 'should restore proper list style attribute after undo merging lists', () => {
+				// ○ 1.
+				// ○ 2.
+				// ○ 3.
+				// <paragraph>
+				// ■ 1.
+				// ■ 2.
+				setModelData( model,
+					'<listItem listIndent="0" listStyle="circle" listType="bulleted">1.</listItem>' +
+					'<listItem listIndent="0" listStyle="circle" listType="bulleted">2.</listItem>' +
+					'<listItem listIndent="0" listStyle="circle" listType="bulleted">3.</listItem>' +
+					'<paragraph>[]</paragraph>' +
+					'<listItem listIndent="0" listStyle="square" listType="bulleted">1.</listItem>' +
+					'<listItem listIndent="0" listStyle="square" listType="bulleted">2.</listItem>'
+				);
+
+				expect( getViewData( view, { withoutSelection: true } ), 'initial data' ).to.equal(
+					'<ul style="list-style-type:circle">' +
+						'<li>1.</li>' +
+						'<li>2.</li>' +
+						'<li>3.</li>' +
+					'</ul>' +
+					'<p></p>' +
+					'<ul style="list-style-type:square">' +
+						'<li>1.</li>' +
+						'<li>2.</li>' +
+					'</ul>'
+				);
+
+				// After removing the paragraph.
+				// ○ 1.
+				// ○ 2.
+				// ○ 3.
+				// ○ 1.
+				// ○ 2.
+				editor.execute( 'delete' );
+
+				expect( getViewData( view, { withoutSelection: true } ), 'executing delete' ).to.equal(
+					'<ul style="list-style-type:circle">' +
+						'<li>1.</li>' +
+						'<li>2.</li>' +
+						'<li>3.</li>' +
+						'<li>1.</li>' +
+						'<li>2.</li>' +
+					'</ul>'
+				);
+
+				// After undo.
+				// ○ 1.
+				// ○ 2.
+				// ○ 3.
+				// <paragraph>
+				// ■ 1.
+				// ■ 2.
+				editor.execute( 'undo' );
+
+				expect( getViewData( view, { withoutSelection: true } ), 'initial data' ).to.equal(
+					'<ul style="list-style-type:circle">' +
+						'<li>1.</li>' +
+						'<li>2.</li>' +
+						'<li>3.</li>' +
+					'</ul>' +
+					'<p></p>' +
+					'<ul style="list-style-type:square">' +
+						'<li>1.</li>' +
+						'<li>2.</li>' +
+					'</ul>'
+				);
+			} );
+		} );
+
 		describe( 'todo list', () => {
 			let editor, model;
 
@@ -1096,8 +1237,8 @@ describe( 'ListStyleEditing', () => {
 				setModelData( model,
 					'<listItem listIndent="0" listStyle="square" listType="bulleted">1.</listItem>' +
 					'<listItem listIndent="0" listStyle="square" listType="bulleted">2.</listItem>' +
-					'<listItem listIndent="1" listStyle="numbered" listType="decimal">2.1.</listItem>' +
-					'<listItem listIndent="2" listStyle="default" listType="default">2.1.1</listItem>' +
+					'<listItem listIndent="1" listStyle="decimal" listType="numbered">2.1.</listItem>' +
+					'<listItem listIndent="2" listStyle="default" listType="numbered">2.1.1</listItem>' +
 					'<paragraph>[]</paragraph>' +
 					'<listItem listIndent="0" listStyle="circle" listType="bulleted">1.</listItem>' +
 					'<listItem listIndent="0" listStyle="circle" listType="bulleted">2.</listItem>'
@@ -1108,8 +1249,8 @@ describe( 'ListStyleEditing', () => {
 				expect( getModelData( model ) ).to.equal(
 					'<listItem listIndent="0" listStyle="square" listType="bulleted">1.</listItem>' +
 					'<listItem listIndent="0" listStyle="square" listType="bulleted">2.</listItem>' +
-					'<listItem listIndent="1" listStyle="numbered" listType="decimal">2.1.</listItem>' +
-					'<listItem listIndent="2" listStyle="default" listType="default">2.1.1[]</listItem>' +
+					'<listItem listIndent="1" listStyle="decimal" listType="numbered">2.1.</listItem>' +
+					'<listItem listIndent="2" listStyle="default" listType="numbered">2.1.1[]</listItem>' +
 					'<listItem listIndent="0" listStyle="square" listType="bulleted">1.</listItem>' +
 					'<listItem listIndent="0" listStyle="square" listType="bulleted">2.</listItem>'
 				);
@@ -1233,6 +1374,45 @@ describe( 'ListStyleEditing', () => {
 				);
 			} );
 
+			// See: #8073.
+			it( 'should not crash when removing a content between intended lists', () => {
+				setModelData( model,
+					'<listItem listIndent="0" listStyle="default" listType="bulleted">aaaa</listItem>' +
+					'<listItem listIndent="1" listStyle="default" listType="bulleted">bb[bb</listItem>' +
+					'<listItem listIndent="2" listStyle="default" listType="bulleted">cc]cc</listItem>' +
+					'<listItem listIndent="3" listStyle="default" listType="bulleted">dddd</listItem>'
+				);
+
+				editor.execute( 'delete' );
+
+				expect( getModelData( model ) ).to.equal(
+					'<listItem listIndent="0" listStyle="default" listType="bulleted">aaaa</listItem>' +
+					'<listItem listIndent="1" listStyle="default" listType="bulleted">bb[]cc</listItem>' +
+					'<listItem listIndent="2" listStyle="default" listType="bulleted">dddd</listItem>'
+				);
+			} );
+
+			it( 'should read the `listStyle` attribute from the most outer selected list while removing content between lists', () => {
+				setModelData( model,
+					'<listItem listIndent="0" listStyle="square" listType="bulleted">1.</listItem>' +
+					'<listItem listIndent="0" listStyle="square" listType="bulleted">2.</listItem>' +
+					'<listItem listIndent="1" listStyle="decimal" listType="numbered">2.1.</listItem>' +
+					'<listItem listIndent="2" listStyle="lower-latin" listType="numbered">2.1.1[foo</listItem>' +
+					'<paragraph>Foo</paragraph>' +
+					'<listItem listIndent="0" listStyle="circle" listType="bulleted">1.</listItem>' +
+					'<listItem listIndent="1" listStyle="circle" listType="bulleted">bar]2.</listItem>'
+				);
+
+				editor.execute( 'delete' );
+
+				expect( getModelData( model ) ).to.equal(
+					'<listItem listIndent="0" listStyle="square" listType="bulleted">1.</listItem>' +
+					'<listItem listIndent="0" listStyle="square" listType="bulleted">2.</listItem>' +
+					'<listItem listIndent="1" listStyle="decimal" listType="numbered">2.1.</listItem>' +
+					'<listItem listIndent="2" listStyle="lower-latin" listType="numbered">2.1.1[]2.</listItem>'
+				);
+			} );
+
 			function simulateTyping( text ) {
 				// While typing, every character is an atomic change.
 				text.split( '' ).forEach( character => {