Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Changed how   are generated on view->DOM rendering #1748

Merged
merged 3 commits into from
Jun 17, 2019
Merged

Changed how   are generated on view->DOM rendering #1748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a249dc
Other: Changed how   are generated on view->DOM rendering.
scofalik Jun 14, 2019
020c743
Docs: Corrected some documentation.
scofalik Jun 17, 2019
78511eb
Internal: Linked changes to a Github issue with a link to the issue i…
scofalik Jun 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 28 additions & 19 deletions src/view/domconverter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,8 @@ export default class DomConverter {
*
* * a space at the beginning is changed to ` ` if this is the first text node in its container
* element or if a previous text node ends with a space character,
* * space at the end of the text node is changed to ` ` if this is the last text node in its container,
* * space at the end of the text node is changed to ` ` if there are two spaces at the end of a node or if next node
* starts with a space or if it is the last text node in its container,
* * remaining spaces are replaced to a chain of spaces and ` ` (e.g. `'x x'` becomes `'x   x'`).
*
* Content of {@link #preElements} is not processed.
Expand Down Expand Up @@ -918,15 +919,24 @@ export default class DomConverter {
}
}

// 2. Replace the last space with a nbsp if this is the last text node (container element boundary).
// 2. Replace the last space with nbsp if there are two spaces at the end or if the next node starts with space or there is no
// next node (container element boundary).
//
// Keep in mind that Firefox prefers $nbsp; before tag, not inside it:
//
// Foo <span>&nbsp;bar</span> <-- bad.
// Foo&nbsp;<span> bar</span> <-- good.
//
// More here: https://github.com/ckeditor/ckeditor5-engine/issues/1747.
if ( data.charAt( data.length - 1 ) == ' ' ) {
const nextNode = this._getTouchingViewTextNode( node, true );

if ( !nextNode ) {
if ( data.charAt( data.length - 2 ) == ' ' || !nextNode || nextNode.data.charAt( 0 ) == ' ' ) {
data = data.substr( 0, data.length - 1 ) + '\u00A0';
}
}

// 3. Create space+nbsp pairs.
return data.replace( / {2}/g, ' \u00A0' );
}

Expand Down Expand Up @@ -955,7 +965,9 @@ export default class DomConverter {
* * multiple whitespaces are replaced to a single space,
* * space at the beginning of a text node is removed if it is the first text node in its container
* element or if the previous text node ends with a space character,
* * space at the end of the text node is removed, if it is the last text node in its container.
* * space at the end of the text node is removed if there are two spaces at the end of a node or if next node
* starts with a space or if it is the last text node in its container
* * nbsps are converted to spaces.
*
* @param {Node} node DOM text node to process.
* @returns {String} Processed data.
Expand Down Expand Up @@ -998,28 +1010,25 @@ export default class DomConverter {
data = getDataWithoutFiller( new Text( data ) );

// At this point we should have removed all whitespaces from DOM text data.

// Now we have to change &nbsp; chars, that were in DOM text data because of rendering reasons, to spaces.
// First, change all ` \u00A0` pairs (space + &nbsp;) to two spaces. DOM converter changes two spaces from model/view as
// ` \u00A0` to ensure proper rendering. Since here we convert back, we recognize those pairs and change them
// to ` ` which is what we expect to have in model/view.
//
// Now, We will reverse the process that happens in `_processDataFromViewText`.
//
// We have to change &nbsp; chars, that were in DOM text data because of rendering reasons, to spaces.
// First, change all ` \u00A0` pairs (space + &nbsp;) to two spaces. DOM converter changes two spaces from model/view to
// ` \u00A0` to ensure proper rendering. Since here we convert back, we recognize those pairs and change them back to ` `.
data = data.replace( / \u00A0/g, ' ' );

// Then, let's change the last nbsp to a space.
if ( /( |\u00A0)\u00A0$/.test( data ) || !nextNode || ( nextNode.data && nextNode.data.charAt( 0 ) == ' ' ) ) {
data = data.replace( /\u00A0$/, ' ' );
}

// Then, change &nbsp; character that is at the beginning of the text node to space character.
// As above, that &nbsp; was created for rendering reasons but it's real meaning is just a space character.
// We do that replacement only if this is the first node or the previous node ends on whitespace character.
if ( shouldLeftTrim ) {
data = data.replace( /^\u00A0/, ' ' );
}

// Since input text data could be: `x_ _`, we would not replace the first &nbsp; after `x` character.
// We have to fix it. Since we already change all ` &nbsp;`, we will have something like this at the end of text data:
// `x_ _ _` -> `x_ `. Find &nbsp; at the end of string (can be followed only by spaces).
// We do that replacement only if this is the last node or the next node starts with &nbsp; or is a <br>.
if ( isText( nextNode ) ? nextNode.data.charAt( 0 ) == '\u00A0' : true ) {
data = data.replace( /\u00A0( *)$/, ' $1' );
}

// At this point, all whitespaces should be removed and all &nbsp; created for rendering reasons should be
// changed to normal space. All left &nbsp; are &nbsp; inserted intentionally.
return data;
Expand Down Expand Up @@ -1048,7 +1057,7 @@ export default class DomConverter {
* be trimmed from the right side.
*
* @param {Node} node
* @param {Node} prevNode
* @param {Node} nextNode
*/
_checkShouldRightTrimDomText( node, nextNode ) {
if ( nextNode ) {
Expand Down
33 changes: 14 additions & 19 deletions tests/view/domconverter/dom-to-view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ describe( 'DomConverter', () => {
// At the end.
test( 'x_', 'x ' );
test( 'x _', 'x ' );
test( 'x_ _', 'x ' );
test( 'x __', 'x ' );
test( 'x _ _', 'x ' );

// In the middle.
Expand All @@ -464,7 +464,7 @@ describe( 'DomConverter', () => {
test( '_x_', ' x ' );
test( '_ x _x _', ' x x ' );
test( '_ _x x _', ' x x ' );
test( '_ _x x_ _', ' x x ' );
test( '_ _x x __', ' x x ' );
test( '_ _x _ _x_', ' x x ' );
test( '_', ' ' );

Expand All @@ -477,10 +477,6 @@ describe( 'DomConverter', () => {
test( 'x_', 'x ' );
test( 'x__', 'x_ ' );
test( 'x___', 'x__ ' );
// This is an edge case, but it's impossible to write elegant and compact algorithm that is also
// 100% correct. We might assume that expected result is `x _` but it will be converted to `x `
// by the algorithm. This is acceptable, though.
test( 'x __', 'x ' );

test( 'x_x', 'x_x' );
test( 'x___x', 'x___x' );
Expand All @@ -496,26 +492,25 @@ describe( 'DomConverter', () => {
test( [ 'x', 'y' ], 'xy' );
test( [ 'x ', 'y' ], 'x y' );
test( [ 'x _', 'y' ], 'x y' );
test( [ 'x _ ', 'y' ], 'x y' );
test( [ 'x __', 'y' ], 'x y' );
test( [ 'x _ _', 'y' ], 'x y' );

test( [ 'x', ' y' ], 'x y' );
test( [ 'x ', '_y' ], 'x y' );
test( [ 'x_ ', '_y' ], 'x y' );
test( [ 'x _ ', '_y' ], 'x y' );
test( [ 'x_ _ ', '_y' ], 'x y' );
test( [ 'x_', ' y' ], 'x y' );
test( [ 'x _', ' y' ], 'x y' );
test( [ 'x __', ' y' ], 'x y' );
test( [ 'x _ _', ' y' ], 'x y' );

test( [ 'x', ' _y' ], 'x y' );
test( [ 'x ', '_ y' ], 'x y' );
test( [ 'x_ ', '_ y' ], 'x y' );
test( [ 'x _ ', '_ y' ], 'x y' );
test( [ 'x_ _ ', '_ y' ], 'x y' );
test( [ 'x_', ' _y' ], 'x y' );
test( [ 'x _', ' _y' ], 'x y' );
test( [ 'x __', ' _y' ], 'x y' );
test( [ 'x _ _', ' _y' ], 'x y' );

// Some tests with hard &nbsp;
test( [ 'x', '_y' ], 'x_y' );
test( [ 'x_', 'y' ], 'x_y' );
test( [ 'x_', ' y' ], 'x_ y' );
test( [ 'x__', ' y' ], 'x__ y' );
test( [ 'x__', ' y' ], 'x_ y' );
test( [ 'x_ _', ' y' ], 'x_ y' );

it( 'not in preformatted blocks', () => {
Expand Down Expand Up @@ -605,9 +600,9 @@ describe( 'DomConverter', () => {
expect( viewP.getChild( 0 ).data ).to.equal( 'foo ' );
} );

it( 'not before a <br> (nbsp+space)', () => {
it( 'not before a <br> (space+nbsp)', () => {
const domP = createElement( document, 'p', {}, [
document.createTextNode( 'foo\u00a0 ' ),
document.createTextNode( 'foo \u00a0' ),
createElement( document, 'br' )
] );

Expand Down
69 changes: 35 additions & 34 deletions tests/view/domconverter/view-to-dom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ describe( 'DomConverter', () => {

const domDiv = converter.viewToDom( viewDiv, document );

expect( domDiv.innerHTML ).to.equal( 'x &nbsp;x &nbsp; x x <b>&nbsp;x </b><i><b><u>&nbsp;x</u></b></i>' );
expect( domDiv.innerHTML ).to.equal( 'x &nbsp;x &nbsp; x x&nbsp;<b> x&nbsp;</b><i><b><u> x</u></b></i>' );
} );

it( 'all together', () => {
Expand All @@ -232,7 +232,7 @@ describe( 'DomConverter', () => {
const domDiv = converter.viewToDom( viewDiv, document );

expect( domDiv.innerHTML ).to.equal(
'<p>&nbsp;x &nbsp;x &nbsp; x x <b>&nbsp;x </b><i><b><u>&nbsp;x&nbsp;</u></b></i></p><p>&nbsp; x &nbsp;</p>'
'<p>&nbsp;x &nbsp;x &nbsp; x x&nbsp;<b> x&nbsp;</b><i><b><u> x&nbsp;</u></b></i></p><p>&nbsp; x &nbsp;</p>'
);
} );

Expand Down Expand Up @@ -321,86 +321,87 @@ describe( 'DomConverter', () => {
test( 'x _ x', 'x _ x' );
test( 'x _ x', 'x __ _x' );

// Two text nodes.
test( [ 'x', 'y' ], 'xy' );
test( [ 'x ', 'y' ], 'x y' );
test( [ 'x ', 'y' ], 'x _y' );
test( [ 'x ', 'y' ], 'x _ y' );
test( [ 'x ', 'y' ], 'x __y' );
test( [ 'x ', 'y' ], 'x _ _y' );

test( [ 'x', ' y' ], 'x y' );
test( [ 'x ', ' y' ], 'x _y' );
test( [ 'x ', ' y' ], 'x_ y' );
test( [ 'x ', ' y' ], 'x _ y' );
test( [ 'x ', ' y' ], 'x _ _y' );
test( [ 'x ', ' y' ], 'x __ y' );
test( [ 'x ', ' y' ], 'x _ _ y' );

test( [ 'x', '_y' ], 'x_y' );
test( [ 'x ', '_y' ], 'x _y' );
test( [ 'x ', '_y' ], 'x __y' );
test( [ 'x ', '_y' ], 'x _ _y' );

// Two text nodes.
test( [ 'x ', '_y' ], 'x ___y' );
test( [ 'x ', '_y' ], 'x _ __y' );

test( [ 'x', ' y' ], 'x _y' );
test( [ 'x ', ' y' ], 'x _ y' );
test( [ 'x ', ' y' ], 'x_ _y' );
test( [ 'x ', ' y' ], 'x _ _y' );
test( [ 'x ', ' y' ], 'x _ _ y' );
test( [ 'x ', ' y' ], 'x __ _y' );
test( [ 'x ', ' y' ], 'x _ _ _y' );

test( [ 'x', ' y' ], 'x _ y' );
test( [ 'x ', ' y' ], 'x _ _y' );
test( [ 'x ', ' y' ], 'x_ _ y' );
test( [ 'x ', ' y' ], 'x _ _ y' );
test( [ 'x ', ' y' ], 'x _ _ _y' );
test( [ 'x ', ' y' ], 'x __ _ y' );
test( [ 'x ', ' y' ], 'x _ _ _ y' );

// "Non-empty" + "empty" text nodes.
test( [ 'x', ' ' ], 'x_' );
test( [ 'x', ' ' ], 'x _' );
test( [ 'x', ' ' ], 'x __' );
test( [ 'x ', ' ' ], 'x _' );
test( [ 'x ', ' ' ], 'x __' );
test( [ 'x ', ' ' ], 'x _ _' );
test( [ 'x ', ' ' ], 'x__' );
test( [ 'x ', ' ' ], 'x_ _' );
test( [ 'x ', ' ' ], 'x_ __' );
test( [ 'x ', ' ' ], 'x __' );
test( [ 'x ', ' ' ], 'x _ _' );
test( [ 'x ', ' ' ], 'x _ __' );
test( [ 'x ', ' ' ], 'x _ _' );
test( [ 'x ', ' ' ], 'x _ __' );
test( [ 'x ', ' ' ], 'x _ _ _' );
test( [ 'x ', ' ' ], 'x ___' );
test( [ 'x ', ' ' ], 'x __ _' );
test( [ 'x ', ' ' ], 'x __ __' );

test( [ ' ', 'x' ], '_x' );
test( [ ' ', 'x' ], '_ x' );
test( [ ' ', 'x' ], '_ _x' );
test( [ ' ', ' x' ], '_ x' );
test( [ ' ', ' x' ], '_ _x' );
test( [ ' ', ' x' ], '__ x' );
test( [ ' ', ' x' ], '_ _ x' );
test( [ ' ', ' x' ], '_ _x' );
test( [ ' ', ' x' ], '_ _ x' );
test( [ ' ', ' x' ], '__ _x' );
test( [ ' ', ' x' ], '_ _ _x' );
test( [ ' ', ' x' ], '_ _ x' );
test( [ ' ', ' x' ], '_ _ _x' );
test( [ ' ', ' x' ], '__ _ x' );
test( [ ' ', ' x' ], '_ _ _ x' );

// "Non-empty" + "empty" text nodes.
test( [ 'x', ' ', 'x' ], 'x x' );
test( [ 'x', ' ', ' x' ], 'x _x' );
test( [ 'x', ' ', ' x' ], 'x_ x' );
test( [ 'x', ' ', ' x' ], 'x _ x' );
test( [ 'x', ' ', ' x' ], 'x _ _ x' );
test( [ 'x ', ' ', ' x' ], 'x _ x' );
test( [ 'x ', ' ', ' x' ], 'x _ _x' );
test( [ 'x ', ' ', ' x' ], 'x _ _ _x' );
test( [ 'x ', ' ', ' x' ], 'x _ _x' );
test( [ 'x', ' ', ' x' ], 'x __ _x' );
test( [ 'x ', ' ', ' x' ], 'x__ x' );
test( [ 'x ', ' ', ' x' ], 'x_ _ x' );
test( [ 'x ', ' ', ' x' ], 'x_ __ _x' );
test( [ 'x ', ' ', ' x' ], 'x __ x' );
test( [ 'x ', ' ', ' x' ], 'x _ _ x' );
test( [ 'x ', ' ', ' x' ], 'x _ _ _ x' );
test( [ 'x ', ' ', ' x' ], 'x _ _ x' );
test( [ 'x ', ' ', ' x' ], 'x _ _ _x' );
test( [ 'x ', ' ', ' x' ], 'x _ _ _ _x' );
test( [ 'x ', ' ', ' x' ], 'x _ __ _x' );
test( [ 'x ', ' ', ' x' ], 'x ___ x' );
test( [ 'x ', ' ', ' x' ], 'x __ _ x' );
test( [ 'x ', ' ', ' x' ], 'x __ __ _x' );

// "Empty" + "empty" text nodes.
test( [ ' ', ' ' ], '__' );
test( [ ' ', ' ' ], '_ _' );
test( [ ' ', ' ' ], '___' );
test( [ ' ', ' ' ], '_ __' );
test( [ ' ', ' ' ], '_ _' );
test( [ ' ', ' ' ], '_ __' );
test( [ ' ', ' ' ], '_ __' );
test( [ ' ', ' ' ], '_ _ _' );
test( [ ' ', ' ' ], '__ _' );
test( [ ' ', ' ' ], '__ __' );
test( [ ' ', ' ' ], '_ _ _' );
test( [ ' ', ' ' ], '_ _ __' );

Expand Down
19 changes: 5 additions & 14 deletions tests/view/domconverter/whitespace-handling-integration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,20 +165,20 @@ describe( 'DomConverter – whitespace handling – integration', () => {
return editor.destroy();
} );

it( 'single spaces around <br>', () => {
it( 'single spaces around <br> #1', () => {
editor.setData( '<p>foo&nbsp;<br>&nbsp;bar</p>' );

expect( getData( editor.model, { withoutSelection: true } ) )
.to.equal( '<paragraph>foo <softBreak></softBreak> bar</paragraph>' );
.to.equal( '<paragraph>foo\u00A0<softBreak></softBreak> bar</paragraph>' );

expect( editor.getData() ).to.equal( '<p>foo&nbsp;<br>&nbsp;bar</p>' );
} );

it( 'single spaces around <br> (normalization)', () => {
it( 'single spaces around <br> #2', () => {
editor.setData( '<p>foo&nbsp;<br> bar</p>' );

expect( getData( editor.model, { withoutSelection: true } ) )
.to.equal( '<paragraph>foo <softBreak></softBreak>bar</paragraph>' );
.to.equal( '<paragraph>foo\u00A0<softBreak></softBreak>bar</paragraph>' );

expect( editor.getData() ).to.equal( '<p>foo&nbsp;<br>bar</p>' );
} );
Expand All @@ -192,16 +192,7 @@ describe( 'DomConverter – whitespace handling – integration', () => {
expect( editor.getData() ).to.equal( '<p>foo &nbsp;<br>bar</p>' );
} );

it( 'two spaces before a <br> (normalization)', () => {
editor.setData( '<p>foo&nbsp; <br>bar</p>' );

expect( getData( editor.model, { withoutSelection: true } ) )
.to.equal( '<paragraph>foo <softBreak></softBreak>bar</paragraph>' );

expect( editor.getData() ).to.equal( '<p>foo &nbsp;<br>bar</p>' );
} );

it( 'two spaces before a <br> (normalization to a model nbsp)', () => {
it( 'two nbsps before a <br>', () => {
editor.setData( '<p>foo&nbsp;&nbsp;<br>bar</p>' );

expect( getData( editor.model, { withoutSelection: true } ) )
Expand Down
Loading