浏览代码

Move block filler handling to DomConverter.

Maciej Gołaszewski 6 年之前
父节点
当前提交
60241340b7

+ 57 - 12
packages/ckeditor5-engine/src/view/domconverter.js

@@ -7,7 +7,7 @@
  * @module engine/view/domconverter
  */
 
-/* globals document, Node, NodeFilter, Text */
+/* globals window, document, Node, NodeFilter, Text */
 
 import ViewText from './text';
 import ViewElement from './element';
@@ -16,15 +16,7 @@ import ViewRange from './range';
 import ViewSelection from './selection';
 import ViewDocumentFragment from './documentfragment';
 import ViewTreeWalker from './treewalker';
-import {
-	BR_FILLER,
-	NBSP_FILLER,
-	INLINE_FILLER_LENGTH,
-	isBlockFiller,
-	isInlineFiller,
-	startsWithFiller,
-	getDataWithoutFiller
-} from './filler';
+import { BR_FILLER, getDataWithoutFiller, INLINE_FILLER_LENGTH, isInlineFiller, NBSP_FILLER, startsWithFiller } from './filler';
 
 import global from '@ckeditor/ckeditor5-utils/src/dom/global';
 import indexOf from '@ckeditor/ckeditor5-utils/src/dom/indexof';
@@ -33,6 +25,9 @@ import getCommonAncestor from '@ckeditor/ckeditor5-utils/src/dom/getcommonancest
 import isText from '@ckeditor/ckeditor5-utils/src/dom/istext';
 import { isElement } from 'lodash-es';
 
+// eslint-disable-next-line new-cap
+const BR_FILLER_REF = BR_FILLER( window.document );
+
 /**
  * DomConverter is a set of tools to do transformations between DOM nodes and view nodes. It also handles
  * {@link module:engine/view/domconverter~DomConverter#bindElements binding} these nodes.
@@ -387,7 +382,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 ( isBlockFiller( domNode, this.blockFillerMode ) ) {
+		if ( this.isBlockFiller( domNode, this.blockFillerMode ) ) {
 			return null;
 		}
 
@@ -545,7 +540,7 @@ export default class DomConverter {
 	 * @returns {module:engine/view/position~Position} viewPosition View position.
 	 */
 	domPositionToView( domParent, domOffset ) {
-		if ( isBlockFiller( domParent, this.blockFillerMode ) ) {
+		if ( this.isBlockFiller( domParent, this.blockFillerMode ) ) {
 			return this.domPositionToView( domParent.parentNode, indexOf( domParent ) );
 		}
 
@@ -803,6 +798,23 @@ export default class DomConverter {
 	}
 
 	/**
+	 * Checks if the node is an instance of the block filler for this DOM converter.
+	 *
+	 *		const converter = new DomConverter( { blockFillerMode: 'br' } );
+	 *
+	 *		converter.isBlockFiller( BR_FILLER( document ) ); // true
+	 *		converter.isBlockFiller( NBSP_FILLER( document ) ); // false
+	 *
+	 * **Note:**: For the `'nbsp'` mode the method also checks context of a node so it cannot be a detached node.
+	 *
+	 * @param {Node} domNode DOM node to check.
+	 * @returns {Boolean} True if a node is considered a block filler for given mode.
+	 */
+	isBlockFiller( domNode ) {
+		return this.blockFillerMode == 'br' ? domNode.isEqualNode( BR_FILLER_REF ) : isNbspBlockFiller( domNode, this.blockElements );
+	}
+
+	/**
 	 * Returns `true` if given selection is a backward selection, that is, if it's `focus` is before `anchor`.
 	 *
 	 * @param {Selection} DOM Selection instance to check.
@@ -1213,3 +1225,36 @@ function forEachDomNodeAncestor( node, callback ) {
 		node = node.parentNode;
 	}
 }
+
+// Checks if given node is a nbsp block filler.
+//
+// A   is a block filler only if it is a single child of a block element.
+//
+// @param {Node} domNode DOM node.
+// @returns {Boolean}
+function isNbspBlockFiller( domNode, blockElements ) {
+	const isNBSP = isText( domNode ) && domNode.data == '\u00A0';
+
+	return isNBSP && hasBlockParent( domNode, blockElements ) && domNode.parentNode.childNodes.length === 1;
+}
+
+// Checks if domNode has block parent.
+//
+// @param {Node} domNode DOM node.
+// @returns {Boolean}
+function hasBlockParent( domNode, blockElements ) {
+	const parent = domNode.parentNode;
+
+	return parent && parent.tagName && blockElements.includes( parent.tagName.toLowerCase() );
+}
+
+/**
+ * Enum representing type of the block filler.
+ *
+ * Possible values:
+ *
+ * * `br` - for `<br>` block filler used in editing view,
+ * * `nbsp` - for `&nbsp;` block fillers used in the data.
+ *
+ * @typedef {String} module:engine/view/filler~BlockFillerMode
+ */

+ 9 - 67
packages/ckeditor5-engine/src/view/filler.js

@@ -3,8 +3,6 @@
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
 
-/* globals window */
-
 import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
 import isText from '@ckeditor/ckeditor5-utils/src/dom/istext';
 
@@ -37,6 +35,15 @@ import isText from '@ckeditor/ckeditor5-utils/src/dom/istext';
  */
 
 /**
+ * Non-breaking space filler creator. This is a function which creates `&nbsp;` text node.
+ * It defines how the filler is created.
+ *
+ * @see module:engine/view/filler~BR_FILLER
+ * @function
+ */
+export const NBSP_FILLER = domDocument => domDocument.createTextNode( '\u00A0' );
+
+/**
  * `<br>` filler creator. This is a function which creates `<br data-cke-filler="true">` element.
  * It defines how the filler is created.
  *
@@ -50,18 +57,6 @@ 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.
- *
- * @see module:engine/view/filler~BR_FILLER
- * @function
- */
-export const NBSP_FILLER = domDocument => domDocument.createTextNode( '\u00A0' );
-
 /**
  * Length of the {@link module:engine/view/filler~INLINE_FILLER INLINE_FILLER}.
  */
@@ -127,23 +122,6 @@ export function getDataWithoutFiller( domText ) {
 }
 
 /**
- * Checks if the node is an instance of the block filler of the given type.
- *
- *		const brFillerInstance = BR_FILLER( document );
- *		isBlockFiller( brFillerInstance, 'br' ); // true
- *		isBlockFiller( brFillerInstance, 'nbsp' ); // false
- *
- * **Note:**: For the `'nbsp'` mode the method also checks context of a node so it cannot be a detached node.
- *
- * @param {Node} domNode DOM node to check.
- * @param {module:engine/view/filler~BlockFillerMode} mode Mode of a block filler.
- * @returns {Boolean} True if a node is considered a block filler for given mode.
- */
-export function isBlockFiller( domNode, mode ) {
-	return mode == 'br' ? domNode.isEqualNode( BR_FILLER_REF ) : isNbspBlockFiller( domNode );
-}
-
-/**
  * Assign key observer which move cursor from the end of the inline filler to the beginning of it when
  * the left arrow is pressed, so the filler does not break navigation.
  *
@@ -168,39 +146,3 @@ function jumpOverInlineFiller( evt, data ) {
 		}
 	}
 }
-
-// Checks if given node is a nbsp block filler.
-//
-// A &nbsp; is a block filler only if it is a single child of a block element.
-//
-// @param {Node} domNode DOM node.
-// @returns {Boolean}
-function isNbspBlockFiller( domNode ) {
-	const isNBSP = isText( domNode ) && domNode.data == '\u00A0';
-
-	return isNBSP && hasBlockParent( domNode ) && domNode.parentNode.childNodes.length === 1;
-}
-
-// Name of DOM nodes that are considered a block.
-const blockElements = [ 'p', 'div', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6' ];
-
-// Checks if domNode has block parent.
-//
-// @param {Node} domNode DOM node.
-// @returns {Boolean}
-function hasBlockParent( domNode ) {
-	const parent = domNode.parentNode;
-
-	return parent && parent.tagName && blockElements.includes( parent.tagName.toLowerCase() );
-}
-
-/**
- * Enum representing type of the block filler.
- *
- * Possible values:
- *
- * * `br` - for `<br>` block filler used in editing view,
- * * `nbsp` - for `&nbsp;` block fillers used in the data.
- *
- * @typedef {String} module:engine/view/filler~BlockFillerMode
- */

+ 5 - 5
packages/ckeditor5-engine/src/view/renderer.js

@@ -11,7 +11,7 @@
 
 import ViewText from './text';
 import ViewPosition from './position';
-import { INLINE_FILLER, INLINE_FILLER_LENGTH, startsWithFiller, isInlineFiller, isBlockFiller } from './filler';
+import { INLINE_FILLER, INLINE_FILLER_LENGTH, startsWithFiller, isInlineFiller } from './filler';
 
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
 import diff from '@ckeditor/ckeditor5-utils/src/diff';
@@ -590,7 +590,7 @@ export default class Renderer {
 	_diffNodeLists( actualDomChildren, expectedDomChildren ) {
 		actualDomChildren = filterOutFakeSelectionContainer( actualDomChildren, this._fakeSelectionContainer );
 
-		return diff( actualDomChildren, expectedDomChildren, sameNodes.bind( null, this.domConverter.blockFillerMode ) );
+		return diff( actualDomChildren, expectedDomChildren, sameNodes.bind( null, this.domConverter ) );
 	}
 
 	/**
@@ -926,7 +926,7 @@ function areSimilar( node1, node2 ) {
 // @param {Node} node1
 // @param {Node} node2
 // @returns {Boolean}
-function sameNodes( blockFillerMode, actualDomChild, expectedDomChild ) {
+function sameNodes( domConverter, actualDomChild, expectedDomChild ) {
 	// Elements.
 	if ( actualDomChild === expectedDomChild ) {
 		return true;
@@ -936,8 +936,8 @@ function sameNodes( blockFillerMode, actualDomChild, expectedDomChild ) {
 		return actualDomChild.data === expectedDomChild.data;
 	}
 	// Block fillers.
-	else if ( isBlockFiller( actualDomChild, blockFillerMode ) &&
-		isBlockFiller( expectedDomChild, blockFillerMode ) ) {
+	else if ( domConverter.isBlockFiller( actualDomChild ) &&
+		domConverter.isBlockFiller( expectedDomChild ) ) {
 		return true;
 	}
 

+ 1 - 1
packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js

@@ -9,7 +9,7 @@ import ViewElement from '../../../src/view/element';
 import ViewDocumentSelection from '../../../src/view/documentselection';
 import DomConverter from '../../../src/view/domconverter';
 import ViewDocumentFragment from '../../../src/view/documentfragment';
-import { INLINE_FILLER, INLINE_FILLER_LENGTH, NBSP_FILLER, BR_FILLER } from '../../../src/view/filler';
+import { BR_FILLER, INLINE_FILLER, INLINE_FILLER_LENGTH, NBSP_FILLER } from '../../../src/view/filler';
 
 import { parse, stringify } from '../../../src/dev-utils/view';
 

+ 69 - 1
packages/ckeditor5-engine/tests/view/domconverter/domconverter.js

@@ -10,7 +10,7 @@ import ViewEditable from '../../../src/view/editableelement';
 import ViewDocument from '../../../src/view/document';
 import ViewUIElement from '../../../src/view/uielement';
 import ViewContainerElement from '../../../src/view/containerelement';
-import { INLINE_FILLER, INLINE_FILLER_LENGTH } from '../../../src/view/filler';
+import { BR_FILLER, INLINE_FILLER, INLINE_FILLER_LENGTH, NBSP_FILLER } from '../../../src/view/filler';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import global from '@ckeditor/ckeditor5-utils/src/dom/global';
 
@@ -290,4 +290,72 @@ describe( 'DomConverter', () => {
 			} );
 		} );
 	} );
+
+	describe( 'isBlockFiller()', () => {
+		describe( 'mode "nbsp"', () => {
+			beforeEach( () => {
+				converter = new DomConverter( { blockFillerMode: 'nbsp' } );
+			} );
+
+			it( 'should return true if the node is an instance of the NBSP block filler', () => {
+				converter = new DomConverter( { blockFillerMode: 'nbsp' } );
+				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( converter.isBlockFiller( nbspFillerInstance ) ).to.be.true;
+			} );
+
+			it( 'should return false if the node is an instance of the BR block filler', () => {
+				const brFillerInstance = BR_FILLER( document ); // eslint-disable-line new-cap
+
+				expect( converter.isBlockFiller( brFillerInstance ) ).to.be.false;
+			} );
+
+			it( 'should return false if the nbsp filler is inside context', () => {
+				converter = new DomConverter( { blockFillerMode: 'nbsp' } );
+				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( converter.isBlockFiller( nbspFillerInstance ) ).to.be.false;
+			} );
+
+			it( 'should return false for inline filler', () => {
+				expect( converter.isBlockFiller( document.createTextNode( INLINE_FILLER ) ) ).to.be.false;
+			} );
+		} );
+
+		describe( 'mode "br"', () => {
+			beforeEach( () => {
+				converter = new DomConverter( { blockFillerMode: 'br' } );
+			} );
+
+			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
+
+				expect( converter.isBlockFiller( brFillerInstance ) ).to.be.true;
+				// Check it twice to ensure that caching breaks nothing.
+				expect( converter.isBlockFiller( brFillerInstance ) ).to.be.true;
+			} );
+
+			it( 'should return false if the node is an instance of the NBSP block filler', () => {
+				converter = new DomConverter( { blockFillerMode: 'br' } );
+				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( converter.isBlockFiller( nbspFillerInstance ) ).to.be.false;
+			} );
+
+			it( 'should return false for inline filler', () => {
+				expect( converter.isBlockFiller( document.createTextNode( INLINE_FILLER ) ) ).to.be.false;
+			} );
+		} );
+	} );
 } );

+ 3 - 3
packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js

@@ -13,7 +13,7 @@ import ViewAttributeElement from '../../../src/view/attributeelement';
 import ViewEmptyElement from '../../../src/view/emptyelement';
 import DomConverter from '../../../src/view/domconverter';
 import ViewDocumentFragment from '../../../src/view/documentfragment';
-import { INLINE_FILLER, INLINE_FILLER_LENGTH, isBlockFiller } from '../../../src/view/filler';
+import { INLINE_FILLER, INLINE_FILLER_LENGTH } from '../../../src/view/filler';
 
 import { parse } from '../../../src/dev-utils/view';
 
@@ -634,7 +634,7 @@ describe( 'DomConverter', () => {
 			const domChildren = Array.from( converter.viewChildrenToDom( viewP, document ) );
 
 			expect( domChildren.length ).to.equal( 1 );
-			expect( isBlockFiller( domChildren[ 0 ], converter.blockFillerMode ) ).to.be.true;
+			expect( converter.isBlockFiller( domChildren[ 0 ] ) ).to.be.true;
 		} );
 
 		it( 'should add filler according to fillerPositionOffset', () => {
@@ -644,7 +644,7 @@ describe( 'DomConverter', () => {
 			const domChildren = Array.from( converter.viewChildrenToDom( viewP, document ) );
 
 			expect( domChildren.length ).to.equal( 2 );
-			expect( isBlockFiller( domChildren[ 0 ], converter.blockFillerMode ) ).to.be.true;
+			expect( converter.isBlockFiller( domChildren[ 0 ] ) ).to.be.true;
 			expect( domChildren[ 1 ].data ).to.equal( 'foo' );
 		} );
 

+ 0 - 42
packages/ckeditor5-engine/tests/view/filler.js

@@ -6,14 +6,11 @@
 /* globals document */
 
 import {
-	BR_FILLER,
-	NBSP_FILLER,
 	INLINE_FILLER_LENGTH,
 	INLINE_FILLER,
 	startsWithFiller,
 	isInlineFiller,
 	getDataWithoutFiller,
-	isBlockFiller
 } from '../../src/view/filler';
 
 describe( 'filler', () => {
@@ -122,43 +119,4 @@ describe( 'filler', () => {
 			document.body.removeChild( iframe );
 		} );
 	} );
-
-	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
-
-			expect( isBlockFiller( brFillerInstance, 'br' ) ).to.be.true;
-			// Check it twice to ensure that caching breaks nothing.
-			expect( isBlockFiller( brFillerInstance, 'br' ) ).to.be.true;
-		} );
-
-		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;
-		} );
-	} );
 } );

+ 4 - 4
packages/ckeditor5-engine/tests/view/renderer.js

@@ -21,7 +21,7 @@ import DocumentFragment from '../../src/view/documentfragment';
 import DowncastWriter from '../../src/view/downcastwriter';
 
 import { parse, setData as setViewData, getData as getViewData } from '../../src/dev-utils/view';
-import { INLINE_FILLER, INLINE_FILLER_LENGTH, isBlockFiller, BR_FILLER } from '../../src/view/filler';
+import { BR_FILLER, INLINE_FILLER, INLINE_FILLER_LENGTH } from '../../src/view/filler';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import createViewRoot from './_utils/createroot';
 import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';
@@ -804,7 +804,7 @@ describe( 'Renderer', () => {
 			// Step 3: Check whether in the first paragraph there's a <br> filler and that
 			// in the second one there are two <b> tags.
 			expect( domP.childNodes.length ).to.equal( 1 );
-			expect( isBlockFiller( domP.childNodes[ 0 ], 'br' ) ).to.be.true;
+			expect( domConverter.isBlockFiller( domP.childNodes[ 0 ] ) ).to.be.true;
 
 			expect( domP2.childNodes.length ).to.equal( 2 );
 			expect( domP2.childNodes[ 0 ].tagName.toLowerCase() ).to.equal( 'b' );
@@ -886,7 +886,7 @@ describe( 'Renderer', () => {
 			const domP = domRoot.childNodes[ 0 ];
 
 			expect( domP.childNodes.length ).to.equal( 1 );
-			expect( isBlockFiller( domP.childNodes[ 0 ], 'br' ) ).to.be.true;
+			expect( domConverter.isBlockFiller( domP.childNodes[ 0 ] ) ).to.be.true;
 
 			expect( domSelection.rangeCount ).to.equal( 1 );
 			expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domP );
@@ -925,7 +925,7 @@ describe( 'Renderer', () => {
 			const domP = domRoot.childNodes[ 0 ];
 
 			expect( domP.childNodes.length ).to.equal( 1 );
-			expect( isBlockFiller( domP.childNodes[ 0 ], 'br' ) ).to.be.true;
+			expect( domConverter.isBlockFiller( domP.childNodes[ 0 ] ) ).to.be.true;
 
 			expect( domSelection.rangeCount ).to.equal( 1 );
 			expect( domSelection.getRangeAt( 0 ).startContainer ).to.equal( domP );

+ 1 - 2
packages/ckeditor5-engine/tests/view/view/view.js

@@ -18,7 +18,6 @@ import ViewRange from '../../../src/view/range';
 import ViewElement from '../../../src/view/element';
 import ViewPosition from '../../../src/view/position';
 import ViewSelection from '../../../src/view/selection';
-import { isBlockFiller } from '../../../src/view/filler';
 
 import count from '@ckeditor/ckeditor5-utils/src/count';
 import global from '@ckeditor/ckeditor5-utils/src/dom/global';
@@ -565,7 +564,7 @@ describe( 'view', () => {
 			view.forceRender();
 
 			expect( domDiv.childNodes.length ).to.equal( 1 );
-			expect( isBlockFiller( domDiv.childNodes[ 0 ], 'br' ) ).to.be.true;
+			expect( view.domConverter.isBlockFiller( domDiv.childNodes[ 0 ] ) ).to.be.true;
 
 			view.destroy();
 			domDiv.remove();