浏览代码

Fixed: `view.Renderer#render` will now always create inline filler at expected position.

Szymon Cofalik 8 年之前
父节点
当前提交
10efae7d56
共有 2 个文件被更改,包括 81 次插入28 次删除
  1. 46 28
      packages/ckeditor5-engine/src/view/renderer.js
  2. 35 0
      packages/ckeditor5-engine/tests/view/renderer.js

+ 46 - 28
packages/ckeditor5-engine/src/view/renderer.js

@@ -219,37 +219,58 @@ export default class Renderer {
 		this.markedAttributes.clear();
 		this.markedChildren.clear();
 
-		// Remember the filler by its node.
-		this._inlineFiller = this._getInlineFillerNode( inlineFillerPosition );
+		// Check whether inline filler is required and where it really is in DOM. At this point in most cases it should
+		// be in DOM, but not always. For example, if inline filler was deep in created DOM structure, it will not be created.
+		// Similarly, if it was removed at the beginning of this function and then neither text nor children were updated,
+		// it will not be present. Fix those and similar scenarios.
+		if ( inlineFillerPosition ) {
+			const fillerDomPosition = this.domConverter.viewPositionToDom( inlineFillerPosition );
+			const domDocument = fillerDomPosition.parent.ownerDocument;
+
+			if ( !startsWithFiller( fillerDomPosition.parent ) ) {
+				// Filler has not been created at filler position. Create it now.
+				// Save created filler element in `this._inlineFiller`.
+				this._inlineFiller = this._applyInlineFiller( domDocument, fillerDomPosition.parent, fillerDomPosition.offset );
+			} else {
+				// Filler has been found, save it.
+				this._inlineFiller = fillerDomPosition.parent;
+			}
+		} else {
+			// There is no filler needed.
+			this._inlineFiller = null;
+		}
 	}
 
 	/**
-	 * Gets the text node in which the inline filler is kept.
+	 * Applies inline filler at given position.
+	 *
+	 * The position can be given as array with DOM nodes and offset in that array, or DOM parent element and offset in that element.
 	 *
 	 * @private
-	 * @param {module:engine/view/position~Position} fillerPosition The position on which the filler is needed in the view.
-	 * @returns {Text} The text node with the filler.
+	 * @param {Document} domDocument
+	 * @param {Element|Array} domParentOrArray
+	 * @param {Number} offset
+	 * @returns {Text} The DOM text node that contains inline filler.
 	 */
-	_getInlineFillerNode( fillerPosition ) {
-		if ( !fillerPosition ) {
-			this._inlineFiller = null;
+	_applyInlineFiller( domDocument, domParentOrArray, offset ) {
+		const childNodes = domParentOrArray instanceof Array ? domParentOrArray : domParentOrArray.childNodes;
+		const nodeAfterFiller = childNodes[ offset ];
 
-			return;
-		}
+		if ( this.domConverter.isText( nodeAfterFiller ) ) {
+			nodeAfterFiller.data = INLINE_FILLER + nodeAfterFiller.data;
+
+			return nodeAfterFiller;
+		} else {
+			const fillerNode = domDocument.createTextNode( INLINE_FILLER );
 
-		const domPosition = this.domConverter.viewPositionToDom( fillerPosition );
+			if ( domParentOrArray instanceof Array ) {
+				childNodes.splice( offset, 0, fillerNode );
+			} else {
+				insertAt( domParentOrArray, offset, fillerNode );
+			}
 
-		/* istanbul ignore if */
-		if ( !domPosition || !startsWithFiller( domPosition.parent ) ) {
-			/**
-			 * Cannot find filler node by its position.
-			 *
-			 * @error view-renderer-cannot-find-filler
-			 */
-			throw new CKEditorError( 'view-renderer-cannot-find-filler: Cannot find filler node by its position.' );
+			return fillerNode;
 		}
-
-		return domPosition.parent;
 	}
 
 	/**
@@ -449,14 +470,11 @@ export default class Renderer {
 		const actualDomChildren = domElement.childNodes;
 		const expectedDomChildren = Array.from( domConverter.viewChildrenToDom( viewElement, domDocument, { bind: true } ) );
 
+		// Inline filler element has to be created during children update because we need it to diff actual dom
+		// elements with expected dom elements. We need inline filler in expected dom elements so we won't re-render
+		// text node if it is not necessary.
 		if ( filler && filler.parent == viewElement ) {
-			const expectedNodeAfterFiller = expectedDomChildren[ filler.offset ];
-
-			if ( this.domConverter.isText( expectedNodeAfterFiller ) ) {
-				expectedNodeAfterFiller.data = INLINE_FILLER + expectedNodeAfterFiller.data;
-			} else {
-				expectedDomChildren.splice( filler.offset, 0, domDocument.createTextNode( INLINE_FILLER ) );
-			}
+			this._applyInlineFiller( domDocument, expectedDomChildren, filler.offset );
 		}
 
 		const actions = diff( actualDomChildren, expectedDomChildren, sameNodes );

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

@@ -7,6 +7,7 @@
 
 import ViewDocument from '../../src/view/document';
 import ViewElement from '../../src/view/element';
+import ViewContainerElement from '../../src/view/containerelement';
 import ViewAttributeElement from '../../src/view/attributeelement';
 import ViewText from '../../src/view/text';
 import ViewRange from '../../src/view/range';
@@ -1216,6 +1217,40 @@ describe( 'Renderer', () => {
 			} ).to.throw( CKEditorError, /^view-renderer-filler-was-lost/ );
 		} );
 
+		// #1014.
+		it( 'should create inline filler in newly created dom nodes', () => {
+			// 1. Create the view structure which needs inline filler.
+			const inputView =
+				'<container:ul>' +
+					'<container:li>Foobar.</container:li>' +
+					'<container:li>[]<container:div></container:div></container:li>' +
+				'</container:ul>';
+
+			const { view: view, selection: newSelection } = parse( inputView );
+
+			viewRoot.appendChildren( view );
+			selection.setTo( newSelection );
+
+			renderer.markToSync( 'children', viewRoot );
+			renderer.render();
+
+			// 2. Check if filler element has been (correctly) created.
+			expect( domRoot.innerHTML.indexOf( INLINE_FILLER ) ).not.to.equal( -1 );
+
+			// 3. Move the inline filler parent to a newly created element.
+			const viewLi = view.getChild( 0 );
+			const viewLiIndented = view.removeChildren( 1, 1 ); // Array with one element.
+			const viewUl = new ViewContainerElement( 'ul', null, viewLiIndented );
+			viewLi.appendChildren( viewUl );
+
+			// 4. Mark changed items and render the view.
+			renderer.markToSync( 'children', view );
+			renderer.markToSync( 'children', viewLi );
+			renderer.render();
+
+			expect( domRoot.innerHTML.indexOf( INLINE_FILLER ) ).not.to.equal( -1 );
+		} );
+
 		it( 'should handle focusing element', () => {
 			const domFocusSpy = testUtils.sinon.spy( domRoot, 'focus' );
 			const editable = selection.editableElement;