浏览代码

Merge pull request #86 from ckeditor/t/ckeditor5-engine/1207

Other: Use post-fixer API.
Piotr Jasiun 8 年之前
父节点
当前提交
19ec1b3797

+ 113 - 132
packages/ckeditor5-list/src/converters.js

@@ -584,144 +584,145 @@ export function viewToModelPosition( evt, data ) {
  *		<listItem type="bulleted" indent=1>Item 3</listItem>   <--- note that indent got post-fixed.
  *
  * @param {module:engine/model/model~Model} model The data model.
- * @returns {Function} A callback to be attached to the {@link module:engine/model/document~Document#event:change document change event}.
+ * @param {module:engine/model/writer~Writer} writer The writer to do changes with.
+ * @returns {Boolean} `true` if any change has been applied, `false` otherwise.
  */
-export function modelChangePostFixer( model ) {
-	return ( evt, type, changes, batch ) => {
-		if ( batch.type == 'transparent' ) {
-			return;
-		}
+export function modelChangePostFixer( model, writer ) {
+	const changes = model.document.differ.getChanges();
+	const itemToListHead = new Map();
 
-		if ( type == 'remove' ) {
-			const howMany = changes.range.end.offset - changes.range.start.offset;
-			const sourcePos = changes.sourcePosition._getTransformedByInsertion( changes.range.start, howMany, true );
-
-			// Fix list items after the cut-out range.
-			// This fix is needed if items in model after cut-out range have now wrong indents compared to their previous siblings.
-			_fixItemsIndent( sourcePos, model, batch );
-			// This fix is needed if two different nested lists got merged, change types of list items "below".
-			_fixItemsType( sourcePos, model, batch );
-		} else if ( type == 'move' ) {
-			const howMany = changes.range.end.offset - changes.range.start.offset;
-			const sourcePos = changes.sourcePosition._getTransformedByInsertion( changes.range.start, howMany, true );
-
-			// Fix list items after the cut-out range.
-			// This fix is needed if items in model after cut-out range have now wrong indents compared to their previous siblings.
-			_fixItemsIndent( sourcePos, model, batch );
-			// This fix is needed if two different nested lists got merged, change types of list items "below".
-			_fixItemsType( sourcePos, model, batch );
-
-			// Fix items in moved range.
-			// This fix is needed if inserted items are too deeply intended.
-			_fixItemsIndent( changes.range.start, model, batch );
-			// This fix is needed if one or more first inserted items have different type.
-			_fixItemsType( changes.range.start, model, batch );
-
-			// Fix list items after inserted range.
-			// This fix is needed if items in model after inserted range have wrong indents.
-			_fixItemsIndent( changes.range.end, model, batch );
-			// This fix is needed if one or more last inserted items have different type.
-			_fixItemsType( changes.range.end, model, batch );
-		} else if ( type == 'rename' && changes.oldName == 'listItem' && changes.newName != 'listItem' ) {
-			const element = changes.element;
-
-			// Element name is changed from list to something else. Remove useless attributes.
-			model.enqueueChange( batch, writer => {
-				writer.removeAttribute( 'indent', element );
-				writer.removeAttribute( 'type', element );
-			} );
-
-			const changePos = ModelPosition.createAfter( changes.element );
-
-			// Fix list items after the renamed element.
-			// This fix is needed if there are items after renamed element, those items should start from indent = 0.
-			_fixItemsIndent( changePos, model, batch );
-		} else if ( type == 'insert' ) {
-			// Fix list items in inserted range.
-			// This fix is needed if inserted items are too deeply intended.
-			_fixItemsIndent( changes.range.start, model, batch );
-			// This fix is needed if one or more first inserted items have different type.
-			_fixItemsType( changes.range.start, model, batch );
-
-			// Fix list items after inserted range.
-			// This fix is needed if items in model after inserted range have wrong indents.
-			_fixItemsIndent( changes.range.end, model, batch );
-			// This fix is needed if one or more last inserted items have different type.
-			_fixItemsType( changes.range.end, model, batch );
-		} else if ( type == 'changeAttribute' && changes.key == 'indent' ) {
-			_fixItemsType( changes.range.start, model, batch );
-		}
-	};
-}
+	let applied = false;
 
-// Helper function for post fixer callback. Performs fixing of model `listElement` items indent attribute. Checks the model at the
-// `changePosition`. Looks at the node before position where change occurred and uses that node as a reference for following list items.
-function _fixItemsIndent( changePosition, model, batch ) {
-	let nextItem = changePosition.nodeAfter;
+	for ( const entry of changes ) {
+		if ( entry.type == 'insert' && entry.name == 'listItem' ) {
+			_addListToFix( entry.position );
+		} else if ( entry.type == 'insert' && entry.name != 'listItem' ) {
+			if ( entry.name != '$text' ) {
+				// In case of renamed element.
+				const item = entry.position.nodeAfter;
 
-	if ( nextItem && nextItem.name == 'listItem' ) {
-		model.enqueueChange( batch, writer => {
-			const prevItem = nextItem.previousSibling;
-			// This is the maximum indent that following model list item may have.
-			const maxIndent = prevItem && prevItem.is( 'listItem' ) ? prevItem.getAttribute( 'indent' ) + 1 : 0;
+				if ( item.hasAttribute( 'indent' ) ) {
+					writer.removeAttribute( 'indent', item );
 
-			// Check how much the next item needs to be outdented.
-			let outdentBy = nextItem.getAttribute( 'indent' ) - maxIndent;
-			const items = [];
+					applied = true;
+				}
+
+				if ( item.hasAttribute( 'type' ) ) {
+					writer.removeAttribute( 'type', item );
 
-			while ( nextItem && nextItem.name == 'listItem' && nextItem.getAttribute( 'indent' ) > maxIndent ) {
-				if ( outdentBy > nextItem.getAttribute( 'indent' ) ) {
-					outdentBy = nextItem.getAttribute( 'indent' );
+					applied = true;
 				}
+			}
+
+			const posAfter = entry.position.getShiftedBy( entry.length );
+
+			_addListToFix( posAfter );
+		} else if ( entry.type == 'remove' && entry.name == 'listItem' ) {
+			_addListToFix( entry.position );
+		} else if ( entry.type == 'attribute' && entry.attributeKey == 'indent' ) {
+			_addListToFix( entry.range.start );
+		} else if ( entry.type == 'attribute' && entry.attributeKey == 'type' ) {
+			_addListToFix( entry.range.start );
+		}
+	}
+
+	for ( const listHead of itemToListHead.values() ) {
+		_fixListIndents( listHead );
+		_fixListTypes( listHead );
+	}
+
+	return applied;
 
-				const newIndent = nextItem.getAttribute( 'indent' ) - outdentBy;
+	function _addListToFix( position ) {
+		const prev = position.nodeBefore;
 
-				items.push( { item: nextItem, indent: newIndent } );
+		if ( !prev || !prev.is( 'listItem' ) ) {
+			const item = position.nodeAfter;
 
-				nextItem = nextItem.nextSibling;
+			if ( item && item.is( 'listItem' ) ) {
+				itemToListHead.set( item, item );
 			}
+		} else {
+			let listHead = prev;
+
+			if ( itemToListHead.has( listHead ) ) {
+				return;
+			}
+
+			while ( listHead.previousSibling && listHead.previousSibling.is( 'listItem' ) ) {
+				listHead = listHead.previousSibling;
 
-			if ( items.length > 0 ) {
-				// Since we are outdenting list items, it is safer to start from the last one (it will maintain correct model state).
-				for ( const item of items.reverse() ) {
-					writer.setAttribute( 'indent', item.indent, item.item );
+				if ( itemToListHead.has( listHead ) ) {
+					return;
 				}
 			}
-		} );
+
+			itemToListHead.set( position.nodeBefore, listHead );
+		}
 	}
-}
 
-// Helper function for post fixer callback. Performs fixing of model nested `listElement` items type attribute.
-// Checks the model at the `changePosition`. Looks at nodes after/before that position and changes those items type
-// to the same as node before/after `changePosition`.
-function _fixItemsType( changePosition, model, batch ) {
-	let item = changePosition.nodeAfter;
+	function _fixListIndents( item ) {
+		let maxIndent = 0;
+		let fixBy = null;
 
-	if ( !item || !item.is( 'listItem' ) || item.getAttribute( 'indent' ) === 0 ) {
-		// !item - when last item got removed.
-		// !item.is( 'listItem' ) - when first element to fix is not a list item already.
-		// indent === 0 - do not fix if changes are done on top level lists.
-		return;
-	}
+		while ( item && item.is( 'listItem' ) ) {
+			const itemIndent = item.getAttribute( 'indent' );
 
-	model.enqueueChange( batch, writer => {
-		const refItem = _getBoundaryItemOfSameList( item );
-		const refIndent = refItem.getAttribute( 'indent' );
+			if ( itemIndent > maxIndent ) {
+				let newIndent;
 
-		if ( refIndent === 0 ) {
-			return;
+				if ( fixBy === null ) {
+					fixBy = itemIndent - maxIndent;
+					newIndent = maxIndent;
+				} else {
+					if ( fixBy > itemIndent ) {
+						fixBy = itemIndent;
+					}
+
+					newIndent = itemIndent - fixBy;
+				}
+
+				writer.setAttribute( 'indent', newIndent, item );
+
+				applied = true;
+			} else {
+				fixBy = null;
+				maxIndent = item.getAttribute( 'indent' ) + 1;
+			}
+
+			item = item.nextSibling;
 		}
+	}
 
-		const refType = refItem.getAttribute( 'type' );
+	function _fixListTypes( item ) {
+		let typesStack = [];
+		let prev = null;
 
-		while ( item && item.is( 'listItem' ) && item.getAttribute( 'indent' ) >= refIndent ) {
-			if ( item.getAttribute( 'type' ) != refType && item.getAttribute( 'indent' ) == refIndent ) {
-				writer.setAttribute( 'type', refType, item );
+		while ( item && item.is( 'listItem' ) ) {
+			const itemIndent = item.getAttribute( 'indent' );
+
+			if ( prev && prev.getAttribute( 'indent' ) > itemIndent ) {
+				typesStack = typesStack.slice( 0, itemIndent + 1 );
 			}
 
+			if ( itemIndent != 0 ) {
+				if ( typesStack[ itemIndent ] ) {
+					const type = typesStack[ itemIndent ];
+
+					if ( item.getAttribute( 'type' ) != type ) {
+						writer.setAttribute( 'type', type, item );
+
+						applied = true;
+					}
+				} else {
+					typesStack[ itemIndent ] = item.getAttribute( 'type' );
+				}
+			}
+
+			prev = item;
 			item = item.nextSibling;
 		}
-	} );
+	}
 }
 
 /**
@@ -1021,26 +1022,6 @@ function hoistNestedLists( nextIndent, modelRemoveStartPosition, viewRemoveStart
 	}
 }
 
-// Helper function to obtain the first or the last model list item which is in on the same indent level as given `item`.
-function _getBoundaryItemOfSameList( item ) {
-	const indent = item.getAttribute( 'indent' );
-
-	let result = item;
-	let prev = item.previousSibling;
-
-	while ( prev && prev.is( 'listItem' ) && prev.getAttribute( 'indent' ) >= indent ) {
-		item = item.previousSibling;
-
-		if ( item.getAttribute( 'indent' ) == indent ) {
-			result = item;
-		}
-
-		prev = item.previousSibling;
-	}
-
-	return result;
-}
-
 // Helper function that for given `view.Position`, returns a `view.Position` that is after all `view.UIElement`s that
 // are after given position.
 // For example:

+ 1 - 1
packages/ckeditor5-list/src/listengine.js

@@ -62,7 +62,7 @@ export default class ListEngine extends Plugin {
 		const data = editor.data;
 		const editing = editor.editing;
 
-		editor.model.document.on( 'change', modelChangePostFixer( editor.model ), { priority: 'high' } );
+		editor.model.document.registerPostFixer( writer => modelChangePostFixer( editor.model, writer ) );
 
 		editing.mapper.registerViewToModelLength( 'li', getViewListItemLength );
 		data.mapper.registerViewToModelLength( 'li', getViewListItemLength );

+ 58 - 64
packages/ckeditor5-list/tests/listengine.js

@@ -921,6 +921,42 @@ describe( 'ListEngine', () => {
 						'</ul>' +
 					'</li>' +
 				'</ul>' +
+				'<p>bar</p>',
+
+				'<p>foo</p>' +
+				'<ul>' +
+					'<li>' +
+						'1' +
+						'<ul>' +
+							'<li>' +
+								'1.1' +
+								'<ul><li>1.1.1</li><li>1.1.2</li><li>1.1.3</li><li>1.1.4</li></ul>' +
+							'</li>' +
+							'<li>' +
+								'1.2' +
+								'<ul><li>1.2.1</li></ul>' +
+							'</li>' +
+						'</ul>' +
+					'</li>' +
+					'<li>2</li>' +
+					'<li>' +
+						'3' +
+						'<ol>' +
+							'<li>' +
+								'3.1' +
+								'<ul>' +
+									'<li>' +
+										'3.1.1' +
+										'<ol><li>3.1.1.1</li><li>3.1.1.2</li></ol>' +
+									'</li>' +
+									'<li>3.1.2</li>' +
+								'</ul>' +
+							'</li>' +
+							'<li>3.2</li>' +
+							'<li>3.3</li>' +
+						'</ol>' +
+					'</li>' +
+				'</ul>' +
 				'<p>bar</p>'
 			);
 
@@ -934,7 +970,7 @@ describe( 'ListEngine', () => {
 				'			xxx' +
 				'			<li>' +
 				'				<ul><li></li><li>1.1.2</li></ul>' +
-				'				<ol><li>1.1.3</li><li>1.1.4</li></ol>' +
+				'				<ol><li>1.1.3</li><li>1.1.4</li></ol>' +		// Will be changed to <ul>.
 				'			</li>' +
 				'			<li>' +
 				'				<ul><li>1.2.1</li></ul>' +
@@ -952,14 +988,14 @@ describe( 'ListEngine', () => {
 				'					<li>' +
 				'						3.1.1' +
 				'						<ol><li>3.1.1.1</li></ol>' +
-				'						<ul><li>3.1.1.2</li></ul>' +
+				'						<ul><li>3.1.1.2</li></ul>' +			// Will be changed to <ol>.
 				'					</li>' +
 				'					<li>3.1.2</li>' +
 				'				</ul>' +
 				'			</li>' +
 				'		</ol>' +
 				'		<p>xxx</p>' +
-				'		<ul>' +
+				'		<ul>' +													// Since <p> gets removed, this will become <ol>.
 				'			<li>3.2</li>' +
 				'			<li>3.3</li>' +
 				'		</ul>' +
@@ -967,6 +1003,7 @@ describe( 'ListEngine', () => {
 				'	<p>xxx</p>' +
 				'</ul>' +
 				'<p>bar</p>',
+
 				'<p>foo</p>' +
 				'<ul>' +
 					'<li>' +
@@ -974,8 +1011,12 @@ describe( 'ListEngine', () => {
 						'<ul>' +
 							'<li>' +
 								'&nbsp;' +
-								'<ul><li>&nbsp;</li><li>1.1.2</li></ul>' +
-								'<ol><li>1.1.3</li><li>1.1.4</li></ol>' +
+								'<ul>' +
+									'<li>&nbsp;</li>' +
+									'<li>1.1.2</li>' +
+									'<li>1.1.3</li>' +
+									'<li>1.1.4</li>' +
+								'</ul>' +
 							'</li>' +
 							'<li>' +
 								'&nbsp;' +
@@ -992,23 +1033,24 @@ describe( 'ListEngine', () => {
 								'<ul>' +
 									'<li>' +
 										'3.1.1' +
-										'<ol><li>3.1.1.1</li></ol>' +
-										'<ul><li>3.1.1.2</li></ul>' +
+										'<ol>' +
+											'<li>3.1.1.1</li>' +
+											'<li>3.1.1.2</li>' +
+										'</ol>' +
 									'</li>' +
 									'<li>3.1.2</li>' +
 								'</ul>' +
 							'</li>' +
-						'</ol>' +
-						'<ul>' +
 							'<li>3.2</li>' +
 							'<li>3.3</li>' +
-						'</ul>' +
+						'</ol>' +
 					'</li>' +
 				'</ul>' +
 				'<p>bar</p>'
 			);
 
 			it( 'model test for nested lists', () => {
+				// <ol> in the middle will be fixed by postfixer to bulleted list.
 				editor.setData(
 					'<p>foo</p>' +
 					'<ul>' +
@@ -1020,9 +1062,9 @@ describe( 'ListEngine', () => {
 							'<ol>' +
 								'<li>' +
 									'1.2' +
-									'<ul>' +
+									'<ol>' +
 										'<li>1.2.1</li>' +
-									'</ul>' +
+									'</ol>' +
 								'</li>' +
 								'<li>1.3</li>' +
 							'</ol>' +
@@ -1036,9 +1078,9 @@ describe( 'ListEngine', () => {
 					'<paragraph>foo</paragraph>' +
 					'<listItem indent="0" type="bulleted">1</listItem>' +
 					'<listItem indent="1" type="bulleted">1.1</listItem>' +
-					'<listItem indent="1" type="numbered">1.2</listItem>' +
-					'<listItem indent="2" type="bulleted">1.2.1</listItem>' +
-					'<listItem indent="1" type="numbered">1.3</listItem>' +
+					'<listItem indent="1" type="bulleted">1.2</listItem>' +
+					'<listItem indent="2" type="numbered">1.2.1</listItem>' +
+					'<listItem indent="1" type="bulleted">1.3</listItem>' +
 					'<listItem indent="0" type="bulleted">2</listItem>' +
 					'<paragraph>bar</paragraph>';
 
@@ -1762,8 +1804,6 @@ describe( 'ListEngine', () => {
 				);
 			} );
 
-			// Note: although the feature itself does not let changing type of singular nested list item,
-			// conversion of those items is done item-by-item and this is tested in this suite.
 			describe( 'change type', () => {
 				testChangeType(
 					'list item that has nested items',
@@ -1783,6 +1823,7 @@ describe( 'ListEngine', () => {
 					'</ul>'
 				);
 
+				// The change will be "prevented" by post fixer.
 				testChangeType(
 					'list item that is a nested item',
 
@@ -1796,36 +1837,12 @@ describe( 'ListEngine', () => {
 							'a' +
 							'<ol>' +
 								'<li>b</li>' +
-							'</ol>' +
-							'<ul>' +
 								'<li>c</li>' +
-							'</ul>' +
-							'<ol>' +
 								'<li>d</li>' +
 							'</ol>' +
 						'</li>' +
 					'</ul>'
 				);
-
-				testChangeType(
-					'list item between two nested items ',
-
-					'<listItem indent="0" type="bulleted">a</listItem>' +
-					'<listItem indent="1" type="bulleted">b</listItem>' +
-					'[<listItem indent="1" type="numbered">c</listItem>]' +
-					'<listItem indent="1" type="bulleted">d</listItem>',
-
-					'<ul>' +
-						'<li>' +
-							'a' +
-							'<ul>' +
-								'<li>b</li>' +
-								'<li>c</li>' +
-								'<li>d</li>' +
-							'</ul>' +
-						'</li>' +
-					'</ul>'
-				);
 			} );
 
 			describe( 'change indent', () => {
@@ -2634,29 +2651,6 @@ describe( 'ListEngine', () => {
 	} );
 
 	describe( 'post fixer', () => {
-		it( 'should not be triggered if change-to-fix is in a transparent batch', () => {
-			// Note that the same example is also tested below in the insert suite, however in a non-transparent batch.
-			const input =
-				'<listItem indent="0" type="bulleted">a</listItem>' +
-				'[]' +
-				'<listItem indent="1" type="bulleted">b</listItem>';
-
-			const inserted = '<paragraph>x</paragraph>';
-
-			const output =
-				'<listItem indent="0" type="bulleted">a</listItem>' +
-				'<paragraph>x</paragraph>' +
-				'<listItem indent="1" type="bulleted">b</listItem>';
-
-			setModelData( model, input );
-
-			model.enqueueChange( 'transparent', writer => {
-				writer.insert( parseModel( inserted, model.schema ), modelDoc.selection.getFirstPosition() );
-			} );
-
-			expect( getModelData( model, { withoutSelection: true } ) ).to.equal( output );
-		} );
-
 		describe( 'insert', () => {
 			function test( testName, input, inserted, output ) {
 				it( testName, () => {