Parcourir la source

Fix: Remove/restore the attributes on the DOM element instead of the view root element in detachDomRoot. This prevents issues with additional change block and rendering when the editor is being destroyed.

Aleksander Nowodzinski il y a 7 ans
Parent
commit
9f313ccec3

+ 6 - 13
packages/ckeditor5-engine/src/view/view.js

@@ -239,20 +239,13 @@ export default class View {
 		const domRoot = this.domRoots.get( name );
 		const viewRoot = this.document.getRoot( name );
 
-		this.change( writer => {
-			// Remove all root attributes so the element is bare.
-			for ( const attributeName of viewRoot.getAttributeKeys() ) {
-				writer.removeAttribute( attributeName, viewRoot );
-			}
-
-			// Clean-up the changes made by the change:isReadOnly listener.
-			writer.removeAttribute( 'contenteditable', viewRoot );
+		// Remove all root attributes so the DOM element is "bare".
+		[ ...domRoot.attributes ].forEach( ( { name } ) => domRoot.removeAttribute( name ) );
 
-			// Revert all view root attributes back to the state before attachDomRoot was called.
-			for ( const attribute in viewRoot._initialDomAttributes ) {
-				writer.setAttribute( attribute, viewRoot._initialDomAttributes[ attribute ], viewRoot );
-			}
-		} );
+		// Revert all view root attributes back to the state before attachDomRoot was called.
+		for ( const attribute in viewRoot._initialDomAttributes ) {
+			domRoot.setAttribute( attribute, viewRoot._initialDomAttributes[ attribute ] );
+		}
 
 		this.domRoots.delete( name );
 		this.domConverter.unbindDomElement( domRoot );

+ 10 - 3
packages/ckeditor5-engine/tests/view/view/view.js

@@ -169,6 +169,8 @@ describe( 'view', () => {
 			view.detachDomRoot( 'main' );
 			expect( count( view.domRoots ) ).to.equal( 0 );
 			expect( view.domConverter.mapViewToDom( viewRoot ) ).to.be.undefined;
+
+			domDiv.remove();
 		} );
 
 		it( 'should restore the DOM root attributes to the state before attachDomRoot()', () => {
@@ -200,20 +202,25 @@ describe( 'view', () => {
 				'data-baz': 'qux',
 				class: 'foo-class'
 			} );
+
+			domDiv.remove();
 		} );
 
-		it( 'should remove the "contenteditable" attribute', () => {
+		it( 'should remove the "contenteditable" attribute from the DOM root', () => {
 			const domDiv = document.createElement( 'div' );
 			const viewRoot = createViewRoot( viewDocument, 'div', 'main' );
 
 			view.attachDomRoot( domDiv );
+			view.render();
 
 			viewRoot.isReadOnly = false;
-			expect( viewRoot.getAttribute( 'contenteditable' ) ).to.equal( 'true' );
+			expect( domDiv.getAttribute( 'contenteditable' ) ).to.equal( 'true' );
 
 			view.detachDomRoot( 'main' );
 
-			expect( viewRoot.hasAttribute( 'contenteditable' ) ).to.be.false;
+			expect( domDiv.hasAttribute( 'contenteditable' ) ).to.be.false;
+
+			domDiv.remove();
 		} );
 	} );