8
0
Quellcode durchsuchen

Rewrite the isBlockFiller method with logic from DomConvrter#domToView().

Maciej Gołaszewski vor 6 Jahren
Ursprung
Commit
9e9eb48c08

+ 1 - 9
packages/ckeditor5-engine/src/view/domconverter.js

@@ -387,15 +387,7 @@ export default class DomConverter {
 	 */
 	domToView( domNode, options = {} ) {
 		if ( isBlockFiller( domNode, this.blockFillerMode ) ) {
-			const isSingle = domNode.parentNode && domNode.parentNode.childNodes.length <= 1;
-
-			if ( isText( domNode ) ) {
-				if ( isSingle && _hasDomParentOfType( domNode, this.blockElements ) ) {
-					return null;
-				}
-			} else {
-				return null;
-			}
+			return null;
 		}
 
 		// When node is inside UIElement return that UIElement as it's view representation.

+ 25 - 14
packages/ckeditor5-engine/src/view/filler.js

@@ -6,6 +6,7 @@
 /* globals window */
 
 import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
+import getAncestors from '@ckeditor/ckeditor5-utils/src/dom/getancestors';
 import isText from '@ckeditor/ckeditor5-utils/src/dom/istext';
 
 /**
@@ -50,6 +51,9 @@ export const BR_FILLER = domDocument => {
 	return fillerBr;
 };
 
+// eslint-disable-next-line new-cap
+const BR_FILLER_REF = BR_FILLER( window.document );
+
 /**
  * Non-breaking space filler creator. This is a function which creates `&nbsp;` text node.
  * It defines how the filler is created.
@@ -119,30 +123,27 @@ export function getDataWithoutFiller( domText ) {
 	}
 }
 
-// Cache block fillers templates to improve performance.
-const templateBlockFillers = new WeakMap();
-
 /**
  * Checks if the node is an instance of the block filler of the given type.
  *
  *		const brFillerInstance = BR_FILLER( document );
- *		isBlockFiller( brFillerInstance, BR_FILLER ); // true
+ *		isBlockFiller( brFillerInstance, 'br' ); // true
+ *		isBlockFiller( brFillerInstance, 'nbsp' ); // false
  *
  * @param {Node} domNode DOM node to check.
- * @param {Function} blockFiller Block filler creator.
- * @returns {Boolean} True if text node contains only {@link module:engine/view/filler~INLINE_FILLER inline filler}.
+ * @param {String} blockFillerMode Block filler mode - either 'br' or 'nbsp'.
+ * @returns {Boolean} True if a node is considered a block filler for given mode.
  */
 export function isBlockFiller( domNode, blockFillerMode ) {
-	const blockFiller = blockFillerMode == 'br' ? BR_FILLER : NBSP_FILLER;
-
-	let templateBlockFiller = templateBlockFillers.get( blockFiller );
-
-	if ( !templateBlockFiller ) {
-		templateBlockFiller = blockFiller( window.document );
-		templateBlockFillers.set( blockFiller, templateBlockFiller );
+	if ( blockFillerMode == 'br' ) {
+		return domNode.isEqualNode( BR_FILLER_REF );
 	}
 
-	return domNode.isEqualNode( templateBlockFiller );
+	const isNBSP = isText( domNode ) && domNode.data == '\u00A0';
+	const isInsideBlockNode = _hasDomParentOfType( domNode, [ 'p', 'div', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6' ] );
+	const isSingle = !!domNode.parentNode && domNode.parentNode.childNodes.length === 1;
+
+	return isNBSP && isSingle && isInsideBlockNode;
 }
 
 /**
@@ -170,3 +171,13 @@ function jumpOverInlineFiller( evt, data ) {
 		}
 	}
 }
+
+function _hasDomParentOfType( node, types, boundaryParent ) {
+	let parents = getAncestors( node );
+
+	if ( boundaryParent ) {
+		parents = parents.slice( parents.indexOf( boundaryParent ) + 1 );
+	}
+
+	return parents.some( parent => parent.tagName && types.includes( parent.tagName.toLowerCase() ) );
+}

+ 20 - 4
packages/ckeditor5-engine/tests/view/filler.js

@@ -23,7 +23,7 @@ describe( 'filler', () => {
 		} );
 	} );
 
-	describe( 'startsWithFiller', () => {
+	describe( 'startsWithFiller()', () => {
 		it( 'should be true for node which contains only filler', () => {
 			const node = document.createTextNode( INLINE_FILLER );
 
@@ -67,7 +67,7 @@ describe( 'filler', () => {
 		} );
 	} );
 
-	describe( 'getDataWithoutFiller', () => {
+	describe( 'getDataWithoutFiller()', () => {
 		it( 'should return data without filler', () => {
 			const node = document.createTextNode( INLINE_FILLER + 'foo' );
 
@@ -87,7 +87,7 @@ describe( 'filler', () => {
 		} );
 	} );
 
-	describe( 'isInlineFiller', () => {
+	describe( 'isInlineFiller()', () => {
 		it( 'should be true for inline filler', () => {
 			const node = document.createTextNode( INLINE_FILLER );
 
@@ -123,7 +123,7 @@ describe( 'filler', () => {
 		} );
 	} );
 
-	describe( 'isBlockFiller', () => {
+	describe( 'isBlockFiller()', () => {
 		it( 'should return true if the node is an instance of the BR block filler', () => {
 			const brFillerInstance = BR_FILLER( document ); // eslint-disable-line new-cap
 
@@ -134,12 +134,28 @@ describe( 'filler', () => {
 
 		it( 'should return true if the node is an instance of the NBSP block filler', () => {
 			const nbspFillerInstance = NBSP_FILLER( document ); // eslint-disable-line new-cap
+			// NBSP must be check inside a context.
+			const context = document.createElement( 'div' );
+			context.appendChild( nbspFillerInstance );
 
 			expect( isBlockFiller( nbspFillerInstance, 'nbsp' ) ).to.be.true;
 			// Check it twice to ensure that caching breaks nothing.
 			expect( isBlockFiller( nbspFillerInstance, 'nbsp' ) ).to.be.true;
 		} );
 
+		it( 'should return false if the nbsp filler is inside context', () => {
+			const nbspFillerInstance = NBSP_FILLER( document ); // eslint-disable-line new-cap
+			// NBSP must be check inside a context.
+			const context = document.createElement( 'div' );
+			context.appendChild( nbspFillerInstance );
+			// eslint-disable-next-line new-cap
+			context.appendChild( NBSP_FILLER( document ) );
+
+			expect( isBlockFiller( nbspFillerInstance, 'nbsp' ) ).to.be.false;
+			// Check it twice to ensure that caching breaks nothing.
+			expect( isBlockFiller( nbspFillerInstance, 'nbsp' ) ).to.be.false;
+		} );
+
 		it( 'should return false for inline filler', () => {
 			expect( isBlockFiller( document.createTextNode( INLINE_FILLER ), 'br' ) ).to.be.false;
 			expect( isBlockFiller( document.createTextNode( INLINE_FILLER ), 'nbsp' ) ).to.be.false;