ソースを参照

Merge branch 'master' into t/1009

Kamil Piechaczek 8 年 前
コミット
ee2dfb7a4a

+ 2 - 17
packages/ckeditor5-engine/src/controller/deletecontent.js

@@ -168,21 +168,6 @@ function checkCanBeMerged( leftPos, rightPos ) {
 	return true;
 }
 
-// Returns the lowest limit element defined in `Schema.limits` for passed selection.
-function getLimitElement( schema, selection ) {
-	let element = selection.getFirstRange().getCommonAncestor();
-
-	while ( !schema.limits.has( element.name ) ) {
-		if ( element.parent ) {
-			element = element.parent;
-		} else {
-			break;
-		}
-	}
-
-	return element;
-}
-
 function insertParagraph( batch, position, selection ) {
 	const paragraph = new Element( 'paragraph' );
 	batch.insert( position, paragraph );
@@ -191,7 +176,7 @@ function insertParagraph( batch, position, selection ) {
 }
 
 function replaceEntireContentWithParagraph( batch, selection ) {
-	const limitElement = getLimitElement( batch.document.schema, selection );
+	const limitElement = batch.document.schema.getLimitElement( selection );
 
 	batch.remove( Range.createIn( limitElement ) );
 	insertParagraph( batch, Position.createAt( limitElement ), selection );
@@ -202,7 +187,7 @@ function replaceEntireContentWithParagraph( batch, selection ) {
 // * selection contains at least two elements,
 // * whether the paragraph is allowed in schema in the common ancestor.
 function shouldEntireContentBeReplacedWithParagraph( schema, selection ) {
-	const limitElement = getLimitElement( schema, selection );
+	const limitElement = schema.getLimitElement( selection );
 	const limitStartPosition = Position.createAt( limitElement );
 	const limitEndPosition = Position.createAt( limitElement, 'end' );
 

+ 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 } );

+ 29 - 0
packages/ckeditor5-engine/src/model/schema.js

@@ -382,6 +382,35 @@ export default class Schema {
 		return validRanges;
 	}
 
+	/**
+	 * Returns the lowest {@link module:engine/model/schema~Schema#limits limit element} containing the entire
+	 * selection or the root otherwise.
+	 *
+	 * @param {module:engine/model/selection~Selection} selection Selection which returns the common ancestor.
+	 * @returns {module:engine/model/element~Element}
+	 */
+	getLimitElement( selection ) {
+		// Find the common ancestor for all selection's ranges.
+		let element = Array.from( selection.getRanges() )
+			.reduce( ( node, range ) => {
+				if ( !node ) {
+					return range.getCommonAncestor();
+				}
+
+				return node.getCommonAncestor( range.getCommonAncestor() );
+			}, null );
+
+		while ( !this.limits.has( element.name ) ) {
+			if ( element.parent ) {
+				element = element.parent;
+			} else {
+				break;
+			}
+		}
+
+		return element;
+	}
+
 	/**
 	 * Returns {@link module:engine/model/schema~SchemaItem schema item} that was registered in the schema under given name.
 	 * If item has not been found, throws error.

+ 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 ] );
+			} );
 		} );
 	} );
 

+ 96 - 0
packages/ckeditor5-engine/tests/model/schema/schema.js

@@ -641,4 +641,100 @@ describe( 'Schema', () => {
 			expect( result[ 1 ].end.path ).to.members( [ 1 ] );
 		} );
 	} );
+
+	describe( 'getLimitElement()', () => {
+		let doc, root;
+
+		beforeEach( () => {
+			doc = new Document();
+			schema = doc.schema;
+			root = doc.createRoot();
+
+			schema.registerItem( 'div', '$block' );
+			schema.registerItem( 'article', '$block' );
+			schema.registerItem( 'section', '$block' );
+			schema.registerItem( 'paragraph', '$block' );
+			schema.registerItem( 'widget', '$block' );
+			schema.registerItem( 'image', '$block' );
+			schema.registerItem( 'caption', '$block' );
+			schema.allow( { name: 'image', inside: 'widget' } );
+			schema.allow( { name: 'caption', inside: 'image' } );
+			schema.allow( { name: 'paragraph', inside: 'article' } );
+			schema.allow( { name: 'article', inside: 'section' } );
+			schema.allow( { name: 'section', inside: 'div' } );
+			schema.allow( { name: 'widget', inside: 'div' } );
+		} );
+
+		it( 'always returns $root element if any other limit was not defined', () => {
+			schema.limits.clear();
+
+			setData( doc, '<div><section><article><paragraph>foo[]bar</paragraph></article></section></div>' );
+			expect( schema.getLimitElement( doc.selection ) ).to.equal( root );
+		} );
+
+		it( 'returns the limit element which is the closest element to common ancestor for collapsed selection', () => {
+			schema.limits.add( 'article' );
+			schema.limits.add( 'section' );
+
+			setData( doc, '<div><section><article><paragraph>foo[]bar</paragraph></article></section></div>' );
+
+			const article = root.getNodeByPath( [ 0, 0, 0 ] );
+
+			expect( schema.getLimitElement( doc.selection ) ).to.equal( article );
+		} );
+
+		it( 'returns the limit element which is the closest element to common ancestor for non-collapsed selection', () => {
+			schema.limits.add( 'article' );
+			schema.limits.add( 'section' );
+
+			setData( doc, '<div><section><article>[foo</article><article>bar]</article></section></div>' );
+
+			const section = root.getNodeByPath( [ 0, 0 ] );
+
+			expect( schema.getLimitElement( doc.selection ) ).to.equal( section );
+		} );
+
+		it( 'works fine with multi-range selections', () => {
+			schema.limits.add( 'article' );
+			schema.limits.add( 'widget' );
+			schema.limits.add( 'div' );
+
+			setData(
+				doc,
+				'<div>' +
+					'<section>' +
+						'<article>' +
+							'<paragraph>[foo]</paragraph>' +
+						'</article>' +
+					'</section>' +
+					'<widget>' +
+						'<image>' +
+							'<caption>b[a]r</caption>' +
+						'</image>' +
+					'</widget>' +
+				'</div>'
+			);
+
+			const div = root.getNodeByPath( [ 0 ] );
+			expect( schema.getLimitElement( doc.selection ) ).to.equal( div );
+		} );
+
+		it( 'works fine with multi-range selections even if limit elements are not defined', () => {
+			schema.limits.clear();
+
+			setData(
+				doc,
+				'<div>' +
+					'<section>' +
+						'<article>' +
+							'<paragraph>[foo]</paragraph>' +
+						'</article>' +
+					'</section>' +
+				'</div>' +
+				'<section>b[]ar</section>'
+			);
+
+			expect( schema.getLimitElement( doc.selection ) ).to.equal( root );
+		} );
+	} );
 } );

+ 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>' ) );