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

T/404: Better handling of   block filler. #1779

Merged
merged 32 commits into from
Sep 18, 2019
Merged

T/404: Better handling of   block filler. #1779

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
c15a2b7
Better detection of block filler in data.
jodator Aug 6, 2019
63e81a5
Introduce isNegligibleBlockFiller helper function.
jodator Aug 7, 2019
fd2ae41
Update docs of isNegligibleBlockFiller() helper function.
jodator Aug 7, 2019
fd76bf2
Change the way how isBlockFiller check nbps fillers.
jodator Aug 8, 2019
be11659
The isBlockFiller() should allow single   inside inline elements.
jodator Aug 8, 2019
f9f89ae
Use more elements in `isInline()` check.
jodator Aug 8, 2019
0ac3b6b
Fix code style issues.
jodator Aug 8, 2019
2e9d550
Merge branch 'master' into t/404
jodator Aug 12, 2019
92da88e
wip
jodator Aug 12, 2019
2daf569
Revert to previous tests of nbsp between blocks.
jodator Aug 13, 2019
86a8970
Handle differently br and nbsp fillers.
jodator Aug 13, 2019
600dacb
Remove .only from tests.
jodator Aug 13, 2019
6053e18
Update "nbsp" between blocks tests cases.
jodator Aug 13, 2019
86eb560
Merge branch 'master' into t/404
jodator Aug 13, 2019
9ba5731
Simplify nbsp between blocks tests.
jodator Aug 14, 2019
37aec88
Merge branch 'master' into t/404
jodator Aug 26, 2019
c782dc9
Introduce blockFillerMode in DomConverter.
jodator Sep 3, 2019
d55bbde
Rewrite the isBlockFiller method with logic from DomConvrter#domToVie…
jodator Sep 3, 2019
e5c7361
Simplify isBlockFiller internals.
jodator Sep 3, 2019
1012605
Expand documentation for new block filler methods.
jodator Sep 4, 2019
dbecbf1
Remove unused import
jodator Sep 4, 2019
529a95d
Merge branch 'master' into t/404
jodator Sep 4, 2019
c62a9fd
Rename isNbspFillerFiller() to isNbspBlockFiller().
jodator Sep 4, 2019
75bad14
Filler docs fixes.
jodator Sep 4, 2019
326372c
Merge branch 'master' into t/404
jodator Sep 4, 2019
a9f4b7a
Docs fix.
jodator Sep 5, 2019
0a82f09
Merge branch 'master' into t/404
Reinmar Sep 17, 2019
d9dbda2
Update src/view/domconverter.js
jodator Sep 17, 2019
d370947
Update src/view/domconverter.js
jodator Sep 17, 2019
23b1b79
Move block filler handling to DomConverter.
jodator Sep 17, 2019
fc769ff
Merge branch 'master' into t/404
jodator Sep 17, 2019
fa8ccb6
Cleanup.
Reinmar Sep 18, 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
10 changes: 9 additions & 1 deletion src/view/domconverter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,15 @@ export default class DomConverter {
*/
domToView( domNode, options = {} ) {
if ( isBlockFiller( domNode, this.blockFiller ) ) {
return null;
const isSingle = domNode.parentNode && domNode.parentNode.childNodes.length <= 1;
jodator marked this conversation as resolved.
Show resolved Hide resolved

if ( isText( domNode ) ) {
if ( isSingle && _hasDomParentOfType( domNode, this.blockElements ) ) {
return null;
}
} else {
return null;
}
}

// When node is inside UIElement return that UIElement as it's view representation.
Expand Down
53 changes: 26 additions & 27 deletions tests/dataprocessor/htmldataprocessor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
// '<meta charset=\'utf-8\'>' +
// '<span>This is the<span>\u00a0</span></span>' +
// '<a href="url">third developer preview</a>' +
// '<span><span>\u00a0</span>of<span>\u00a0</span></span>' +
// '<strong>CKEditor\u00a05</strong>' +
// '<span>.</span>'
// );

// expect( stringify( fragment ) ).to.equal(
// '<span>This is the<span>\u00a0</span></span>' +
// '<a href="url">third developer preview</a>' +
// '<span><span>\u00a0</span>of<span>\u00a0</span></span>' +
// '<strong>CKEditor\u00a05</strong>' +
// '<span>.</span>'
// );

// // 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(
'<meta charset=\'utf-8\'>' +
'<span>This is the<span>\u00a0</span></span>' +
'<a href="url">third developer preview</a>' +
'<span><span>\u00a0</span>of<span>\u00a0</span></span>' +
'<strong>CKEditor\u00a05</strong>' +
'<span>.</span>'
);

expect( stringify( fragment ) ).to.equal(
'<span>This is the<span>\u00a0</span></span>' +
'<a href="url">third developer preview</a>' +
'<span><span>\u00a0</span>of<span>\u00a0</span></span>' +
'<strong>CKEditor\u00a05</strong>' +
'<span>.</span>'
);

// 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()', () => {
Expand Down
79 changes: 68 additions & 11 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 @@ -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' } );
} );
} );

Expand All @@ -48,14 +51,13 @@ describe( 'DomConverter – whitespace handling – integration', () => {
expect( editor.getData() ).to.equal( '<p>foo</p>' );
} );

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

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

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

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

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

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

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

it( 'new line between blocks is ignored', () => {
Expand All @@ -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( '<p>foo</p>&nbsp;<p>bar</p>' );

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

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

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( '<block>foo</block>&nbsp;<p>bar</p>' );

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

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

it( 'new lines inside blocks are ignored', () => {
Expand Down Expand Up @@ -141,6 +162,15 @@ describe( 'DomConverter – whitespace handling – integration', () => {
expect( editor.getData() ).to.equal( '<p>&nbsp;foo&nbsp;</p>' );
} );

it( 'single nbsp inside blocks are ignored', () => {
editor.setData( '<p>&nbsp;</p>' );

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

expect( editor.getData() ).to.equal( '' ); // trimmed
} );

it( 'all whitespaces together are ignored', () => {
editor.setData( '\n<p>foo\n\r\n \t</p>\n<p> bar</p>' );

Expand All @@ -149,6 +179,33 @@ describe( 'DomConverter – whitespace handling – integration', () => {

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

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 } ) )
.to.equal( '<paragraph><$text bold="true">foo</$text>\u00A0<$text bold="true">bar</$text></paragraph>' );

expect( editor.getData() ).to.equal( '<p><b>foo</b>&nbsp;<b>bar</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> <$text bold="true">bar</$text></paragraph>' );

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>' );
} );
} );

// https://github.com/ckeditor/ckeditor5/issues/1024
Expand Down