浏览代码

Always update attributes of reused elements while rendering.

Krzysztof Krztoń 7 年之前
父节点
当前提交
5f2729f4ea
共有 2 个文件被更改,包括 39 次插入18 次删除
  1. 11 18
      packages/ckeditor5-engine/src/view/renderer.js
  2. 28 0
      packages/ckeditor5-engine/tests/view/renderer.js

+ 11 - 18
packages/ckeditor5-engine/src/view/renderer.js

@@ -294,30 +294,23 @@ export default class Renderer {
 	 * @param {Node} domElement The DOM element representing the given view element.
 	 */
 	_updateElementMappings( viewElement, domElement ) {
-		// Because we replace new view element mapping with the existing one, the corresponding DOM element
-		// will not be rerendered. The new view element may have different attributes than the previous one.
-		// Since its corresponding DOM element will not be rerendered, new attributes will not be added
-		// to the DOM, so we need to mark it here to make sure its attributes gets updated.
-		// Such situations may happen if only new view element was added to `this.markedAttributes`
-		// or none of the elements were added (relying on 'this._updateChildren()' which by rerendering the element
-		// also rerenders its attributes). See #1427 for more detailed case study.
-		const newViewChild = this.domConverter.mapDomToView( domElement );
-
-		// It may also happen that 'newViewChild' mapping is not present since its parent mapping
-		// was already removed (the 'domConverter.unbindDomElement()' method also unbinds children
-		// mappings) so we also check for '!newViewChild'.
-		// Also check if new element ('newViewChild') was marked to have its attributes rerenderd,
-		// if so, marked reused view element too (#1560).
-		if ( !newViewChild || newViewChild && !newViewChild.isSimilar( viewElement ) || this.markedAttributes.has( newViewChild ) ) {
-			this.markedAttributes.add( viewElement );
-		}
-
 		// Remap 'DomConverter' bindings.
 		this.domConverter.unbindDomElement( domElement );
 		this.domConverter.bindElements( domElement, viewElement );
 
 		// View element may have children which needs to be updated, but are not marked, mark them to update.
 		this.markedChildren.add( viewElement );
+
+		// Because we replace new view element mapping with the existing one, the corresponding DOM element
+		// will not be rerendered. The new view element may have different attributes than the previous one.
+		// Since its corresponding DOM element will not be rerendered, new attributes will not be added
+		// to the DOM, so we need to mark it here to make sure its attributes gets updated. See #1427 for more
+		// detailed case study.
+		// Also there are cases where replaced element can be removed from view structure and then has
+		// its attributes changed. In such cases it si not present in `markedAttributes` and may be the same
+		// (`element.isSimilar()`) as the reused element. To prevent such cases we always mark reused element
+		// to have its attrbiutes rerenderd (#1560).
+		this.markedAttributes.add( viewElement );
 	}
 
 	/**

+ 28 - 0
packages/ckeditor5-engine/tests/view/renderer.js

@@ -3100,6 +3100,34 @@ describe( 'Renderer', () => {
 
 				expect( domRoot.innerHTML ).to.equal( '<h1>h1</h1><p class="cke-test1">p</p><p>p2</p>' );
 			} );
+
+			it( 'should rerender element if it was removed and have its attributes removed after', () => {
+				const writer = new DowncastWriter();
+
+				// 1. Setup initial view/DOM.
+				viewRoot._appendChild( parse( '<container:p>1</container:p>' ) );
+
+				const viewP = viewRoot.getChild( 0 );
+
+				writer.setAttribute( 'data-placeholder', 'Body', viewP );
+
+				renderer.markToSync( 'children', viewRoot );
+				renderer.render();
+
+				expect( domRoot.innerHTML ).to.equal( '<p data-placeholder="Body">1</p>' );
+
+				// 2. Modify view.
+				viewRoot._removeChildren( 0, viewRoot.childCount );
+
+				writer.removeAttribute( 'data-placeholder', viewP );
+
+				viewRoot._appendChild( parse( '<container:p>1</container:p><container:p>2</container:p>' ) );
+
+				renderer.markToSync( 'children', viewRoot );
+				renderer.render();
+
+				expect( domRoot.innerHTML ).to.equal( '<p>1</p><p>2</p>' );
+			} );
 		} );
 	} );