瀏覽代碼

Added filtering of nested nodes in deleteContent.

Oskar Wróbel 8 年之前
父節點
當前提交
0600108acf

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

@@ -68,6 +68,13 @@ export default function deleteContent( selection, batch, options = {} ) {
 	// want to override that behavior anyway.
 	if ( !options.leaveUnmerged ) {
 		mergeBranches( batch, startPos, endPos );
+
+		// We need to check and strip disallowed attributes in all nested nodes because after merge
+		// some attributes could end up in a path where are disallowed.
+		//
+		// e.g. bold is disallowed for <H1>
+		// <h1>Fo{o</h1><p>b}a<b>r</b><p> -> <h1>Fo{}a<b>r</b><h1> -> <h1>Fo{}ar<h1>.
+		removeDisallowedAttributes( batch, Array.from( startPos.parent.getChildren() ), startPos );
 	}
 
 	selection.setCollapsedAt( startPos );
@@ -111,8 +118,8 @@ function mergeBranches( batch, startPos, endPos ) {
 	// <a><b>x[]</b></a><c><d>{}y</d></c>
 	// will become:
 	// <a><b>xy</b>[]</a><c>{}</c>
-	const nextStartPos = Position.createAfter( startParent );
-	let nextEndPos = Position.createBefore( endParent );
+	startPos = Position.createAfter( startParent );
+	endPos = Position.createBefore( endParent );
 
 	if ( endParent.isEmpty ) {
 		batch.remove( endParent );
@@ -125,20 +132,13 @@ function mergeBranches( batch, startPos, endPos ) {
 
 		// Move the end parent only if needed.
 		// E.g. not in this case: <p>ab</p>[]{}<p>cd</p>
-		if ( !nextEndPos.isEqual( nextStartPos ) ) {
-			batch.move( endParent, nextStartPos );
+		if ( !endPos.isEqual( startPos ) ) {
+			batch.move( endParent, startPos );
 		}
 
 		// To then become:
 		// <a><b>xy</b>[]</a><c>{}</c>
-		batch.merge( nextStartPos );
-
-		// We need to check and strip disallowed attributes in direct children because after merge
-		// some attributes could end up in a parent where are disallowed.
-		//
-		// e.g. bold is disallowed for <H1>
-		// <h1>Fo{o</h1><p>b}a<b>r</b><p> -> <h1>Fo{}a<b>r</b><h1> -> <h1>Fo{}ar<h1>.
-		removeDisallowedAttributes( batch, Array.from( startParent.getChildren() ), startPos );
+		batch.merge( startPos );
 	}
 
 	// Removes empty end ancestors:
@@ -146,16 +146,16 @@ function mergeBranches( batch, startPos, endPos ) {
 	// becomes:
 	// <a>fo[]</a><b><a>{}</a></b>
 	// So we can remove <a> and <b>.
-	while ( nextEndPos.parent.isEmpty ) {
-		const parentToRemove = nextEndPos.parent;
+	while ( endPos.parent.isEmpty ) {
+		const parentToRemove = endPos.parent;
 
-		nextEndPos = Position.createBefore( parentToRemove );
+		endPos = Position.createBefore( parentToRemove );
 
 		batch.remove( parentToRemove );
 	}
 
 	// Continue merging next level.
-	mergeBranches( batch, nextStartPos, nextEndPos );
+	mergeBranches( batch, startPos, endPos );
 }
 
 function shouldAutoparagraph( doc, position ) {
@@ -226,7 +226,7 @@ function getNodeSchemaName( node ) {
 	return node.is( 'text' ) ? '$text' : node.name;
 }
 
-// Adds deltas with `removeAttributes` operation for disallowed by schema attributes on given nodes.
+// Adds deltas with `removeAttributes` operation for disallowed by schema attributes on given nodes and its children.
 //
 // @param {module:engine/model/batch~Batch} batch Batch to which the deltas will be added.
 // @param {module:engine/model/node~Node|Array<module:engine/model/node~Node>} nodes List of nodes or a single node to filter.
@@ -240,5 +240,9 @@ function removeDisallowedAttributes( batch, nodes, schemaPath ) {
 				batch.removeAttribute( node, attribute );
 			}
 		}
+
+		if ( node.is( 'element' ) ) {
+			removeDisallowedAttributes( batch, Array.from( node.getChildren() ), Position.createAt( node ) );
+		}
 	}
 }

+ 45 - 23
packages/ckeditor5-engine/tests/controller/deletecontent.js

@@ -167,11 +167,6 @@ describe( 'DataController', () => {
 				schema.allow( { name: '$text', inside: 'pparent' } );
 
 				schema.allow( { name: 'paragraph', attributes: [ 'align' ] } );
-				schema.allow( { name: 'image', attributes: [ 'a', 'b' ], inside: 'paragraph' } );
-				schema.allow( { name: 'image', attributes: [ 'b' ], inside: 'heading1' } );
-				schema.allow( { name: '$text', attributes: [ 'a', 'b', 'c' ], inside: 'paragraph' } );
-				schema.allow( { name: '$text', attributes: [ 'b' ], inside: 'heading1' } );
-				schema.allow( { name: '$text', attributes: [ 'c', 'd' ], inside: 'pchild' } );
 			} );
 
 			test(
@@ -268,12 +263,6 @@ describe( 'DataController', () => {
 				expect( spyMerge.called ).to.be.true;
 			} );
 
-			test(
-				'filters out disallowed attributes after merge',
-				'<heading1>fo[o</heading1><paragraph>b]a<$text a="1" b="1">r</$text><image a="1" b="1"></image></paragraph>',
-				'<heading1>fo[]a<$text b="1">r</$text><image b="1"></image></heading1>'
-			);
-
 			// Note: in all these cases we ignore the direction of merge.
 			// If https://github.com/ckeditor/ckeditor5-engine/issues/470 was fixed we could differently treat
 			// forward and backward delete.
@@ -411,18 +400,6 @@ describe( 'DataController', () => {
 					expect( getData( doc ) )
 						.to.equal( '<paragraph>fo[]</paragraph>' );
 				} );
-
-				test(
-					'filters out disallowed attributes after left merge',
-					'<paragraph>x<pchild>fo[o</pchild></paragraph><paragraph>y]<$text b="true" c="true">z</$text></paragraph>',
-					'<paragraph>x<pchild>fo[]<$text c="true">z</$text></pchild></paragraph>'
-				);
-
-				test(
-					'filters out disallowed attributes after right merge',
-					'<paragraph>fo[o</paragraph><paragraph><pchild>x<$text c="true" d="true">y]z</$text></pchild></paragraph>',
-					'<paragraph>fo[]<$text c="true">z</$text></paragraph>'
-				);
 			} );
 
 			describe( 'with object elements', () => {
@@ -453,6 +430,51 @@ describe( 'DataController', () => {
 					'<paragraph>ba[]</paragraph><blockWidget><nestedEditable>oo</nestedEditable></blockWidget>'
 				);
 			} );
+
+			describe( 'filtering out', () => {
+				beforeEach( () => {
+					const schema = doc.schema;
+
+					schema.allow( { name: '$text', attributes: [ 'a', 'b' ], inside: 'paragraph' } );
+					schema.allow( { name: '$text', attributes: [ 'b', 'c' ], inside: 'pchild' } );
+					schema.allow( { name: 'pchild', inside: 'pchild' } );
+					schema.disallow( { name: '$text', attributes: [ 'c' ], inside: 'pchild pchild' } );
+				} );
+
+				test(
+					'filters out disallowed attributes after left merge',
+					'<paragraph>x<pchild>fo[o</pchild></paragraph><paragraph>y]<$text a="1" b="1">z</$text></paragraph>',
+					'<paragraph>x<pchild>fo[]<$text b="1">z</$text></pchild></paragraph>'
+				);
+
+				test(
+					'filters out disallowed attributes from nested nodes after left merge',
+					'<paragraph>' +
+						'x' +
+						'<pchild>fo[o</pchild>' +
+					'</paragraph>' +
+					'<paragraph>' +
+						'b]a<$text a="1" b="1">r</$text>' +
+						'<pchild>b<$text b="1" c="1">i</$text>z</pchild>' +
+						'y' +
+					'</paragraph>',
+
+					'<paragraph>' +
+						'x' +
+						'<pchild>' +
+							'fo[]a<$text b="1">r</$text>' +
+							'<pchild>b<$text b="1">i</$text>z</pchild>' +
+							'y' +
+						'</pchild>' +
+					'</paragraph>'
+				);
+
+				test(
+					'filters out disallowed attributes after right merge',
+					'<paragraph>fo[o</paragraph><paragraph><pchild>x<$text b="1" c="1">y]z</$text></pchild></paragraph>',
+					'<paragraph>fo[]<$text b="1">z</$text></paragraph>'
+				);
+			} );
 		} );
 
 		describe( 'in element selections scenarios', () => {