Преглед на файлове

Change the way how isBlockFiller check nbps fillers.

Maciej Gołaszewski преди 6 години
родител
ревизия
45f1036539

+ 2 - 41
packages/ckeditor5-engine/src/view/domconverter.js

@@ -16,7 +16,7 @@ import ViewRange from './range';
 import ViewSelection from './selection';
 import ViewDocumentFragment from './documentfragment';
 import ViewTreeWalker from './treewalker';
-import { BR_FILLER, getDataWithoutFiller, INLINE_FILLER_LENGTH, isBlockFiller, isInlineFiller, startsWithFiller } from './filler';
+import { BR_FILLER, INLINE_FILLER_LENGTH, isBlockFiller, isInlineFiller, startsWithFiller, getDataWithoutFiller } from './filler';
 
 import global from '@ckeditor/ckeditor5-utils/src/dom/global';
 import indexOf from '@ckeditor/ckeditor5-utils/src/dom/indexof';
@@ -81,14 +81,6 @@ export default class DomConverter {
 		this.blockElements = [ 'p', 'div', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6' ];
 
 		/**
-		 * Tag names of DOM `Element`s which are considered inline elements.
-		 *
-		 * @readonly
-		 * @member {Array.<String>} module:engine/view/domconverter~DomConverter#inlineElements
-		 */
-		this.inlineElements = [ 'span', 'strong', 'i', 'b', 'a' ];
-
-		/**
 		 * DOM to View mapping.
 		 *
 		 * @private
@@ -378,7 +370,7 @@ export default class DomConverter {
 	 * or `null` if DOM node is a {@link module:engine/view/filler filler} or the given node is an empty text node.
 	 */
 	domToView( domNode, options = {} ) {
-		if ( isNegligibleBlockFiller( domNode, this.blockFiller, this.inlineElements, this.blockElements ) ) {
+		if ( isBlockFiller( domNode, this.blockFiller ) ) {
 			return null;
 		}
 
@@ -1204,34 +1196,3 @@ function forEachDomNodeAncestor( node, callback ) {
 		node = node.parentNode;
 	}
 }
-
-// Checks if given node is negligible and should be removed.
-// The negligible block fillers are:
-// - &nbsp; between block nodes
-// - single &nbsp; in blocks
-// - &nbsp; between blocks
-//
-// The relevant block fillers are:
-// - &nbsp; between inline elements
-// - &nbsp; inside inline elements
-function isNegligibleBlockFiller( domNode, blockFiller, inlineElements ) {
-	if ( !isBlockFiller( domNode, blockFiller ) ) {
-		return false;
-	}
-
-	const isSingleDomNode = !domNode.parentNode || domNode.parentNode.childNodes.length === 1;
-
-	if ( isSingleDomNode ) {
-		// Single DOM nodes are negligible - unless they are in inline element.
-		return !isInlineElement( domNode.parentNode, inlineElements );
-	}
-
-	const prevIsInline = isInlineElement( domNode.previousSibling, inlineElements );
-	const nextIsInline = isInlineElement( domNode.nextSibling, inlineElements );
-
-	return !( prevIsInline || nextIsInline );
-}
-
-function isInlineElement( domNode, types ) {
-	return domNode && types.includes( domNode.tagName.toLowerCase() );
-}

+ 80 - 1
packages/ckeditor5-engine/src/view/filler.js

@@ -140,7 +140,9 @@ export function isBlockFiller( domNode, blockFiller ) {
 		templateBlockFillers.set( blockFiller, templateBlockFiller );
 	}
 
-	return domNode.isEqualNode( templateBlockFiller );
+	const isFiller = domNode.isEqualNode( templateBlockFiller );
+
+	return isText( templateBlockFiller ) ? isFiller && !hasInlineSibling( domNode ) : isFiller;
 }
 
 /**
@@ -168,3 +170,80 @@ function jumpOverInlineFiller( evt, data ) {
 		}
 	}
 }
+
+// Checks if domNode has inline sibling.
+//
+// @param {Node} domNode DOM node.
+// @returns {Boolean}
+function hasInlineSibling( domNode ) {
+	const hasParent = !!domNode.parentNode;
+
+	if ( !hasParent || domNode.parentNode.childNodes.length <= 1 ) {
+		return false;
+	}
+
+	const prevIsInline = isInlineElement( domNode.previousSibling );
+	const nextIsInline = isInlineElement( domNode.nextSibling );
+
+	return prevIsInline || nextIsInline;
+}
+
+// Tag names of DOM `Element`s which are considered block elements.
+// todo which other inline elements to include?
+// - abbr
+// - acronym
+// - audio (if it has visible controls)
+// - bdi
+// - bdo
+// - big
+// - br
+// - button
+// - canvas
+// - cite
+// - code
+// - data
+// - datalist
+// - del
+// - dfn
+// - em
+// - embed
+// - iframe
+// - img
+// - input
+// - ins
+// - kbd
+// - label
+// - map
+// - mark
+// - meter
+// - noscript
+// - object
+// - output
+// - picture
+// - progress
+// - q
+// - ruby
+// - s
+// - samp
+// - script
+// - select
+// - slot
+// - small
+// - strong
+// - sub
+// - sup
+// - svg
+// - template
+// - textarea
+// - time
+// - u
+// - tt
+// - var
+// - video
+// - wbr
+const inlineElements = [ 'a', 'b', 'span', 'i', 'span' ];
+
+// Checks if passed domNode is considered inline.
+function isInlineElement( domNode ) {
+	return domNode && inlineElements.includes( domNode.nodeName.toLowerCase() );
+}

+ 14 - 5
packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js

@@ -183,7 +183,7 @@ describe( 'DomConverter – whitespace handling – integration', () => {
 			expect( editor.getData() ).to.equal( '<p>foo</p><p>bar</p>' );
 		} );
 
-		it( 'nbsp between inline elements inside blocks are not ignored', () => {
+		it( 'nbsp between inline elements is not ignored', () => {
 			editor.setData( '<p><b>foo</b>&nbsp;<b>bar</b></p>' );
 
 			expect( getData( editor.model, { withoutSelection: true } ) )
@@ -192,13 +192,22 @@ describe( 'DomConverter – whitespace handling – integration', () => {
 			expect( editor.getData() ).to.equal( '<p><b>foo</b>&nbsp;<b>bar</b></p>' );
 		} );
 
-		it( 'nbsp inside inline element is not ignored', () => {
-			editor.setData( '<p>Foo<b>&nbsp;</b></p>' );
+		it( 'nbsp before inline element is not ignored', () => {
+			editor.setData( '<p>&nbsp;<b>bar</b></p>' );
 
 			expect( getData( editor.model, { withoutSelection: true } ) )
-				.to.equal( '<paragraph>Foo<$text bold="true"> </$text></paragraph>' );
+				.to.equal( '<paragraph> <$text bold="true">bar</$text></paragraph>' );
 
-			expect( editor.getData() ).to.equal( '<p>Foo<b>&nbsp;</b></p>' );
+			expect( editor.getData() ).to.equal( '<p>&nbsp;<b>bar</b></p>' );
+		} );
+
+		it( 'nbsp after inline element is not ignored', () => {
+			editor.setData( '<p><b>bar</b>&nbsp;</p>' );
+
+			expect( getData( editor.model, { withoutSelection: true } ) )
+				.to.equal( '<paragraph><$text bold="true">bar</$text> </paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p><b>bar</b>&nbsp;</p>' );
 		} );
 	} );