Skip to content

Commit

Permalink
Merge pull request #8272 from ckeditor/i/8158
Browse files Browse the repository at this point in the history 
Fix (clipboard): Pasting plain text inside a link or restricted editing editable region will no longer break them. Closes #8158.

Tests (link): Added tests for pasting plain text over the link.
  • Loading branch information
@Reinmar
Reinmar authored Dec 2, 2020
2 parents c16b7f2 + 5050d12 commit 8d376fd
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 37 deletions.
41 changes: 28 additions & 13 deletions packages/ckeditor5-clipboard/src/clipboard.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,26 +115,36 @@ export default class Clipboard extends Plugin {
return;
}

// Plain text can be determined based on event flag (#7799) or auto detection (#1006). If detected
// preserve selection attributes on pasted items.
if ( data.asPlainText || isPlainTextFragment( modelFragment ) ) {
// Consider only formatting attributes.
const textAttributes = new Map( Array.from( modelDocument.selection.getAttributes() ).filter(
keyValuePair => editor.model.schema.getAttributeProperties( keyValuePair[ 0 ] ).isFormatting
) );

model.change( writer => {
model.change( writer => {
const selection = model.document.selection;

// Plain text can be determined based on event flag (#7799) or auto detection (#1006). If detected
// preserve selection attributes on pasted items.
if ( data.asPlainText || isPlainTextFragment( modelFragment, model.schema ) ) {
// Formatting attributes should be preserved.
const textAttributes = Array.from( selection.getAttributes() )
.filter( ( [ key ] ) => model.schema.getAttributeProperties( key ).isFormatting );

if ( !selection.isCollapsed ) {
model.deleteContent( selection, { doNotAutoparagraph: true } );
}

// But also preserve other attributes if they survived the content deletion (because they were not fully selected).
// For example linkHref is not a formatting attribute but it should be preserved if pasted text was in the middle
// of a link.
textAttributes.push( ...selection.getAttributes() );

const range = writer.createRangeIn( modelFragment );

for ( const item of range.getItems() ) {
if ( item.is( '$text' ) || item.is( '$textProxy' ) ) {
writer.setAttributes( textAttributes, item );
}
}
} );
}
}

model.insertContent( modelFragment );
model.insertContent( modelFragment );
} );

evt.stop();
}
Expand Down Expand Up @@ -235,13 +245,18 @@ export default class Clipboard extends Plugin {
// Returns true if specified `documentFragment` represents a plain text.
//
// @param {module:engine/view/documentfragment~DocumentFragment} documentFragment
// @param {module:engine/model/schema~Schema} schema
// @returns {Boolean}
function isPlainTextFragment( documentFragment ) {
function isPlainTextFragment( documentFragment, schema ) {
if ( documentFragment.childCount > 1 ) {
return false;
}

const child = documentFragment.getChild( 0 );

if ( schema.isObject( child ) ) {
return false;
}

return [ ...child.getAttributeKeys() ].length == 0;
}
55 changes: 47 additions & 8 deletions packages/ckeditor5-clipboard/tests/clipboard.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ describe( 'Clipboard feature', () => {

viewDocument.fire( 'paste', {
dataTransfer: dataTransferMock,
stopPropagation() {},
preventDefault() {}
} );

Expand Down Expand Up @@ -526,23 +527,61 @@ describe( 'Clipboard feature', () => {
expect( getModelData( model ) ).to.equal( '<paragraph><$text bold="true">Bolded []text.</$text></paragraph>' );
} );

it( 'ignores non-formatting text attributes', () => {
setModelData( model, '<paragraph><$text test="true">Bolded []text.</$text></paragraph>' );
it( 'should preserve non formatting attribute if it wasn\'t fully selected', () => {
setModelData( model, '<paragraph><$text test="true">Linked [text].</$text></paragraph>' );

const dataTransferMock = createDataTransfer( {
'text/html': 'foo',
'text/plain': 'foo'
viewDocument.fire( 'clipboardInput', {
dataTransfer: createDataTransfer( {
'text/html': 'foo',
'text/plain': 'foo'
} ),
stopPropagation() {},
preventDefault() {}
} );

expect( getModelData( model ) ).to.equal( '<paragraph><$text test="true">Linked foo[].</$text></paragraph>' );
} );

it( 'should not preserve non formatting attribute if it was fully selected', () => {
setModelData( model, '<paragraph><$text test="true">[Linked text.]</$text></paragraph>' );

viewDocument.fire( 'clipboardInput', {
dataTransfer: dataTransferMock,
asPlainText: false,
dataTransfer: createDataTransfer( {
'text/html': 'foo',
'text/plain': 'foo'
} ),
stopPropagation() {},
preventDefault() {}
} );

expect( getModelData( model ) ).to.equal( '<paragraph>foo[]</paragraph>' );
} );

it( 'should not treat a pasted object as a plain text', () => {
model.schema.register( 'obj', {
allowWhere: '$block',
isObject: true,
isBlock: true
} );

editor.conversion.elementToElement( { model: 'obj', view: 'obj' } );

setModelData( model, '<paragraph><$text bold="true">Bolded [text].</$text></paragraph>' );

viewDocument.fire( 'clipboardInput', {
dataTransfer: createDataTransfer( {
'text/html': '<obj></obj>',
'text/plain': 'foo'
} ),
stopPropagation() {},
preventDefault() {}
} );

expect( getModelData( model ) ).to.equal(
'<paragraph><$text test="true">Bolded </$text>foo[]<$text test="true">text.</$text></paragraph>' );
'<paragraph><$text bold="true">Bolded </$text></paragraph>' +
'[<obj></obj>]' +
'<paragraph><$text bold="true">.</$text></paragraph>'
);
} );
} );

Expand Down
72 changes: 56 additions & 16 deletions packages/ckeditor5-link/tests/linkediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ describe( 'LinkEditing', () => {
} );

editor.model.schema.extend( '$text', { allowAttributes: 'bold' } );
editor.model.schema.setAttributeProperties( 'bold', {
isFormatting: true
} );

editor.conversion.attributeToElement( {
model: 'bold',
Expand Down Expand Up @@ -143,7 +146,7 @@ describe( 'LinkEditing', () => {

// https://github.com/ckeditor/ckeditor5/issues/6053
describe( 'selection attribute management on paste', () => {
it( 'should remove link atttributes when pasting a link', () => {
it( 'should remove link attributes when pasting a link', () => {
setModelData( model, '<paragraph>foo[]</paragraph>' );

model.change( writer => {
Expand All @@ -155,7 +158,7 @@ describe( 'LinkEditing', () => {
expect( [ ...model.document.selection.getAttributeKeys() ] ).to.be.empty;
} );

it( 'should remove all atttributes starting with "link" (e.g. decorator attributes) when pasting a link', () => {
it( 'should remove all attributes starting with "link" (e.g. decorator attributes) when pasting a link', () => {
setModelData( model, '<paragraph>foo[]</paragraph>' );

model.change( writer => {
Expand All @@ -171,7 +174,7 @@ describe( 'LinkEditing', () => {
expect( [ ...model.document.selection.getAttributeKeys() ] ).to.be.empty;
} );

it( 'should not remove link atttributes when pasting a non-link content', () => {
it( 'should not remove link attributes when pasting a non-link content', () => {
setModelData( model, '<paragraph><$text linkHref="ckeditor.com">foo[]</$text></paragraph>' );

model.change( writer => {
Expand All @@ -188,7 +191,7 @@ describe( 'LinkEditing', () => {
expect( model.document.selection ).to.have.attribute( 'bold' );
} );

it( 'should not remove link atttributes when pasting in the middle of a link with the same URL', () => {
it( 'should not remove link attributes when pasting in the middle of a link with the same URL', () => {
setModelData( model, '<paragraph><$text linkHref="ckeditor.com">fo[]o</$text></paragraph>' );

model.change( writer => {
Expand All @@ -199,7 +202,7 @@ describe( 'LinkEditing', () => {
expect( model.document.selection ).to.have.attribute( 'linkHref' );
} );

it( 'should not remove link atttributes from the selection when pasting before a link when the gravity is overridden', () => {
it( 'should not remove link attributes from the selection when pasting before a link when the gravity is overridden', () => {
setModelData( model, '<paragraph>foo[]<$text linkHref="ckeditor.com">bar</$text></paragraph>' );

view.document.fire( 'keydown', {
Expand All @@ -226,7 +229,7 @@ describe( 'LinkEditing', () => {
expect( model.document.selection ).to.have.attribute( 'linkHref' );
} );

it( 'should not remove link atttributes when pasting a link into another link (different URLs, no merge)', () => {
it( 'should not remove link attributes when pasting a link into another link (different URLs, no merge)', () => {
setModelData( model, '<paragraph><$text linkHref="ckeditor.com">f[]oo</$text></paragraph>' );

model.change( writer => {
Expand All @@ -244,7 +247,7 @@ describe( 'LinkEditing', () => {
expect( model.document.selection ).to.have.attribute( 'linkHref' );
} );

it( 'should not remove link atttributes when pasting before another link (different URLs, no merge)', () => {
it( 'should not remove link attributes when pasting before another link (different URLs, no merge)', () => {
setModelData( model, '<paragraph>[]<$text linkHref="ckeditor.com">foo</$text></paragraph>' );

expect( model.document.selection ).to.have.property( 'isGravityOverridden', false );
Expand All @@ -263,6 +266,43 @@ describe( 'LinkEditing', () => {
expect( model.document.selection ).to.have.attribute( 'linkHref' );
expect( model.document.selection ).to.have.attribute( 'linkHref', 'http://INSERTED' );
} );

// https://github.com/ckeditor/ckeditor5/issues/8158
it( 'should expand link text on pasting plain text', () => {
setModelData( model, '<paragraph><$text linkHref="ckeditor.com">f[]oo</$text></paragraph>' );

view.document.fire( 'paste', {
dataTransfer: createDataTransfer( {
'text/html': '<p>bar</p>',
'text/plain': 'bar'
} ),
preventDefault: sinon.spy(),
stopPropagation: sinon.spy()
} );

expect( getModelData( model ) ).to.equal(
'<paragraph>' +
'<$text linkHref="ckeditor.com">fbar[]oo</$text>' +
'</paragraph>'
);
} );

it( 'doesn\'t affect attributes other than link', () => {
setModelData( model, '<paragraph><$text bold="true">[foo]</$text></paragraph>' );

view.document.fire( 'paste', {
dataTransfer: createDataTransfer( {
'text/html': '<p>bar</p>',
'text/plain': 'bar'
} ),
preventDefault: sinon.spy(),
stopPropagation: sinon.spy()
} );

expect( getModelData( model ) ).to.equal(
'<paragraph><$text bold="true">bar[]</$text></paragraph>'
);
} );
} );

describe( 'command', () => {
Expand Down Expand Up @@ -1397,15 +1437,6 @@ describe( 'LinkEditing', () => {
'<paragraph>This is Abcde[]from <$text linkHref="bar">Bar</$text>.</paragraph>'
);
} );

function createDataTransfer( data ) {
return {
getData( type ) {
return data[ type ];
},
setData() {}
};
}
} );

// https://github.com/ckeditor/ckeditor5/issues/7521
Expand Down Expand Up @@ -1586,4 +1617,13 @@ describe( 'LinkEditing', () => {
expect( model.document.selection.hasAttribute( 'linkHref' ), 'removing space after the link' ).to.equal( true );
} );
} );

function createDataTransfer( data ) {
return {
getData( type ) {
return data[ type ];
},
setData() {}
};
}
} );

0 comments on commit 8d376fd

Please sign in to comment.