浏览代码

Changed: Removed unnecessary if statement in `clearAttributesStoredInElement` in `documentselection.js`.

Szymon Cofalik 8 年之前
父节点
当前提交
8de46d3a0a

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

@@ -707,15 +707,11 @@ function clearAttributesStoredInElement( changes, batch, document ) {
 		return;
 	}
 
-	const anyStoredAttributes = Array.from( changeParent.getAttributeKeys() ).some( key => key.startsWith( storePrefix ) );
+	document.enqueueChanges( () => {
+		const storedAttributes = Array.from( changeParent.getAttributeKeys() ).filter( key => key.startsWith( storePrefix ) );
 
-	if ( anyStoredAttributes ) {
-		document.enqueueChanges( () => {
-			const storedAttributes = Array.from( changeParent.getAttributeKeys() ).filter( key => key.startsWith( storePrefix ) );
-
-			for ( const key of storedAttributes ) {
-				batch.removeAttribute( changeParent, key );
-			}
-		} );
-	}
+		for ( const key of storedAttributes ) {
+			batch.removeAttribute( changeParent, key );
+		}
+	} );
 }

+ 32 - 38
packages/ckeditor5-engine/tests/model/documentselection.js

@@ -990,12 +990,8 @@ describe( 'DocumentSelection', () => {
 					batchTypes.set( batch, batch.type );
 				} );
 
-				sinon.spy( doc, 'enqueueChanges' );
-
 				doc.batch().insert( rangeInEmptyP.start, 'x' );
 
-				expect( doc.enqueueChanges.calledOnce ).to.be.true;
-
 				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
 				expect( emptyP.hasAttribute( abcStoreAttrKey ) ).to.be.false;
 
@@ -1006,11 +1002,8 @@ describe( 'DocumentSelection', () => {
 				selection.setRanges( [ rangeInEmptyP ] );
 				selection.setAttribute( 'foo', 'bar' );
 
-				sinon.spy( doc, 'enqueueChanges' );
-
 				doc.batch().move( fullP.getChild( 0 ), rangeInEmptyP.start );
 
-				expect( doc.enqueueChanges.calledOnce ).to.be.true;
 				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
 			} );
 
@@ -1021,48 +1014,20 @@ describe( 'DocumentSelection', () => {
 				emptyP.setAttribute( fooStoreAttrKey, 'bar' );
 				emptyP2.setAttribute( fooStoreAttrKey, 'bar' );
 
-				sinon.spy( doc, 'enqueueChanges' );
-
 				// <emptyP>{}<emptyP2>
 				doc.batch().merge( Position.createAfter( emptyP ) );
 
-				expect( doc.enqueueChanges.calledOnce ).to.be.true;
-
 				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
 				expect( emptyP.parent ).to.equal( root ); // Just to be sure we're checking the right element.
 			} );
 
-			it( 'are not removed or merged when containing element is merged with another empty element', () => {
-				const emptyP2 = new Element( 'p', null );
-				root.appendChildren( emptyP2 );
-
-				sinon.spy( doc, 'enqueueChanges' );
-
-				emptyP.setAttribute( fooStoreAttrKey, 'bar' );
-				emptyP2.setAttribute( abcStoreAttrKey, 'bar' );
-
-				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.true;
-				expect( emptyP.hasAttribute( abcStoreAttrKey ) ).to.be.false;
-
-				// <emptyP>{}<emptyP2>
-				doc.batch().merge( Position.createAfter( emptyP ) );
-
-				expect( doc.enqueueChanges.called ).to.be.false;
-
-				expect( emptyP.getAttribute( fooStoreAttrKey ) ).to.equal( 'bar' );
-				expect( emptyP.parent ).to.equal( root ); // Just to be sure we're checking the right element.
-			} );
-
 			it( 'are removed even when there is no selection in it', () => {
 				emptyP.setAttribute( fooStoreAttrKey, 'bar' );
 
 				selection.setRanges( [ rangeInFullP ] );
 
-				sinon.spy( doc, 'enqueueChanges' );
-
 				doc.batch().insert( rangeInEmptyP.start, 'x' );
 
-				expect( doc.enqueueChanges.calledOnce ).to.be.true;
 				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
 			} );
 
@@ -1076,17 +1041,46 @@ describe( 'DocumentSelection', () => {
 				const batch = doc.batch();
 				const spy = sinon.spy( batch, 'removeAttribute' );
 
-				sinon.spy( doc, 'enqueueChanges' );
-
 				// <emptyP>{}<emptyP2>
 				batch.merge( Position.createAfter( emptyP ) );
 
 				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
 
-				expect( doc.enqueueChanges.calledOnce ).to.be.true;
 				expect( spy.calledOnce ).to.be.true;
 			} );
 
+			it( 'uses document enqueue changes to clear attributes', () => {
+				selection.setRanges( [ rangeInEmptyP ] );
+				selection.setAttribute( 'foo', 'bar' );
+
+				doc.enqueueChanges( () => {
+					doc.batch().insert( rangeInEmptyP.start, 'x' );
+
+					// `emptyP` still has the attribute, because attribute clearing is in enqueued block.
+					expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.true;
+				} );
+
+				// When the dust settles, `emptyP` should not have the attribute.
+				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
+			} );
+
+			it( 'are not removed or merged when containing element is merged with another empty element', () => {
+				const emptyP2 = new Element( 'p', null );
+				root.appendChildren( emptyP2 );
+
+				emptyP.setAttribute( fooStoreAttrKey, 'bar' );
+				emptyP2.setAttribute( abcStoreAttrKey, 'bar' );
+
+				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.true;
+				expect( emptyP.hasAttribute( abcStoreAttrKey ) ).to.be.false;
+
+				// <emptyP>{}<emptyP2>
+				doc.batch().merge( Position.createAfter( emptyP ) );
+
+				expect( emptyP.getAttribute( fooStoreAttrKey ) ).to.equal( 'bar' );
+				expect( emptyP.parent ).to.equal( root ); // Just to be sure we're checking the right element.
+			} );
+
 			it( 'are not removed on transparent batches', () => {
 				selection.setRanges( [ rangeInEmptyP ] );
 				selection.setAttribute( 'foo', 'bar' );