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

[Chrome] Handle pasted whitespaces #25

Merged
merged 2 commits into from
Jul 7, 2017
Merged

[Chrome] Handle pasted whitespaces #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a667e1b
Remove also unnamed spans to avoid polluting the content with   …
Reinmar Jul 3, 2017
f29a1b8
Merge branch 'master' into t/24
Reinmar Jul 4, 2017
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
2 changes: 1 addition & 1 deletion src/utils/normalizeclipboarddata.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/
export default function normalizeClipboardData( data ) {
return data
.replace( /<span class="Apple-converted-space">(\s+)<\/span>/g, ( fullMatch, spaces ) => {
.replace( /<span(?: class="Apple-converted-space"|)>(\s+)<\/span>/g, ( fullMatch, spaces ) => {
// Handle the most popular and problematic case when even a single space becomes an nbsp;.
// Decode those to normal spaces. Read more in https://github.com/ckeditor/ckeditor5-clipboard/issues/2.
if ( spaces.length == 1 ) {
Expand Down
20 changes: 20 additions & 0 deletions tests/manual/pasting.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,23 @@ <h3>Notes</h3>

<p>It has <em>bugs</em> that we are aware of – and that we will be working on in the next few iterations of the project. Stay tuned for some updates soon!</p>
</div>

<h2 style="margin-top: 100px">Some rich content to copy</h2>

<p>Copy also this content to check how pasting from outside of the editor works. Feel free to also use content from other websites.</p>

<p>This is the <a href="http://ckeditor.com/blog/Third-Developer-Preview-of-CKEditor-5-Available">third developer preview</a> of <strong>CKEditor&nbsp;5</strong>.</p>

<p>After 2 years of work, building the next generation editor from scratch and closing over 670 tickets, we created a highly <strong>extensible and flexible architecture</strong> which consists of an <strong>amazing editing framework</strong> and <strong>editing solutions</strong> that will be built on top of it.</p>

<h3>Notes</h3>

<p><a href="https://ckeditor5.github.io">CKEditor&nbsp;5</a> is <i>under heavy development</i> and this demo is not production-ready software. For example:</p>

<ul>
<li><strong>only Chrome, Opera and Safari are supported</strong>,</li>
<li>Firefox requires enabling the <a href="https://developer.mozilla.org/en-US/docs/Web/API/Document/onselectionchange">&ldquo;dom.select_events.enabled&rdquo;</a> option,</li>
<li><a href="https://github.com/ckeditor/ckeditor5/issues/342">support for pasting</a> is under development.</li>
</ul>

<p>It has <em>bugs</em> that we are aware of – and that we will be working on in the next few iterations of the project. Stay tuned for some updates soon!</p>
83 changes: 83 additions & 0 deletions tests/pasting-integration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,89 @@ describe( 'Pasting – integration', () => {
} );
} );
} );

describe( 'white spaces', () => {
// See https://github.com/ckeditor/ckeditor5-clipboard/issues/2#issuecomment-310417731.
it( 'keeps spaces around inline styles (Chrome)', () => {
return ClassicTestEditor
.create( element, { plugins: [ Clipboard, Paragraph, Bold, Italic, Link ] } )
.then( editor => {
setData( editor.document, '<paragraph>x[]y</paragraph>' );

pasteHtml( editor,
'<meta charset=\'utf-8\'>' +
'<span style="color: rgb(0, 0, 0); font-family: Times;">This is the<span>\u00a0</span></span>' +
'<a href="url" style="font-family: Times; font-size: medium;">third developer preview</a>' +
'<span style="color: rgb(0, 0, 0); font-family: Times;"><span>\u00a0</span>of<span>\u00a0</span></span>' +
'<strong style="color: rgb(0, 0, 0); font-family: Times;">CKEditor\u00a05</strong>' +
'<span style="color: rgb(0, 0, 0); font-family: Times;">.</span>'
);

expect( getData( editor.document ) ).to.equal(
'<paragraph>' +
'xThis is the ' +
'<$text linkHref="url">third developer preview</$text> of <$text bold="true">CKEditor\u00a05</$text>' +
'.[]y' +
'</paragraph>'
);

return editor.destroy();
} );
} );

// See https://github.com/ckeditor/ckeditor5-clipboard/issues/2#issuecomment-310417731.
it( 'keeps spaces around inline styles (Safari)', () => {
return ClassicTestEditor
.create( element, { plugins: [ Clipboard, Paragraph, Bold, Italic, Link ] } )
.then( editor => {
setData( editor.document, '<paragraph>x[]y</paragraph>' );

/* eslint-disable max-len */
pasteHtml( editor,
'<span style="color: rgb(0, 0, 0); font-family: -webkit-standard;">This is the<span class="Apple-converted-space">\u00a0</span></span>' +
'<a href="url" style="font-family: -webkit-standard; font-style: normal;">third developer preview</a>' +
'<span style="color: rgb(0, 0, 0); font-family: -webkit-standard;"><span class="Apple-converted-space">\u00a0</span>of<span class="Apple-converted-space">\u00a0</span></span>' +
'<strong style="color: rgb(0, 0, 0); font-family: -webkit-standard;">CKEditor\u00a05</strong>' +
'<span style="color: rgb(0, 0, 0); font-family: -webkit-standard;">.</span>'
);
/* eslint-enable max-len */

expect( getData( editor.document ) ).to.equal(
'<paragraph>' +
'xThis is the ' +
'<$text linkHref="url">third developer preview</$text> of <$text bold="true">CKEditor\u00a05</$text>' +
'.[]y' +
'</paragraph>'
);

return editor.destroy();
} );
} );

it( 'keeps spaces around inline styles (Firefox)', () => {
return ClassicTestEditor
.create( element, { plugins: [ Clipboard, Paragraph, Bold, Italic, Link ] } )
.then( editor => {
setData( editor.document, '<paragraph>x[]y</paragraph>' );

// Note, when copying the HTML from Firefox's console you'll see only normal spaces,
// but when you check it later in the model it's still an nbsp.
pasteHtml( editor,
'This is the <a href="url">third developer preview</a> of <strong>CKEditor\u00a05</strong>.'
);

expect( getData( editor.document ) ).to.equal(
'<paragraph>' +
'xThis is the ' +
'<$text linkHref="url">third developer preview</$text> of <$text bold="true">CKEditor\u00a05</$text>' +
'.[]y' +
'</paragraph>'
);

return editor.destroy();
} );
} );
} );
} );

function pasteHtml( editor, html ) {
Expand Down
42 changes: 41 additions & 1 deletion tests/utils/normalizeclipboarddata.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import normalizeClipboardData from '../../src/utils/normalizeclipboarddata';

describe( 'normalizeClipboardData', () => {
describe( 'normalizeClipboardData()', () => {
it( 'should strip all span.Apple-converted-space', () => {
expect( normalizeClipboardData(
'<span class="Apple-converted-space"> \t\n</span>x<span class="Apple-converted-space">\u00a0\u00a0</span>'
Expand All @@ -17,4 +17,44 @@ describe( 'normalizeClipboardData', () => {
normalizeClipboardData( '<span class="Apple-converted-space"> </span>x<span class="Apple-converted-space">\u00a0</span>' )
).to.equal( ' x ' );
} );

it( 'should strip all spans with no attributes', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering whether this is cleanup should be really done.

On one hand the data is correct and, hypothetically, there could be a plugin that creates empty spans and want them retained when copied from editor to editor.

On the other hand, in 99.9% of cases empty span is just a garbage because spans are used mostly to hold formatting. Also a feature that creates just an empty span is probably a bad practice and something that may blow up :).

So I think this is fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. The entire #2 is about this. We may lose some data. But, at least for now, it's more important to keep the content clean from non-breaking spaces. Then, we can somehow support copy-pasting them within the editor (they'd need to be marked in a special way when stringified to HTML). And we'll never be able to handle copy-pasting them from outside of the editor because, by converting many normal spaces to non-breaking ones, Chrome loses that data.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And as for features which want to produce empty spans with no classes/attributes (cause that's what cleaned here), they will simply be incorrect. Just like you mentioned – they'd be treated as a garbage.

expect( normalizeClipboardData(
'<span> \t\n</span>x<span>\u00a0\u00a0</span>'
) ).to.equal( ' \t\nx\u00a0\u00a0' );
} );

it( 'should replace spans with no attributes with a normal space', () => {
expect(
normalizeClipboardData( '<span> </span>x<span>\u00a0</span>' )
).to.equal( ' x ' );
} );

it( 'should not strip spans with no attributes if they contain anything but spaces', () => {
expect(
normalizeClipboardData( '<span> a</span>x<span>b\u00a0</span>x<span>c</span>' )
).to.equal( '<span> a</span>x<span>b\u00a0</span>x<span>c</span>' );
} );

it( 'should not replace spans of length 1+ with normal space', () => {
expect(
normalizeClipboardData( '<span> </span>x<span>\u00a0 </span>x<span>\u00a0\u00a0</span>x<span> \u00a0</span>' )
).to.equal( ' x\u00a0 x\u00a0\u00a0x \u00a0' );
} );

it( 'should not strip spans with any attribute (except span.Apple-converted-space)', () => {
const input =
'<span style="color: red"> </span>x' +
'<span foo="1">\u00a0</span>x' +
'<span foo> </span>x' +
'<span class="bar">\u00a0</span>';

expect( normalizeClipboardData( input ) ).to.equal( input );
} );

it( 'should not be greedy', () => {
expect(
normalizeClipboardData( '<span class="Apple-converted-space"> </span><span foo> </span><span>a</span>' )
).to.equal( ' <span foo> </span><span>a</span>' );
} );
} );
2 changes: 1 addition & 1 deletion tests/utils/plaintexttohtml.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import plainTextToHtml from '../../src/utils/plaintexttohtml';

describe( 'plainTextToHtml', () => {
describe( 'plainTextToHtml()', () => {
it( 'encodes < and >', () => {
expect( plainTextToHtml( 'x y <z>' ) ).to.equal( 'x y &lt;z&gt;' );
} );
Expand Down
2 changes: 1 addition & 1 deletion tests/utils/viewtoplaintext.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import viewToPlainText from '../../src/utils/viewtoplaintext';

import { parse as parseView } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';

describe( 'viewToPlainText', () => {
describe( 'viewToPlainText()', () => {
function test( viewString, expectedText ) {
const view = parseView( viewString );
const text = viewToPlainText( view );
Expand Down