瀏覽代碼

Simplified the lookup for mutations container.

Piotrek Koszuliński 8 年之前
父節點
當前提交
779af80a9b
共有 2 個文件被更改,包括 38 次插入39 次删除
  1. 14 32
      packages/ckeditor5-typing/src/input.js
  2. 24 7
      packages/ckeditor5-typing/tests/input.js

+ 14 - 32
packages/ckeditor5-typing/src/input.js

@@ -174,7 +174,7 @@ class MutationHandler {
 	 */
 	_handleContainerChildrenMutations( mutations, viewSelection ) {
 		// Get common ancestor of all mutations.
-		const mutationsCommonAncestor = getMutationsCommonAncestor( mutations );
+		const mutationsCommonAncestor = getMutationsContainer( mutations );
 
 		// Quit if there is no common ancestor.
 		if ( !mutationsCommonAncestor ) {
@@ -390,40 +390,22 @@ function getSingleTextNodeChange( mutation ) {
 // @private
 // @param {Array.<module:engine/view/observer/mutationobserver~MutatedText|
 // module:engine/view/observer/mutationobserver~MutatedChildren>} mutations
-function getMutationsCommonAncestor( mutations ) {
-	let shortestList;
-
-	// Create array of ancestors list, extract the shortest one.
-	const ancestorsLists = mutations
-		.map( mutation => {
-			const ancestors = mutation.node.getAncestors( { includeNode: true, parentFirst: true } );
-
-			if ( !shortestList || ancestors.length < shortestList.length ) {
-				shortestList = ancestors;
-			}
-
-			return ancestors;
-		} ).filter( item => item !== shortestList );
-
-	// Find common ancestor from the lists. Go from the shortest list to the top. If common ancestor is found - return it.
-	for ( const ancestor of shortestList ) {
-		// Check only container and root elements.
-		if ( !ancestor.is( 'containerElement' ) && !ancestor.is( 'rootElement' ) ) {
-			continue;
-		}
-
-		let hasCommonAncestor = true;
-
-		for ( const list of ancestorsLists ) {
-			hasCommonAncestor = list.indexOf( ancestor ) > -1;
-		}
+// @returns {module:engine/view/containerelement~ContainerElement|engine/view/rootelement~RootElement|undefined}
+function getMutationsContainer( mutations ) {
+	const lca = mutations
+		.map( mutation => mutation.node )
+		.reduce( ( commonAncestor, node ) => {
+			return commonAncestor.getCommonAncestor( node, { includeSelf: true } );
+		} );
 
-		if ( hasCommonAncestor ) {
-			return ancestor;
-		}
+	if ( !lca ) {
+		return;
 	}
 
-	return null;
+	// We need to look for container and root elements only, so check all LCA's
+	// ancestors (starting from itself).
+	return lca.getAncestors( { includeSelf: true, parentFirst: true } )
+		.find( element => element.is( 'containerElement' ) || element.is( 'rootElement' ) );
 }
 
 // Returns true if container children have mutated and more than a single text node was changed. Single text node

+ 24 - 7
packages/ckeditor5-typing/tests/input.js

@@ -23,6 +23,7 @@ import ViewText from '@ckeditor/ckeditor5-engine/src/view/text';
 import ViewElement from '@ckeditor/ckeditor5-engine/src/view/element';
 import ViewContainerElement from '@ckeditor/ckeditor5-engine/src/view/containerelement';
 import ViewSelection from '@ckeditor/ckeditor5-engine/src/view/selection';
+import MutationObserver from '@ckeditor/ckeditor5-engine/src/view/observer/mutationobserver';
 
 import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin';
 import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard';
@@ -549,6 +550,14 @@ describe( 'Input feature', () => {
 		} );
 	} );
 
+	// NOTE: In all these tests we need to simulate the mutations. However, it's really tricky to tell what
+	// should be in "newChildren" because we don't have yet access to these nodes. We pass new instances,
+	// but this means that DomConverter which is used somewhere internally may return a different instance
+	// (which wouldn't happen in practice because it'd cache it). Besides, it's really hard to tell if the
+	// browser will keep the instances of the old elements when modifying the tree when the user is typing
+	// or if it will create new instances itself too.
+	// However, the code handling these mutations doesn't really care what's inside new/old children. It
+	// just needs the mutations common ancestor to understand how big fragment of the tree has changed.
 	describe( '#100', () => {
 		let domElement, domRoot;
 
@@ -575,10 +584,18 @@ describe( 'Input feature', () => {
 					buildViewConverter().for( newEditor.data.viewToModel )
 						.fromElement( 'img' )
 						.toElement( 'image' );
+
+					// Disable MO completely and in a way it won't be reenabled on some Document#render() call.
+					const mutationObserver = view.getObserver( MutationObserver );
+
+					mutationObserver.disable();
+					mutationObserver.enable = () => {};
 				} );
 		} );
 
 		afterEach( () => {
+			domElement.remove();
+
 			return editor.destroy();
 		} );
 
@@ -615,7 +632,7 @@ describe( 'Input feature', () => {
 					type: 'children',
 					node: paragraph,
 					oldChildren: [ link ],
-					newChildren: [ italic ]
+					newChildren: [ new ViewElement( 'i' ) ]
 				},
 
 				// Third mutation - italic's new children.
@@ -662,7 +679,7 @@ describe( 'Input feature', () => {
 					type: 'children',
 					node: paragraph,
 					oldChildren: [ link ],
-					newChildren: [ italic ]
+					newChildren: [ new ViewElement( 'i' ) ]
 				},
 
 				// Third mutation - italic's new children.
@@ -670,7 +687,7 @@ describe( 'Input feature', () => {
 					type: 'children',
 					node: italic,
 					oldChildren: [ text ],
-					newChildren: [ new ViewText( 'x' ), new ViewElement( 'a', null, text ) ]
+					newChildren: [ new ViewText( 'x' ), new ViewElement( 'a', null, 'text' ) ]
 				}
 			] );
 
@@ -710,7 +727,7 @@ describe( 'Input feature', () => {
 					type: 'children',
 					node: paragraph,
 					oldChildren: [ textBefore, link ],
-					newChildren: [ textBefore, italic ]
+					newChildren: [ new ViewText( 'xxx' ), new ViewElement( 'i' ) ]
 				},
 
 				// Third mutation - italic's new children.
@@ -718,7 +735,7 @@ describe( 'Input feature', () => {
 					type: 'children',
 					node: italic,
 					oldChildren: [ text ],
-					newChildren: [ new ViewElement( 'a', null, text ), new ViewText( 'x' ) ]
+					newChildren: [ new ViewElement( 'a', null, 'text' ), new ViewText( 'x' ) ]
 				}
 			] );
 
@@ -748,7 +765,7 @@ describe( 'Input feature', () => {
 					type: 'children',
 					node: paragraph,
 					oldChildren: [ strong ],
-					newChildren: [ new ViewElement( 'b', null, new ViewText( 'fixed text' ) ) ]
+					newChildren: [ new ViewElement( 'b', null, 'fixed text' ) ]
 				}
 			] );
 
@@ -772,7 +789,7 @@ describe( 'Input feature', () => {
 			const text = strong.getChild( 0 );
 
 			// Simulate mutations and DOM change.
-			domRoot.childNodes[ 0 ].innerHTML = '<strong>this is bar text</strong>';
+			domRoot.childNodes[ 0 ].childNodes[ 0 ].innerHTML = 'this is bar text';
 			view.fire( 'mutations', [
 				{
 					type: 'children',