8
0
Quellcode durchsuchen

Merge branch 'master' into t/1015

Piotrek Koszuliński vor 8 Jahren
Ursprung
Commit
f2f66a297c

+ 9 - 10
packages/ckeditor5-engine/src/model/documentselection.js

@@ -649,12 +649,7 @@ export default class DocumentSelection extends Selection {
 		const positionCandidate = Position.createFromPosition( removedRange.start );
 
 		// Find a range that is a correct selection range and is closest to the start of removed range.
-		let selectionRange = this._document.getNearestSelectionRange( positionCandidate );
-
-		// If nearest valid selection range cannot be found - use one created at the beginning of the root.
-		if ( !selectionRange ) {
-			selectionRange = new Range( new Position( positionCandidate.root, [ 0 ] ) );
-		}
+		const selectionRange = this._document.getNearestSelectionRange( positionCandidate );
 
 		// Remove the old selection range before preparing and adding new selection range. This order is important,
 		// because new range, in some cases, may intersect with old range (it depends on `getNearestSelectionRange()` result).
@@ -662,11 +657,15 @@ export default class DocumentSelection extends Selection {
 		this._ranges.splice( index, 1 );
 		liveRange.detach();
 
-		// Check the range, convert it to live range, bind events, etc.
-		const newRange = this._prepareRange( selectionRange );
+		// If nearest valid selection range has been found - add it in the place of old range.
+		if ( selectionRange ) {
+			// Check the range, convert it to live range, bind events, etc.
+			const newRange = this._prepareRange( selectionRange );
 
-		// Add new range in the place of old range.
-		this._ranges.splice( index, 0, newRange );
+			// Add new range in the place of old range.
+			this._ranges.splice( index, 0, newRange );
+		}
+		// If nearest valid selection range cannot be found - just removing the old range is fine.
 
 		// Fire an event informing about selection change.
 		this.fire( 'change:range', { directChange: false } );

+ 35 - 10
packages/ckeditor5-engine/src/view/observer/mutationobserver.js

@@ -12,6 +12,7 @@
 import Observer from './observer';
 import ViewSelection from '../selection';
 import { startsWithFiller, getDataWithoutFiller } from '../filler';
+import isEqualWith from '@ckeditor/ckeditor5-utils/src/lib/lodash/isEqualWith';
 
 /**
  * Mutation observer class observes changes in the DOM, fires {@link module:engine/view/document~Document#event:mutations} event, mark view
@@ -204,16 +205,21 @@ export default class MutationObserver extends Observer {
 
 		for ( const viewElement of mutatedElements ) {
 			const domElement = domConverter.mapViewToDom( viewElement );
-			const viewChildren = viewElement.getChildren();
-			const newViewChildren = domConverter.domChildrenToView( domElement );
-
-			this.renderer.markToSync( 'children', viewElement );
-			viewMutations.push( {
-				type: 'children',
-				oldChildren: Array.from( viewChildren ),
-				newChildren: Array.from( newViewChildren ),
-				node: viewElement
-			} );
+			const viewChildren = Array.from( viewElement.getChildren() );
+			const newViewChildren = Array.from( domConverter.domChildrenToView( domElement ) );
+
+			// It may happen that as a result of many changes (sth was inserted and then removed),
+			// both elements haven't really changed. #1031
+			if ( !isEqualWith( viewChildren, newViewChildren, sameNodes ) ) {
+				this.renderer.markToSync( 'children', viewElement );
+
+				viewMutations.push( {
+					type: 'children',
+					oldChildren: viewChildren,
+					newChildren: newViewChildren,
+					node: viewElement
+				} );
+			}
 		}
 
 		// Retrieve `domSelection` using `ownerDocument` of one of mutated nodes.
@@ -244,6 +250,25 @@ export default class MutationObserver extends Observer {
 		// If nothing changes on `mutations` event, at this point we have "dirty DOM" (changed) and de-synched
 		// view (which has not been changed). In order to "reset DOM" we render the view again.
 		this.document.render();
+
+		function sameNodes( child1, child2 ) {
+			// First level of comparison (array of children vs array of children) – use the Lodash's default behavior.
+			if ( Array.isArray( child1 ) ) {
+				return;
+			}
+
+			// Elements.
+			if ( child1 === child2 ) {
+				return true;
+			}
+			// Texts.
+			else if ( child1.is( 'text' ) && child2.is( 'text' ) ) {
+				return child1.data === child2.data;
+			}
+
+			// Not matching types.
+			return false;
+		}
 	}
 
 	/**

+ 24 - 0
packages/ckeditor5-engine/tests/model/documentselection.js

@@ -680,6 +680,30 @@ describe( 'DocumentSelection', () => {
 
 				expect( selection.getFirstPosition().path ).to.deep.equal( [ 0, 6 ] );
 			} );
+
+			it( 'fix selection range if it ends up in graveyard #4 - whole content removed', () => {
+				doc.applyOperation( wrapInDelta(
+					new RemoveOperation(
+						new Position( root, [ 0 ] ),
+						3,
+						new Position( doc.graveyard, [ 0 ] ),
+						doc.version
+					)
+				) );
+
+				expect( selection.getFirstPosition().path ).to.deep.equal( [ 0 ] );
+
+				doc.applyOperation( wrapInDelta(
+					new InsertOperation(
+						new Position( root, [ 0 ] ),
+						new Element( 'p' ),
+						doc.version
+					)
+				) );
+
+				// Now it's clear that it's the default range.
+				expect( selection.getFirstPosition().path ).to.deep.equal( [ 0, 0 ] );
+			} );
 		} );
 	} );
 

+ 34 - 0
packages/ckeditor5-engine/tests/view/observer/mutationobserver.js

@@ -291,6 +291,40 @@ describe( 'MutationObserver', () => {
 		expect( lastMutations[ 0 ].newText ).to.equal( 'foo ' );
 	} );
 
+	it( 'should ignore child mutations which resulted in no changes – when element contains elements', () => {
+		viewRoot.appendChildren( parse( '<container:p><container:x></container:x></container:p>' ) );
+
+		viewDocument.render();
+
+		const domP = domEditor.childNodes[ 2 ];
+		const domY = document.createElement( 'y' );
+		domP.appendChild( domY );
+		domY.remove();
+
+		mutationObserver.flush();
+
+		expect( lastMutations.length ).to.equal( 0 );
+	} );
+
+	// This case is more tricky than the previous one because DOMConverter will return a different
+	// instances of view text nodes every time it converts a DOM text node.
+	it( 'should ignore child mutations which resulted in no changes – when element contains text nodes', () => {
+		const domP = domEditor.childNodes[ 0 ];
+		const domText = document.createTextNode( 'x' );
+		domP.appendChild( domText );
+		domText.remove();
+
+		const domP2 = domEditor.childNodes[ 1 ];
+		domP2.appendChild( document.createTextNode( 'x' ) );
+
+		mutationObserver.flush();
+
+		// There was onlu P2 change. P1 must be ignored.
+		const viewP2 = viewRoot.getChild( 1 );
+		expect( lastMutations.length ).to.equal( 1 );
+		expect( lastMutations[ 0 ].node ).to.equal( viewP2 );
+	} );
+
 	it( 'should not ignore mutation with br inserted not on the end of the paragraph', () => {
 		viewRoot.appendChildren( parse( '<container:p>foo</container:p>' ) );