Skip to content

Commit

Permalink
Merge pull request #9357 from ckeditor/i/9345
Browse files Browse the repository at this point in the history
Feature (engine): Introduced new block filler mode "markedNbsp" in `DomConverter`, in which `<span data-cke-filler="true">&nbsp;</span>` are inserted, to prevent leaking extra space characters into the data.

Feature (engine): Introduced `useFillerType()` in `HtmlDataProcessor` and `XmlDataProcessor` to switch between using marked and regular nbsp block fillers. Closes #9345.

MINOR BREAKING CHANGE (engine): Added new method `useFillerType()` in `DataProcessor` interface. Classes that base on this interface should implement `useFillerType()` to avoid errors.
  • Loading branch information
niegowski authored Apr 14, 2021
2 parents 5b99c75 + 93b106b commit 5217b30
Show file tree
Hide file tree
Showing 11 changed files with 264 additions and 86 deletions.
13 changes: 13 additions & 0 deletions packages/ckeditor5-engine/src/dataprocessor/dataprocessor.jsdoc
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,16 @@
* @param {module:engine/view/matcher~MatcherPattern} pattern Pattern matching all view elements whose content should
* be treated as plain text.
*/

/**
* If the processor is set to use marked fillers, it will insert nbsp fillers wrapped in spans
* (`<span data-cke-filler="true">&nbsp;</span>`), instead of regular nbsp characters (`&nbsp;`).
*
* This mode allows for more precise handling of block fillers (so they don't leak into editor content) but bloats the editor
* data with additional markup.
*
* This mode may be required by some features and will be turned on by them automatically.
*
* @method #useFillerType
* @param {'default'|'marked'} type Whether to use default or marked nbsp block fillers.
*/
15 changes: 15 additions & 0 deletions packages/ckeditor5-engine/src/dataprocessor/htmldataprocessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,21 @@ export default class HtmlDataProcessor {
this._domConverter.registerRawContentMatcher( pattern );
}

/**
* If the processor is set to use marked fillers, it will insert nbsp fillers wrapped in spans
* (`<span data-cke-filler="true">&nbsp;</span>`), instead of regular nbsp characters (`&nbsp;`).
*
* This mode allows for more precise handling of block fillers (so they don't leak into editor content) but bloats the editor
* data with additional markup.
*
* This mode may be required by some features and will be turned on by them automatically.
*
* @param {'default'|'marked'} type Whether to use default or marked nbsp block fillers.
*/
useFillerType( type ) {
this._domConverter.blockFillerMode = type == 'marked' ? 'markedNbsp' : 'nbsp';
}

/**
* Converts an HTML string to its DOM representation. Returns a document fragment containing nodes parsed from
* the provided data.
Expand Down
15 changes: 15 additions & 0 deletions packages/ckeditor5-engine/src/dataprocessor/xmldataprocessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,21 @@ export default class XmlDataProcessor {
this._domConverter.registerRawContentMatcher( pattern );
}

/**
* If the processor is set to use marked fillers, it will insert nbsp fillers wrapped in spans
* (`<span data-cke-filler="true">&nbsp;</span>`), instead of regular nbsp characters (`&nbsp;`).
*
* This mode allows for more precise handling of block fillers (so they don't leak into editor content) but bloats the editor
* data with additional markup.
*
* This mode may be required by some features and will be turned on by them automatically.
*
* @param {'default'|'marked'} type Whether to use default or marked nbsp block fillers.
*/
useFillerType( type ) {
this._domConverter.blockFillerMode = type == 'marked' ? 'markedNbsp' : 'nbsp';
}

/**
* Converts an XML string to its DOM representation. Returns a document fragment containing nodes parsed from
* the provided data.
Expand Down
62 changes: 37 additions & 25 deletions packages/ckeditor5-engine/src/view/domconverter.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ import ViewSelection from './selection';
import ViewDocumentFragment from './documentfragment';
import ViewTreeWalker from './treewalker';
import Matcher from './matcher';
import { BR_FILLER, getDataWithoutFiller, INLINE_FILLER_LENGTH, isInlineFiller, NBSP_FILLER, startsWithFiller } from './filler';
import {
BR_FILLER, INLINE_FILLER_LENGTH, NBSP_FILLER, MARKED_NBSP_FILLER,
getDataWithoutFiller, isInlineFiller, startsWithFiller
} from './filler';

import global from '@ckeditor/ckeditor5-utils/src/dom/global';
import indexOf from '@ckeditor/ckeditor5-utils/src/dom/indexof';
Expand All @@ -26,8 +29,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 );
const BR_FILLER_REF = BR_FILLER( document ); // eslint-disable-line new-cap
const NBSP_FILLER_REF = NBSP_FILLER( document ); // eslint-disable-line new-cap
const MARKED_NBSP_FILLER_REF = MARKED_NBSP_FILLER( document ); // eslint-disable-line new-cap

/**
* `DomConverter` is a set of tools to do transformations between DOM nodes and view nodes. It also handles
Expand Down Expand Up @@ -60,8 +64,7 @@ export default class DomConverter {
/**
* The mode of a block filler used by the DOM converter.
*
* @readonly
* @member {'br'|'nbsp'} module:engine/view/domconverter~DomConverter#blockFillerMode
* @member {'br'|'nbsp'|'markedNbsp'} module:engine/view/domconverter~DomConverter#blockFillerMode
*/
this.blockFillerMode = options.blockFillerMode || 'br';

Expand All @@ -86,16 +89,6 @@ export default class DomConverter {
*/
this.blockElements = [ 'p', 'div', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'li', 'dd', 'dt', 'figcaption', 'td', 'th' ];

/**
* 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;

/**
* The DOM-to-view mapping.
*
Expand Down Expand Up @@ -297,7 +290,7 @@ export default class DomConverter {

for ( const childView of viewElement.getChildren() ) {
if ( fillerPositionOffset === offset ) {
yield this._blockFiller( domDocument );
yield this._getBlockFiller( domDocument );
}

yield this.viewToDom( childView, domDocument, options );
Expand All @@ -306,7 +299,7 @@ export default class DomConverter {
}

if ( fillerPositionOffset === offset ) {
yield this._blockFiller( domDocument );
yield this._getBlockFiller( domDocument );
}
}

Expand Down Expand Up @@ -413,7 +406,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 ( this.isBlockFiller( domNode, this.blockFillerMode ) ) {
if ( this.isBlockFiller( domNode ) ) {
return null;
}

Expand Down Expand Up @@ -581,7 +574,7 @@ export default class DomConverter {
* @returns {module:engine/view/position~Position} viewPosition View position.
*/
domPositionToView( domParent, domOffset ) {
if ( this.isBlockFiller( domParent, this.blockFillerMode ) ) {
if ( this.isBlockFiller( domParent ) ) {
return this.domPositionToView( domParent.parentNode, indexOf( domParent ) );
}

Expand Down Expand Up @@ -863,13 +856,13 @@ export default class DomConverter {
return domNode.isEqualNode( BR_FILLER_REF );
}

// Special case for <p><br></p> in which case the <br> should be treated as filler even
// when we're in the 'nbsp' mode. See ckeditor5#5564.
// Special case for <p><br></p> in which <br> should be treated as filler even when we are not in the 'br' mode. See ckeditor5#5564.
if ( domNode.tagName === 'BR' && hasBlockParent( domNode, this.blockElements ) && domNode.parentNode.childNodes.length === 1 ) {
return true;
}

return isNbspBlockFiller( domNode, this.blockElements );
// If not in 'br' mode, try recognizing both marked and regular nbsp block fillers.
return domNode.isEqualNode( MARKED_NBSP_FILLER_REF ) || isNbspBlockFiller( domNode, this.blockElements );
}

/**
Expand Down Expand Up @@ -956,6 +949,24 @@ export default class DomConverter {
this._rawContentElementMatcher.add( pattern );
}

/**
* Returns block {@link module:engine/view/filler filler} node based on current {@link #blockFillerMode} setting.
*
* @private
* @params {Document} domDocument
* @returns {Node} filler
*/
_getBlockFiller( domDocument ) {
switch ( this.blockFillerMode ) {
case 'nbsp':
return NBSP_FILLER( domDocument ); // eslint-disable-line new-cap
case 'markedNbsp':
return MARKED_NBSP_FILLER( domDocument ); // eslint-disable-line new-cap
case 'br':
return BR_FILLER( domDocument ); // eslint-disable-line new-cap
}
}

/**
* Checks if the given DOM position is a correct place for selection boundary. See {@link #isDomSelectionCorrect}.
*
Expand Down Expand Up @@ -1320,7 +1331,7 @@ function forEachDomNodeAncestor( node, callback ) {
// @param {Node} domNode DOM node.
// @returns {Boolean}
function isNbspBlockFiller( domNode, blockElements ) {
const isNBSP = isText( domNode ) && domNode.data == '\u00A0';
const isNBSP = domNode.isEqualNode( NBSP_FILLER_REF );

return isNBSP && hasBlockParent( domNode, blockElements ) && domNode.parentNode.childNodes.length === 1;
}
Expand All @@ -1340,8 +1351,9 @@ function hasBlockParent( domNode, blockElements ) {
*
* Possible values:
*
* * `br` - for `<br>` block filler used in editing view,
* * `nbsp` - for `&nbsp;` block fillers used in the data.
* * `br` - for `<br data-cke-filler="true">` block filler used in the editing view,
* * `nbsp` - for `&nbsp;` block fillers used in the data,
* * `markedNbsp` - for nbsp block fillers wrapped in a span: `<span data-cke-filler="true">&nbsp;</span>` used in the data.
*
* @typedef {String} module:engine/view/filler~BlockFillerMode
*/
21 changes: 20 additions & 1 deletion packages/ckeditor5-engine/src/view/filler.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import isText from '@ckeditor/ckeditor5-utils/src/dom/istext';
* it is transparent for the selection, so when the caret is before the `<br>` and user presses right arrow he will be
* moved to the next paragraph, not after the `<br>`. The disadvantage is that it breaks a block, so it can not be used
* in the middle of a line of text. The {@link module:engine/view/filler~BR_FILLER `<br>` filler} can be replaced with any other
* character in the data output, for instance {@link module:engine/view/filler~NBSP_FILLER non-breaking space}.
* character in the data output, for instance {@link module:engine/view/filler~NBSP_FILLER non-breaking space} or
* {@link module:engine/view/filler~MARKED_NBSP_FILLER marked non-breaking space}.
*
* * Inline filler is a filler which does not break a line of text, so it can be used inside the text, for instance in the empty
* `<b>` surrendered by text: `foo<b></b>bar`, if we want to put the caret there. CKEditor uses a sequence of the zero-width
Expand All @@ -38,16 +39,34 @@ 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~MARKED_NBSP_FILLER
* @see module:engine/view/filler~BR_FILLER
* @function
*/
export const NBSP_FILLER = domDocument => domDocument.createTextNode( '\u00A0' );

/**
* Marked non-breaking space filler creator. This is a function which creates `<span data-cke-filler="true">&nbsp;</span>` element.
* It defines how the filler is created.
*
* @see module:engine/view/filler~NBSP_FILLER
* @see module:engine/view/filler~BR_FILLER
* @function
*/
export const MARKED_NBSP_FILLER = domDocument => {
const span = domDocument.createElement( 'span' );
span.dataset.ckeFiller = true;
span.innerHTML = '\u00A0';

return span;
};

/**
* `<br>` filler creator. This is a function which creates `<br data-cke-filler="true">` element.
* It defines how the filler is created.
*
* @see module:engine/view/filler~NBSP_FILLER
* @see module:engine/view/filler~MARKED_NBSP_FILLER
* @function
*/
export const BR_FILLER = domDocument => {
Expand Down
16 changes: 16 additions & 0 deletions packages/ckeditor5-engine/tests/dataprocessor/htmldataprocessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,20 @@ describe( 'HtmlDataProcessor', () => {
expect( fragment.getChild( 1 ).getCustomProperty( '$rawContent' ) ).to.equal( '<!-- 123 --> abc <!-- 456 -->' );
} );
} );

describe( 'useFillerType()', () => {
it( 'should turn on and off using marked block fillers', () => {
const fragment = parse( '<container:p></container:p>' );

expect( dataProcessor.toData( fragment ) ).to.equal( '<p>&nbsp;</p>' );

dataProcessor.useFillerType( 'marked' );

expect( dataProcessor.toData( fragment ) ).to.equal( '<p><span data-cke-filler="true">&nbsp;</span></p>' );

dataProcessor.useFillerType( 'default' );

expect( dataProcessor.toData( fragment ) ).to.equal( '<p>&nbsp;</p>' );
} );
} );
} );
16 changes: 16 additions & 0 deletions packages/ckeditor5-engine/tests/dataprocessor/xmldataprocessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,20 @@ describe( 'XmlDataProcessor', () => {
expect( fragment.getChild( 1 ).getCustomProperty( '$rawContent' ) ).to.equal( '<!-- 123 --> abc <!-- 456 -->' );
} );
} );

describe( 'useFillerType()', () => {
it( 'should turn on and off using marked block fillers', () => {
const fragment = parse( '<container:p></container:p>' );

expect( dataProcessor.toData( fragment ) ).to.equal( '<p>&nbsp;</p>' );

dataProcessor.useFillerType( 'marked' );

expect( dataProcessor.toData( fragment ) ).to.equal( '<p><span data-cke-filler="true">&nbsp;</span></p>' );

dataProcessor.useFillerType( 'default' );

expect( dataProcessor.toData( fragment ) ).to.equal( '<p>&nbsp;</p>' );
} );
} );
} );
Loading

0 comments on commit 5217b30

Please sign in to comment.