Jelajahi Sumber

Changed: Reverted refreshing touching text nodes in `view.Renderer`. Moved `getTouchingTextNode` back to `view.DomConverter`.

Szymon Cofalik 8 tahun lalu
induk
melakukan
5c05872945

+ 31 - 3
packages/ckeditor5-engine/src/view/domconverter.js

@@ -15,8 +15,8 @@ import ViewPosition from './position';
 import ViewRange from './range';
 import ViewSelection from './selection';
 import ViewDocumentFragment from './documentfragment';
+import ViewTreeWalker from './treewalker';
 import { BR_FILLER, INLINE_FILLER_LENGTH, isBlockFiller, isInlineFiller, startsWithFiller, getDataWithoutFiller } from './filler';
-import { getTouchingTextNode as getTouchingViewTextNode } from './treewalker-utils';
 
 import global from '@ckeditor/ckeditor5-utils/src/dom/global';
 import indexOf from '@ckeditor/ckeditor5-utils/src/dom/indexof';
@@ -913,7 +913,7 @@ export default class DomConverter {
 		// 1. Replace the first space with a nbsp if the previous node ends with a space or there is no previous node
 		// (container element boundary).
 		if ( data.charAt( 0 ) == ' ' ) {
-			const prevNode = getTouchingViewTextNode( node, 'backward' );
+			const prevNode = this._getTouchingViewTextNode( node, false );
 			const prevEndsWithSpace = prevNode && this._nodeEndsWithSpace( prevNode );
 
 			if ( prevEndsWithSpace || !prevNode ) {
@@ -923,7 +923,7 @@ export default class DomConverter {
 
 		// 2. Replace the last space with a nbsp if this is the last text node (container element boundary).
 		if ( data.charAt( data.length - 1 ) == ' ' ) {
-			const nextNode = getTouchingViewTextNode( node, 'forward' );
+			const nextNode = this._getTouchingViewTextNode( node, true );
 
 			if ( !nextNode ) {
 				data = data.substr( 0, data.length - 1 ) + '\u00A0';
@@ -1017,6 +1017,34 @@ export default class DomConverter {
 		return data;
 	}
 
+	/**
+	 * Helper function. For given {@link module:engine/view/text~Text view text node}, it finds previous or next sibling
+	 * that is contained in the same container element. If there is no such sibling, `null` is returned.
+	 *
+	 * @param {module:engine/view/text~Text} node Reference node.
+	 * @param {Boolean} getNext
+	 * @returns {module:engine/view/text~Text|null} Touching text node or `null` if there is no next or previous touching text node.
+	 */
+	_getTouchingViewTextNode( node, getNext ) {
+		const treeWalker = new ViewTreeWalker( {
+			startPosition: getNext ? ViewPosition.createAfter( node ) : ViewPosition.createBefore( node ),
+			direction: getNext ? 'forward' : 'backward'
+		} );
+
+		for ( const value of treeWalker ) {
+			if ( value.item.is( 'containerElement' ) ) {
+				// ViewContainerElement is found on a way to next ViewText node, so given `node` was first/last
+				// text node in its container element.
+				return null;
+			} else if ( value.item.is( 'text' ) ) {
+				// Found a text node in the same container element.
+				return value.item;
+			}
+		}
+
+		return null;
+	}
+
 	/**
 	 * Helper function. For given `Text` node, it finds previous or next sibling that is contained in the same block element.
 	 * If there is no such sibling, `null` is returned.

+ 3 - 23
packages/ckeditor5-engine/src/view/renderer.js

@@ -10,7 +10,6 @@
 import ViewText from './text';
 import ViewPosition from './position';
 import { INLINE_FILLER, INLINE_FILLER_LENGTH, startsWithFiller, isInlineFiller, isBlockFiller } from './filler';
-import { getTouchingTextNode } from './treewalker-utils';
 
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
 import diff from '@ckeditor/ckeditor5-utils/src/diff';
@@ -199,7 +198,9 @@ export default class Renderer {
 		}
 
 		for ( const node of this.markedTexts ) {
-			this._updateText( node, { inlineFillerPosition } );
+			if ( !this.markedChildren.has( node.parent ) && this.domConverter.mapViewToDom( node.parent ) ) {
+				this._updateText( node, { inlineFillerPosition } );
+			}
 		}
 
 		for ( const element of this.markedAttributes ) {
@@ -406,12 +407,6 @@ export default class Renderer {
 	 */
 	_updateText( viewText, options ) {
 		const domText = this.domConverter.findCorrespondingDomText( viewText );
-
-		// If this is a new text node and it is not in DOM, it will be created and handled in `_updateChildren`.
-		if ( !domText ) {
-			return;
-		}
-
 		const newDomText = this.domConverter.viewToDom( viewText, domText.ownerDocument );
 
 		const actualText = domText.data;
@@ -425,21 +420,6 @@ export default class Renderer {
 
 		if ( actualText != expectedText ) {
 			domText.data = expectedText;
-
-			// If we changed text node's data, we might need to re-render one or more following text nodes.
-			// This is because of space handling. If the text node starts with a space, that space might be
-			// changed to ` `. Whether it will be changed, depends on the previous text node's data.
-			// So when we change a text node, we need to refresh all following text nodes.
-			//
-			// Example:
-			// 1. Initial state:             <p>foo<em> bar</em></p>
-			// 2. Space is typed after foo:  <p>foo <em> bar</em></p>
-			// 3. After refresh:             <p>foo <em>&nbsp;bar</em><p>
-			const nextTouchingTextNode = getTouchingTextNode( viewText );
-
-			if ( nextTouchingTextNode ) {
-				this.markedTexts.add( nextTouchingTextNode );
-			}
 		}
 	}
 

+ 0 - 42
packages/ckeditor5-engine/src/view/treewalker-utils.js

@@ -1,42 +0,0 @@
-/**
- * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
- * For licensing, see LICENSE.md.
- */
-
-import ViewPosition from './position';
-import ViewTreeWalker from './treewalker';
-
-/**
- * Contains utility functions for working on view.
- *
- * @module engine/view/utils
- */
-
-/**
- * For given {@link module:engine/view/text~Text view text node}, it finds previous or next sibling that is contained
- * in the same container element. If there is no such sibling, `null` is returned.
- *
- * @param {module:engine/view/text~Text} node Reference node.
- * @param {'forward'|'backward'} [direction='forward'] If set to `'forward'`, next touching sibling will be returned.
- * If set to `'backward'` previous touching sibling will be returned.
- * @returns {module:engine/view/text~Text|null} Touching text node or `null` if there is no next or previous touching text node.
- */
-export function getTouchingTextNode( node, direction = 'forward' ) {
-	const treeWalker = new ViewTreeWalker( {
-		startPosition: direction == 'forward' ? ViewPosition.createAfter( node ) : ViewPosition.createBefore( node ),
-		direction
-	} );
-
-	for ( const value of treeWalker ) {
-		if ( value.item.is( 'containerElement' ) ) {
-			// ViewContainerElement is found on a way to next ViewText node, so given `node` was first/last
-			// text node in its container element.
-			return null;
-		} else if ( value.item.is( 'text' ) ) {
-			// Found a text node in the same container element.
-			return value.item;
-		}
-	}
-
-	return null;
-}

+ 5 - 16
packages/ckeditor5-engine/tests/view/renderer.js

@@ -214,29 +214,18 @@ describe( 'Renderer', () => {
 			expect( renderer.markedTexts.size ).to.equal( 0 );
 		} );
 
-		it( 'should update next touching text nodes of updated text node', () => {
+		it( 'should not update text parent child list changed', () => {
+			const viewImg = new ViewElement( 'img' );
 			const viewText = new ViewText( 'foo' );
-
-			viewRoot.appendChildren( [
-				viewText,
-				new ViewAttributeElement( 'strong', null, new ViewText( ' bar' ) )
-			] );
+			viewRoot.appendChildren( [ viewImg, viewText ] );
 
 			renderer.markToSync( 'children', viewRoot );
-			renderer.render();
-
-			expect( domRoot.childNodes.length ).to.equal( 2 );
-			expect( domRoot.childNodes[ 0 ].data ).to.equal( 'foo' );
-			expect( domRoot.childNodes[ 1 ].childNodes[ 0 ].data ).to.equal( ' bar' );
-
-			viewText.data = 'foo ';
-
 			renderer.markToSync( 'text', viewText );
 			renderer.render();
 
 			expect( domRoot.childNodes.length ).to.equal( 2 );
-			expect( domRoot.childNodes[ 0 ].data ).to.equal( 'foo ' );
-			expect( domRoot.childNodes[ 1 ].childNodes[ 0 ].data ).to.equal( '\u00A0bar' );
+			expect( domRoot.childNodes[ 0 ].tagName ).to.equal( 'IMG' );
+			expect( domRoot.childNodes[ 1 ].data ).to.equal( 'foo' );
 		} );
 
 		it( 'should not change text if it is the same during text rendering', () => {

+ 0 - 72
packages/ckeditor5-engine/tests/view/treewalker-utils.js

@@ -1,72 +0,0 @@
-/**
- * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
- * For licensing, see LICENSE.md.
- */
-
-import ViewAttributeElement from '../../src/view/attributeelement';
-import ViewContainerElement from '../../src/view/containerelement';
-import ViewText from '../../src/view/text';
-
-import { getTouchingTextNode } from '../../src/view/treewalker-utils';
-
-describe( 'getTouchingTextNode()', () => {
-	let a, b, x, y, z;
-
-	beforeEach( () => {
-		a = new ViewText( 'a' );
-		b = new ViewText( 'b' );
-		x = new ViewText( 'x' );
-		y = new ViewText( 'y' );
-		z = new ViewText( 'z' );
-
-		// <div><p>ab</p><p><em><strong>x</strong></em>y</p></div>
-		/* eslint-disable no-new */
-		new ViewContainerElement( 'div', null, [
-			new ViewContainerElement( 'p', null, [ a, b ] ),
-
-			new ViewContainerElement( 'p', null, [
-				new ViewAttributeElement( 'em', null, new ViewAttributeElement( 'strong', null, x ) ),
-				y
-			] ),
-
-			z,
-
-			new ViewContainerElement( 'p', null, [ new ViewText( '_' ) ] )
-		] );
-	} );
-
-	it( 'should return next touching view text node when direction is not specified', () => {
-		expect( getTouchingTextNode( a ) ).to.equal( b );
-	} );
-
-	it( 'should return next touching view text node', () => {
-		expect( getTouchingTextNode( a, 'forward' ) ).to.equal( b );
-	} );
-
-	it( 'should return previous touching view text node', () => {
-		expect( getTouchingTextNode( b, 'backward' ) ).to.equal( a );
-	} );
-
-	it( 'should go outside of attribute element looking for text nodes', () => {
-		expect( getTouchingTextNode( x, 'forward' ) ).to.equal( y );
-	} );
-
-	it( 'should go inside attribute element looking for text nodes', () => {
-		expect( getTouchingTextNode( y, 'backward' ) ).to.equal( x );
-	} );
-
-	it( 'should return null if there is no next text node in given container element', () => {
-		expect( getTouchingTextNode( b, 'forward' ) ).to.be.null;
-		expect( getTouchingTextNode( y, 'forward' ) ).to.be.null;
-	} );
-
-	it( 'should return null if there is no previous text node in given container element', () => {
-		expect( getTouchingTextNode( a, 'backward' ) ).to.be.null;
-		expect( getTouchingTextNode( x, 'backward' ) ).to.be.null;
-	} );
-
-	it( 'should not enter container element when looking for touching text node', () => {
-		expect( getTouchingTextNode( z ) ).to.be.null;
-		expect( getTouchingTextNode( z, 'backward' ) ).to.be.null;
-	} );
-} );