diff --git a/src/dataprocessor/htmldataprocessor.js b/src/dataprocessor/htmldataprocessor.js index 66d9893fa..76565d1cd 100644 --- a/src/dataprocessor/htmldataprocessor.js +++ b/src/dataprocessor/htmldataprocessor.js @@ -11,7 +11,6 @@ import BasicHtmlWriter from './basichtmlwriter'; import DomConverter from '../view/domconverter'; -import { NBSP_FILLER } from '../view/filler'; /** * The HTML data processor class. @@ -38,7 +37,7 @@ export default class HtmlDataProcessor { * @private * @member {module:engine/view/domconverter~DomConverter} */ - this._domConverter = new DomConverter( { blockFiller: NBSP_FILLER } ); + this._domConverter = new DomConverter( { blockFillerMode: 'nbsp' } ); /** * A basic HTML writer instance used to convert DOM elements to an HTML string. diff --git a/src/dataprocessor/xmldataprocessor.js b/src/dataprocessor/xmldataprocessor.js index 29d55f284..584637558 100644 --- a/src/dataprocessor/xmldataprocessor.js +++ b/src/dataprocessor/xmldataprocessor.js @@ -11,7 +11,6 @@ import BasicHtmlWriter from './basichtmlwriter'; import DomConverter from '../view/domconverter'; -import { NBSP_FILLER } from '../view/filler'; /** * The XML data processor class. @@ -54,7 +53,7 @@ export default class XmlDataProcessor { * @private * @member {module:engine/view/domconverter~DomConverter} */ - this._domConverter = new DomConverter( { blockFiller: NBSP_FILLER } ); + this._domConverter = new DomConverter( { blockFillerMode: 'nbsp' } ); /** * A basic HTML writer instance used to convert DOM elements to an XML string. diff --git a/src/view/domconverter.js b/src/view/domconverter.js index f7c61b4e6..bd908d84d 100644 --- a/src/view/domconverter.js +++ b/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, 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'; @@ -25,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( 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. @@ -42,30 +45,19 @@ export default class DomConverter { * Creates DOM converter. * * @param {Object} options Object with configuration options. - * @param {Function} [options.blockFiller=module:engine/view/filler~BR_FILLER] Block filler creator. + * @param {module:engine/view/filler~BlockFillerMode} [options.blockFillerMode='br'] The type of the block filler to use. */ constructor( options = {} ) { - // Using WeakMap prevent memory leaks: when the converter will be destroyed all referenced between View and DOM - // will be removed. Also because it is a *Weak*Map when both view and DOM elements will be removed referenced - // will be also removed, isn't it brilliant? - // - // Yes, PJ. It is. - // - // You guys so smart. - // - // I've been here. Seen stuff. Afraid of code now. - /** - * Block {@link module:engine/view/filler filler} creator, which is used to create all block fillers during the - * view to DOM conversion and to recognize block fillers during the DOM to view conversion. + * The mode of a block filler used by DOM converter. * * @readonly - * @member {Function} module:engine/view/domconverter~DomConverter#blockFiller + * @member {'br'|'nbsp'} module:engine/view/domconverter~DomConverter#blockFillerMode */ - this.blockFiller = options.blockFiller || BR_FILLER; + this.blockFillerMode = options.blockFillerMode || 'br'; /** - * Tag names of DOM `Element`s which are considered pre-formatted elements. + * Elements which are considered pre-formatted elements. * * @readonly * @member {Array.} module:engine/view/domconverter~DomConverter#preElements @@ -73,12 +65,27 @@ export default class DomConverter { this.preElements = [ 'pre' ]; /** - * Tag names of DOM `Element`s which are considered block elements. + * Elements which are considered block elements (and hence should be filled with a + * {@link ~isBlockFiller block filler}). + * + * Whether an element is considered a block element also affects handling of trailing whitespaces. + * + * You can extend this array if you introduce support for block elements which are not yet recognized here. * * @readonly * @member {Array.} module:engine/view/domconverter~DomConverter#blockElements */ - this.blockElements = [ 'p', 'div', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6' ]; + this.blockElements = [ 'p', 'div', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'li', 'dd', 'dt', 'figcaption' ]; + + /** + * Block {@link module:engine/view/filler filler} creator, which is used to create all block fillers during the + * view to DOM conversion and to recognize block fillers during the DOM to view conversion. + * + * @readonly + * @private + * @member {Function} module:engine/view/domconverter~DomConverter#_blockFiller + */ + this._blockFiller = this.blockFillerMode == 'br' ? BR_FILLER : NBSP_FILLER; /** * DOM to View mapping. @@ -255,7 +262,7 @@ export default class DomConverter { for ( const childView of viewElement.getChildren() ) { if ( fillerPositionOffset === offset ) { - yield this.blockFiller( domDocument ); + yield this._blockFiller( domDocument ); } yield this.viewToDom( childView, domDocument, options ); @@ -264,7 +271,7 @@ export default class DomConverter { } if ( fillerPositionOffset === offset ) { - yield this.blockFiller( domDocument ); + yield this._blockFiller( domDocument ); } } @@ -371,7 +378,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.blockFiller ) ) { + if ( this.isBlockFiller( domNode, this.blockFillerMode ) ) { return null; } @@ -529,7 +536,7 @@ export default class DomConverter { * @returns {module:engine/view/position~Position} viewPosition View position. */ domPositionToView( domParent, domOffset ) { - if ( isBlockFiller( domParent, this.blockFiller ) ) { + if ( this.isBlockFiller( domParent, this.blockFillerMode ) ) { return this.domPositionToView( domParent.parentNode, indexOf( domParent ) ); } @@ -786,6 +793,23 @@ export default class DomConverter { return node && node.nodeType == Node.COMMENT_NODE; } + /** + * 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`. * @@ -1197,3 +1221,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 `
` block filler used in editing view, + * * `nbsp` - for ` ` block fillers used in the data. + * + * @typedef {String} module:engine/view/filler~BlockFillerMode + */ diff --git a/src/view/filler.js b/src/view/filler.js index aad0890fc..f8bca06a6 100644 --- a/src/view/filler.js +++ b/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'; @@ -36,6 +34,15 @@ import isText from '@ckeditor/ckeditor5-utils/src/dom/istext'; * @module engine/view/filler */ +/** + * Non-breaking space filler creator. This is a function which creates ` ` 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' ); + /** * `
` filler creator. This is a function which creates `
` element. * It defines how the filler is created. @@ -50,28 +57,23 @@ export const BR_FILLER = domDocument => { return fillerBr; }; -/** - * Non-breaking space filler creator. This is a function which creates ` ` 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}. */ export const INLINE_FILLER_LENGTH = 7; /** - * Inline filler which is sequence of the zero width spaces. + * Inline filler which is a sequence of the zero width spaces. */ -export let INLINE_FILLER = ''; +export const INLINE_FILLER = ( () => { + let inlineFiller = ''; -for ( let i = 0; i < INLINE_FILLER_LENGTH; i++ ) { - INLINE_FILLER += '\u200b'; -} + for ( let i = 0; i < INLINE_FILLER_LENGTH; i++ ) { + inlineFiller += '\u200b'; + } + + return inlineFiller; +} )(); // Usu IIF so the INLINE_FILLER appears as a constant in the docs. /** * Checks if the node is a text node which starts with the {@link module:engine/view/filler~INLINE_FILLER inline filler}. @@ -119,30 +121,6 @@ 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 - * - * @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}. - */ -export function isBlockFiller( domNode, blockFiller ) { - let templateBlockFiller = templateBlockFillers.get( blockFiller ); - - if ( !templateBlockFiller ) { - templateBlockFiller = blockFiller( window.document ); - templateBlockFillers.set( blockFiller, templateBlockFiller ); - } - - return domNode.isEqualNode( templateBlockFiller ); -} - /** * 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. diff --git a/src/view/renderer.js b/src/view/renderer.js index a44fa10c6..e2f7435a4 100644 --- a/src/view/renderer.js +++ b/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.blockFiller ) ); + return diff( actualDomChildren, expectedDomChildren, sameNodes.bind( null, this.domConverter ) ); } /** @@ -938,11 +938,11 @@ function areSimilar( node1, node2 ) { // * Two block filler elements. // // @private -// @param {Function} blockFiller Block filler creator function, see {@link module:engine/view/domconverter~DomConverter#blockFiller}. +// @param {String} blockFillerMode Block filler mode, see {@link module:engine/view/domconverter~DomConverter#blockFillerMode}. // @param {Node} node1 // @param {Node} node2 // @returns {Boolean} -function sameNodes( blockFiller, actualDomChild, expectedDomChild ) { +function sameNodes( domConverter, actualDomChild, expectedDomChild ) { // Elements. if ( actualDomChild === expectedDomChild ) { return true; @@ -952,8 +952,8 @@ function sameNodes( blockFiller, actualDomChild, expectedDomChild ) { return actualDomChild.data === expectedDomChild.data; } // Block fillers. - else if ( isBlockFiller( actualDomChild, blockFiller ) && - isBlockFiller( expectedDomChild, blockFiller ) ) { + else if ( domConverter.isBlockFiller( actualDomChild ) && + domConverter.isBlockFiller( expectedDomChild ) ) { return true; } diff --git a/tests/dataprocessor/htmldataprocessor.js b/tests/dataprocessor/htmldataprocessor.js index 8828916f1..f881f0101 100644 --- a/tests/dataprocessor/htmldataprocessor.js +++ b/tests/dataprocessor/htmldataprocessor.js @@ -59,33 +59,32 @@ describe( 'HtmlDataProcessor', () => { } ); } - // Uncomment this test after fixing #404. - // describe( 'https://github.com/ckeditor/ckeditor5-clipboard/issues/2#issuecomment-310417731 + #404', () => { - // it( 'does not lose whitespaces in Chrome\'s paste-like content', () => { - // const fragment = dataProcessor.toView( - // '' + - // 'This is the\u00a0' + - // 'third developer preview' + - // '\u00a0of\u00a0' + - // 'CKEditor\u00a05' + - // '.' - // ); - - // expect( stringify( fragment ) ).to.equal( - // 'This is the\u00a0' + - // 'third developer preview' + - // '\u00a0of\u00a0' + - // 'CKEditor\u00a05' + - // '.' - // ); - - // // Just to be sure... stringify() uses conversion and the browser extensively, - // // so it's not entirely safe. - // expect( fragment.getChild( 0 ).getChild( 1 ).getChild( 0 ).data ).to.equal( '\u00a0' ); - // expect( fragment.getChild( 2 ).getChild( 0 ).getChild( 0 ).data ).to.equal( '\u00a0' ); - // expect( fragment.getChild( 2 ).getChild( 2 ).getChild( 0 ).data ).to.equal( '\u00a0' ); - // } ); - // } ); + describe( 'https://github.com/ckeditor/ckeditor5-clipboard/issues/2#issuecomment-310417731 + #404', () => { + it( 'does not lose whitespaces in Chrome\'s paste-like content', () => { + const fragment = dataProcessor.toView( + '' + + 'This is the\u00a0' + + 'third developer preview' + + '\u00a0of\u00a0' + + 'CKEditor\u00a05' + + '.' + ); + + expect( stringify( fragment ) ).to.equal( + 'This is the\u00a0' + + 'third developer preview' + + '\u00a0of\u00a0' + + 'CKEditor\u00a05' + + '.' + ); + + // Just to be sure... stringify() uses conversion and the browser extensively, + // so it's not entirely safe. + expect( fragment.getChild( 0 ).getChild( 1 ).getChild( 0 ).data ).to.equal( '\u00a0' ); + expect( fragment.getChild( 2 ).getChild( 0 ).getChild( 0 ).data ).to.equal( '\u00a0' ); + expect( fragment.getChild( 2 ).getChild( 2 ).getChild( 0 ).data ).to.equal( '\u00a0' ); + } ); + } ); } ); describe( 'toData()', () => { diff --git a/tests/view/domconverter/dom-to-view.js b/tests/view/domconverter/dom-to-view.js index f5f9b43fa..5880cea45 100644 --- a/tests/view/domconverter/dom-to-view.js +++ b/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 } 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'; @@ -157,7 +157,8 @@ describe( 'DomConverter', () => { } ); it( 'should return null for block filler', () => { - const domFiller = converter.blockFiller( document ); + // eslint-disable-next-line new-cap + const domFiller = BR_FILLER( document ); expect( converter.domToView( domFiller ) ).to.be.null; } ); @@ -682,7 +683,8 @@ describe( 'DomConverter', () => { } ); it( 'should skip filler', () => { - const domFiller = converter.blockFiller( document ); + // eslint-disable-next-line new-cap + const domFiller = BR_FILLER( document ); const domP = createElement( document, 'p', null, domFiller ); const viewChildren = Array.from( converter.domChildrenToView( domP ) ); @@ -761,7 +763,7 @@ describe( 'DomConverter', () => { } ); it( 'should converter position inside block filler', () => { - const converter = new DomConverter( { blockFiller: NBSP_FILLER } ); + const converter = new DomConverter( { blockFillerMode: 'nbsp' } ); const domFiller = NBSP_FILLER( document ); // eslint-disable-line new-cap const domP = createElement( document, 'p', null, domFiller ); diff --git a/tests/view/domconverter/domconverter.js b/tests/view/domconverter/domconverter.js index 661c181a0..0a276ae5b 100644 --- a/tests/view/domconverter/domconverter.js +++ b/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 { BR_FILLER, NBSP_FILLER, 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'; @@ -24,13 +24,13 @@ describe( 'DomConverter', () => { } ); describe( 'constructor()', () => { - it( 'should create converter with BR block filler by default', () => { - expect( converter.blockFiller ).to.equal( BR_FILLER ); + it( 'should create converter with BR block filler mode by default', () => { + expect( converter.blockFillerMode ).to.equal( 'br' ); } ); - it( 'should create converter with defined block filler', () => { - converter = new DomConverter( { blockFiller: NBSP_FILLER } ); - expect( converter.blockFiller ).to.equal( NBSP_FILLER ); + it( 'should create converter with defined block mode filler', () => { + converter = new DomConverter( { blockFillerMode: 'nbsp' } ); + expect( converter.blockFillerMode ).to.equal( 'nbsp' ); } ); } ); @@ -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; + } ); + } ); + } ); } ); diff --git a/tests/view/domconverter/view-to-dom.js b/tests/view/domconverter/view-to-dom.js index 475db8c03..5948600fa 100644 --- a/tests/view/domconverter/view-to-dom.js +++ b/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.blockFiller ) ).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.blockFiller ) ).to.be.true; + expect( converter.isBlockFiller( domChildren[ 0 ] ) ).to.be.true; expect( domChildren[ 1 ].data ).to.equal( 'foo' ); } ); diff --git a/tests/view/domconverter/whitespace-handling-integration.js b/tests/view/domconverter/whitespace-handling-integration.js index 13eea812e..156d2549e 100644 --- a/tests/view/domconverter/whitespace-handling-integration.js +++ b/tests/view/domconverter/whitespace-handling-integration.js @@ -23,6 +23,9 @@ describe( 'DomConverter – whitespace handling – integration', () => { .create( { plugins: [ Paragraph ] } ) .then( newEditor => { editor = newEditor; + + editor.model.schema.extend( '$text', { allowAttributes: [ 'bold' ] } ); + editor.conversion.attributeToElement( { model: 'bold', view: 'b' } ); } ); } ); @@ -48,14 +51,13 @@ describe( 'DomConverter – whitespace handling – integration', () => { expect( editor.getData() ).to.equal( '

foo

' ); } ); - // Controversial result. See https://github.com/ckeditor/ckeditor5-engine/issues/987. it( 'nbsp at the end of the content is not ignored', () => { - editor.setData( '

foo

' ); + editor.setData( '

foo 

' ); expect( getData( editor.model, { withoutSelection: true } ) ) - .to.equal( 'foo' ); + .to.equal( 'foo ' ); - expect( editor.getData() ).to.equal( '

foo

' ); + expect( editor.getData() ).to.equal( '

foo 

' ); } ); it( 'new line at the beginning of the content is ignored', () => { @@ -76,14 +78,13 @@ describe( 'DomConverter – whitespace handling – integration', () => { expect( editor.getData() ).to.equal( '

foo

' ); } ); - // Controversial result. See https://github.com/ckeditor/ckeditor5-engine/issues/987. it( 'nbsp at the beginning of the content is not ignored', () => { - editor.setData( '

foo

' ); + editor.setData( '

 foo

' ); expect( getData( editor.model, { withoutSelection: true } ) ) - .to.equal( 'foo' ); + .to.equal( ' foo' ); - expect( editor.getData() ).to.equal( '

foo

' ); + expect( editor.getData() ).to.equal( '

 foo

' ); } ); it( 'new line between blocks is ignored', () => { @@ -105,13 +106,33 @@ describe( 'DomConverter – whitespace handling – integration', () => { } ); // Controversial result. See https://github.com/ckeditor/ckeditor5-engine/issues/987. - it( 'nbsp between blocks is not ignored', () => { + it( 'nbsp between blocks is not ignored (between paragraphs)', () => { editor.setData( '

foo

 

bar

' ); expect( getData( editor.model, { withoutSelection: true } ) ) - .to.equal( 'foobar' ); + .to.equal( 'foo bar' ); - expect( editor.getData() ).to.equal( '

foo

bar

' ); + expect( editor.getData() ).to.equal( '

foo

 

bar

' ); + } ); + + it( 'nbsp between blocks is not ignored (different blocks)', () => { + editor.model.schema.register( 'block', { inheritAllFrom: '$block' } ); + editor.conversion.elementToElement( { model: 'block', view: 'block' } ); + editor.setData( 'foo 

bar

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( + 'foo' + + ' ' + + 'bar' + ); + + expect( editor.getData() ) + .to.equal( + 'foo' + + '

 

' + + '

bar

' + ); } ); it( 'new lines inside blocks are ignored', () => { @@ -141,6 +162,15 @@ describe( 'DomConverter – whitespace handling – integration', () => { expect( editor.getData() ).to.equal( '

 foo 

' ); } ); + it( 'single nbsp inside blocks are ignored', () => { + editor.setData( '

 

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( '' ); + + expect( editor.getData() ).to.equal( '' ); // trimmed + } ); + it( 'all whitespaces together are ignored', () => { editor.setData( '\n

foo\n\r\n \t

\n

bar

' ); @@ -149,6 +179,33 @@ describe( 'DomConverter – whitespace handling – integration', () => { expect( editor.getData() ).to.equal( '

foo

bar

' ); } ); + + it( 'nbsp between inline elements is not ignored', () => { + editor.setData( '

foo bar

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( '<$text bold="true">foo\u00A0<$text bold="true">bar' ); + + expect( editor.getData() ).to.equal( '

foo bar

' ); + } ); + + it( 'nbsp before inline element is not ignored', () => { + editor.setData( '

 bar

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( ' <$text bold="true">bar' ); + + expect( editor.getData() ).to.equal( '

 bar

' ); + } ); + + it( 'nbsp after inline element is not ignored', () => { + editor.setData( '

bar 

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( '<$text bold="true">bar ' ); + + expect( editor.getData() ).to.equal( '

bar 

' ); + } ); } ); // https://github.com/ckeditor/ckeditor5/issues/1024 diff --git a/tests/view/filler.js b/tests/view/filler.js index 4390fda27..8f0ec00d4 100644 --- a/tests/view/filler.js +++ b/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', () => { @@ -23,7 +20,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 +64,7 @@ describe( 'filler', () => { } ); } ); - describe( 'getDataWithoutFiller', () => { + describe( 'getDataWithoutFiller()', () => { it( 'should return data without filler', () => { const node = document.createTextNode( INLINE_FILLER + 'foo' ); @@ -87,7 +84,7 @@ describe( 'filler', () => { } ); } ); - describe( 'isInlineFiller', () => { + describe( 'isInlineFiller()', () => { it( 'should be true for inline filler', () => { const node = document.createTextNode( INLINE_FILLER ); @@ -122,27 +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_FILLER ) ).to.be.true; - // Check it twice to ensure that caching breaks nothing. - expect( isBlockFiller( brFillerInstance, BR_FILLER ) ).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 - - expect( isBlockFiller( nbspFillerInstance, NBSP_FILLER ) ).to.be.true; - // Check it twice to ensure that caching breaks nothing. - expect( isBlockFiller( nbspFillerInstance, NBSP_FILLER ) ).to.be.true; - } ); - - it( 'should return false for inline filler', () => { - expect( isBlockFiller( document.createTextNode( INLINE_FILLER ), BR_FILLER ) ).to.be.false; - expect( isBlockFiller( document.createTextNode( INLINE_FILLER ), NBSP_FILLER ) ).to.be.false; - } ); - } ); } ); diff --git a/tests/view/renderer.js b/tests/view/renderer.js index 87150dae7..2c9f15078 100644 --- a/tests/view/renderer.js +++ b/tests/view/renderer.js @@ -21,7 +21,7 @@ import DocumentFragment from '../../src/view/documentfragment'; import DowncastWriter from '../../src/view/downcastwriter'; import { parse, stringify, 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
filler and that // in the second one there are two tags. expect( domP.childNodes.length ).to.equal( 1 ); - expect( isBlockFiller( domP.childNodes[ 0 ], BR_FILLER ) ).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_FILLER ) ).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_FILLER ) ).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 ); diff --git a/tests/view/view/view.js b/tests/view/view/view.js index d8ccd3aa3..b26a98391 100644 --- a/tests/view/view/view.js +++ b/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, BR_FILLER } 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_FILLER ) ).to.be.true; + expect( view.domConverter.isBlockFiller( domDiv.childNodes[ 0 ] ) ).to.be.true; view.destroy(); domDiv.remove();