diff --git a/packages/ckeditor5-link/package.json b/packages/ckeditor5-link/package.json index 3935234f533..8abd6a59f9d 100644 --- a/packages/ckeditor5-link/package.json +++ b/packages/ckeditor5-link/package.json @@ -10,6 +10,7 @@ "ckeditor5-plugin" ], "dependencies": { + "@ckeditor/ckeditor5-clipboard": "^20.0.0", "@ckeditor/ckeditor5-core": "^20.0.0", "@ckeditor/ckeditor5-engine": "^20.0.0", "@ckeditor/ckeditor5-image": "^20.0.0", @@ -21,7 +22,6 @@ "devDependencies": { "@ckeditor/ckeditor5-basic-styles": "^20.0.0", "@ckeditor/ckeditor5-block-quote": "^20.0.0", - "@ckeditor/ckeditor5-clipboard": "^20.0.0", "@ckeditor/ckeditor5-code-block": "^20.0.0", "@ckeditor/ckeditor5-editor-classic": "^20.0.0", "@ckeditor/ckeditor5-enter": "^20.0.0", diff --git a/packages/ckeditor5-link/src/linkediting.js b/packages/ckeditor5-link/src/linkediting.js index 9d8b633e59d..5376f25e5a1 100644 --- a/packages/ckeditor5-link/src/linkediting.js +++ b/packages/ckeditor5-link/src/linkediting.js @@ -10,6 +10,8 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import MouseObserver from '@ckeditor/ckeditor5-engine/src/view/observer/mouseobserver'; import TwoStepCaretMovement from '@ckeditor/ckeditor5-typing/src/twostepcaretmovement'; +import Input from '@ckeditor/ckeditor5-typing/src/input'; +import Clipboard from '@ckeditor/ckeditor5-clipboard/src/clipboard'; import LinkCommand from './linkcommand'; import UnlinkCommand from './unlinkcommand'; import AutomaticDecorators from './utils/automaticdecorators'; @@ -44,7 +46,8 @@ export default class LinkEditing extends Plugin { * @inheritDoc */ static get requires() { - return [ TwoStepCaretMovement ]; + // Clipboard is required for handling cut and paste events while typing over the link. + return [ TwoStepCaretMovement, Input, Clipboard ]; } /** @@ -110,6 +113,9 @@ export default class LinkEditing extends Plugin { // Handle a click at the beginning/end of a link element. this._enableClickingAfterLink(); + + // Handle typing over the link. + this._enableTypingOverLink(); } /** @@ -414,4 +420,132 @@ export default class LinkEditing extends Plugin { } } ); } + + /** + * Starts listening to {@link module:engine/model/model~Model#deleteContent} and {@link module:engine/model/model~Model#insertContent} + * and checks whether typing over the link. If so, attributes of removed text are preserved and applied to the inserted text. + * + * The purpose of this action is to allow modifying a text without loosing the `linkHref` attribute (and other). + * + * See https://github.com/ckeditor/ckeditor5/issues/4762. + * + * @private + */ + _enableTypingOverLink() { + const editor = this.editor; + const view = editor.editing.view; + + // Selection attributes when started typing over the link. + let selectionAttributes; + + // Whether pressed `Backspace` or `Delete`. If so, attributes should not be preserved. + let deletedContent; + + // Detect pressing `Backspace` / `Delete`. + this.listenTo( view.document, 'delete', () => { + deletedContent = true; + }, { priority: 'high' } ); + + // Listening to `model#deleteContent` allows detecting whether selected content was a link. + // If so, before removing the element, we will copy its attributes. + this.listenTo( editor.model, 'deleteContent', () => { + const selection = editor.model.document.selection; + + // Copy attributes only if anything is selected. + if ( selection.isCollapsed ) { + return; + } + + // When the content was deleted, do not preserve attributes. + if ( deletedContent ) { + deletedContent = false; + + return; + } + + // Enabled only when typing. + if ( !isTyping( editor ) ) { + return; + } + + if ( shouldCopyAttributes( editor.model ) ) { + selectionAttributes = selection.getAttributes(); + } + }, { priority: 'high' } ); + + // Listening to `model#insertContent` allows detecting the content insertion. + // We want to apply attributes that were removed while typing over the link. + this.listenTo( editor.model, 'insertContent', ( evt, [ element ] ) => { + deletedContent = false; + + // Enabled only when typing. + if ( !isTyping( editor ) ) { + return; + } + + if ( !selectionAttributes ) { + return; + } + + editor.model.change( writer => { + for ( const [ attribute, value ] of selectionAttributes ) { + writer.setAttribute( attribute, value, element ); + } + } ); + + selectionAttributes = null; + }, { priority: 'high' } ); + } +} + +// Checks whether selection's attributes should be copied to the new inserted text. +// +// @param {module:engine/model/model~Model} model +// @returns {Boolean} +function shouldCopyAttributes( model ) { + const selection = model.document.selection; + const firstPosition = selection.getFirstPosition(); + const lastPosition = selection.getLastPosition(); + const nodeAtFirstPosition = firstPosition.nodeAfter; + + // The text link node does not exist... + if ( !nodeAtFirstPosition ) { + return false; + } + + // ...or it isn't the text node... + if ( !nodeAtFirstPosition.is( 'text' ) ) { + return false; + } + + // ...or isn't the link. + if ( !nodeAtFirstPosition.hasAttribute( 'linkHref' ) ) { + return false; + } + + // `textNode` = the position is inside the link element. + // `nodeBefore` = the position is at the end of the link element. + const nodeAtLastPosition = lastPosition.textNode || lastPosition.nodeBefore; + + // If both references the same node selection contains a single text node. + if ( nodeAtFirstPosition === nodeAtLastPosition ) { + return true; + } + + // If nodes are not equal, maybe the link nodes has defined additional attributes inside. + // First, we need to find the entire link range. + const linkRange = findLinkRange( firstPosition, nodeAtFirstPosition.getAttribute( 'linkHref' ), model ); + + // Then we can check whether selected range is inside the found link range. If so, attributes should be preserved. + return linkRange.containsRange( model.createRange( firstPosition, lastPosition ), true ); +} + +// Checks whether provided changes were caused by typing. +// +// @params {module:core/editor/editor~Editor} editor +// @returns {Boolean} +function isTyping( editor ) { + const input = editor.plugins.get( 'Input' ); + + return input.isInput( editor.model.change( writer => writer.batch ) ); } diff --git a/packages/ckeditor5-link/tests/linkediting.js b/packages/ckeditor5-link/tests/linkediting.js index 12b31e754ab..c7f73fc8c42 100644 --- a/packages/ckeditor5-link/tests/linkediting.js +++ b/packages/ckeditor5-link/tests/linkediting.js @@ -8,15 +8,19 @@ import LinkCommand from '../src/linkcommand'; import UnlinkCommand from '../src/unlinkcommand'; import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; -import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; -import Enter from '@ckeditor/ckeditor5-enter/src/enter'; import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; +import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting'; +import ItalicEditing from '@ckeditor/ckeditor5-basic-styles/src/italic/italicediting'; +import Clipboard from '@ckeditor/ckeditor5-clipboard/src/clipboard'; +import Enter from '@ckeditor/ckeditor5-enter/src/enter'; +import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventdata'; +import ImageEditing from '@ckeditor/ckeditor5-image/src/image/imageediting'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Input from '@ckeditor/ckeditor5-typing/src/input'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; -import { isLinkElement } from '../src/utils'; import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; -import Typing from '@ckeditor/ckeditor5-typing/src/typing'; -import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting'; +import { isLinkElement } from '../src/utils'; /* global document */ @@ -869,7 +873,7 @@ describe( 'LinkEditing', () => { beforeEach( async () => { editor = await ClassicTestEditor.create( element, { - plugins: [ Paragraph, LinkEditing, Enter, Typing, BoldEditing ], + plugins: [ Paragraph, LinkEditing, Enter, Input, BoldEditing ], link: { decorators: { isFoo: { @@ -892,6 +896,11 @@ describe( 'LinkEditing', () => { model = editor.model; view = editor.editing.view; + + model.schema.extend( '$text', { + allowIn: '$root', + allowAttributes: [ 'linkIsFoo', 'linkIsBar' ] + } ); } ); afterEach( async () => { @@ -1033,11 +1042,6 @@ describe( 'LinkEditing', () => { } ); it( 'should remove manual decorators', () => { - model.schema.extend( '$text', { - allowIn: '$root', - allowAttributes: [ 'linkIsFoo', 'linkIsBar' ] - } ); - setModelData( model, '<$text linkIsFoo="true" linkIsBar="true" linkHref="url">Bar[]' ); editor.editing.view.document.fire( 'mousedown' ); @@ -1056,4 +1060,340 @@ describe( 'LinkEditing', () => { ); } ); } ); + + // https://github.com/ckeditor/ckeditor5/issues/4762 + describe( 'typing over the link', () => { + let editor; + + beforeEach( async () => { + editor = await ClassicTestEditor.create( element, { + plugins: [ Paragraph, LinkEditing, Enter, BoldEditing, ItalicEditing, ImageEditing ], + link: { + decorators: { + isFoo: { + mode: 'manual', + label: 'Foo', + attributes: { + class: 'foo' + } + }, + isBar: { + mode: 'manual', + label: 'Bar', + attributes: { + target: '_blank' + } + } + } + } + } ); + + model = editor.model; + view = editor.editing.view; + + model.schema.extend( '$text', { + allowIn: '$root', + allowAttributes: [ 'linkIsFoo', 'linkIsBar' ] + } ); + } ); + + afterEach( async () => { + await editor.destroy(); + } ); + + it( 'should require Clipboard plugin', () => { + expect( LinkEditing.requires.includes( Clipboard ) ).to.equal( true ); + } ); + + it( 'should require Input plugin', () => { + expect( LinkEditing.requires.includes( Input ) ).to.equal( true ); + } ); + + it( 'should preserve selection attributes when the entire link is selected', () => { + setModelData( model, + 'This is [<$text linkHref="foo">Foo] from <$text linkHref="bar">Bar.' + ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( + 'This is <$text linkHref="foo">Abcde[] from <$text linkHref="bar">Bar.' + ); + } ); + + it( 'should preserve selection attributes when the entire link is selected (mixed attributes in the link)', () => { + setModelData( model, + '' + + 'This is [' + + '<$text linkHref="foo" italic="true">F' + + '<$text linkHref="foo" bold="true">o' + + '<$text linkHref="foo" bold="true" italic="true">o' + + '<$text linkHref="foo" bold="true">B' + + '<$text linkHref="foo" bold="true" italic="true">a' + + '<$text linkHref="foo">r]' + + ' from ' + + '<$text linkHref="bar">Bar' + + '.' + + '' + ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( + '' + + 'This is ' + + '<$text italic="true" linkHref="foo">Abcde' + + '<$text italic="true">[]' + + ' from ' + + '<$text linkHref="bar">Bar' + + '.' + + '' + ); + } ); + + it( 'should preserve selection attributes when the selection starts at the beginning of the link', () => { + setModelData( model, + 'This is [<$text linkHref="foo">Fo]o from <$text linkHref="bar">Bar.' + ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( + 'This is <$text linkHref="foo">Abcde[]o from <$text linkHref="bar">Bar.' + ); + } ); + + it( 'should preserve selection attributes when it starts at the beginning of the link (mixed attributes in the link)', () => { + setModelData( model, + '' + + 'This is [' + + '<$text linkHref="foo" italic="true">F' + + '<$text linkHref="foo" bold="true">o' + + '<$text linkHref="foo" bold="true" italic="true">o' + + '<$text linkHref="foo" bold="true">B' + + '<$text linkHref="foo" bold="true" italic="true">a]' + + '<$text linkHref="foo">r' + + ' from ' + + '<$text linkHref="bar">Bar' + + '.' + + '' + ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( + '' + + 'This is ' + + '<$text italic="true" linkHref="foo">Abcde[]' + + '<$text linkHref="foo">r' + + ' from ' + + '<$text linkHref="bar">Bar' + + '.' + + '' + ); + } ); + + it( 'should preserve all attributes of the link node (decorators check)', () => { + setModelData( model, + '' + + 'This is ' + + '<$text linkIsFoo="true" linkIsBar="true" linkHref="foo">[Foo]' + + ' from ' + + '<$text linkHref="bar">Bar' + + '.' + + '' + ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( + '' + + 'This is ' + + '<$text linkHref="foo" linkIsBar="true" linkIsFoo="true">Abcde[]' + + ' from ' + + '<$text linkHref="bar">Bar' + + '.' + + '' + ); + } ); + + it( 'should not preserve selection attributes when the changes are not caused by typing', () => { + setModelData( model, + 'This is [<$text linkHref="foo">Foo] from <$text linkHref="bar">Bar.' + ); + + model.change( writer => { + model.deleteContent( model.document.selection ); + model.insertContent( writer.createText( 'Abcde' ) ); + } ); + + expect( getModelData( model ) ).to.equal( + 'This is Abcde[] from <$text linkHref="bar">Bar.' + ); + } ); + + it( 'should not preserve selection attributes when the changes are not caused by typing (pasting check)', () => { + setModelData( model, + 'This is [<$text linkHref="foo">Foo] from <$text linkHref="bar">Bar.' + ); + + view.document.fire( 'paste', { + dataTransfer: createDataTransfer( { + 'text/html': '

Abcde

', + 'text/plain': 'Abcde' + } ), + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + } ); + + expect( getModelData( model ) ).to.equal( + 'This is Abcde[] from <$text linkHref="bar">Bar.' + ); + } ); + + it( 'should not preserve selection attributes when typed after cutting the content', () => { + setModelData( model, + 'This is [<$text linkHref="foo">Foo] from <$text linkHref="bar">Bar.' + ); + + view.document.fire( 'cut', { + dataTransfer: createDataTransfer(), + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + } ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( + 'This is Abcde[] from <$text linkHref="bar">Bar.' + ); + } ); + + it( 'should not preserve anything if selected an element instead of text', () => { + setModelData( model, + '[]' + ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( + 'Abcde[]' + ); + } ); + + it( 'should not preserve anything if selected text does not have the `linkHref` attribute`', () => { + setModelData( model, + 'This is [<$text bold="foo">Foo] from <$text linkHref="bar">Bar.' + ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( + 'This is Abcde[] from <$text linkHref="bar">Bar.' + ); + } ); + + it( 'should not preserve selection attributes when the entire link is selected and pressed "Backspace"', () => { + setModelData( model, + 'This is [<$text linkHref="foo">Foo] from <$text linkHref="bar">Bar.' + ); + + view.document.fire( 'delete', new DomEventData( view.document, { + keyCode: keyCodes.backspace, + preventDefault: () => {} + } ) ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( + 'This is Abcde[] from <$text linkHref="bar">Bar.' + ); + } ); + + it( 'should not preserve selection attributes when the entire link is selected and pressed "Delete"', () => { + setModelData( model, + 'This is [<$text linkHref="foo">Foo] from <$text linkHref="bar">Bar.' + ); + + view.document.fire( 'delete', new DomEventData( view.document, { + keyCode: keyCodes.delete, + preventDefault: () => {} + } ) ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( + 'This is Abcde[] from <$text linkHref="bar">Bar.' + ); + } ); + + it( 'should not preserve selection attributes when selected different links', () => { + setModelData( model, + 'This is <$text linkHref="foo">[Foo from <$text linkHref="bar">Bar].' + ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( 'This is Abcde[].' ); + } ); + + it( 'should not preserve selection attributes when selected more than single link (start of the selection)', () => { + setModelData( model, + 'This is[ <$text linkHref="foo">Foo] from <$text linkHref="bar">Bar.' + ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( + 'This isAbcde[] from <$text linkHref="bar">Bar.' + ); + } ); + + it( 'should not preserve selection attributes when selected more than single link (end of the selection)', () => { + setModelData( model, + 'This is <$text linkHref="foo">[Foo ]from <$text linkHref="bar">Bar.' + ); + + editor.execute( 'input', { + text: 'Abcde' + } ); + + expect( getModelData( model ) ).to.equal( + 'This is Abcde[]from <$text linkHref="bar">Bar.' + ); + } ); + + function createDataTransfer( data ) { + return { + getData( type ) { + return data[ type ]; + }, + setData() {} + }; + } + } ); } ); diff --git a/packages/ckeditor5-typing/src/inputcommand.js b/packages/ckeditor5-typing/src/inputcommand.js index 9f473469078..5b31ab3a5f3 100644 --- a/packages/ckeditor5-typing/src/inputcommand.js +++ b/packages/ckeditor5-typing/src/inputcommand.js @@ -89,6 +89,9 @@ export default class InputCommand extends Command { model.enqueueChange( this._buffer.batch, writer => { this._buffer.lock(); + // Store the batch as an 'input' batch for the Input.isInput( batch ) check. + this._batches.add( this._buffer.batch ); + model.deleteContent( selection ); if ( text ) { @@ -104,9 +107,6 @@ export default class InputCommand extends Command { this._buffer.unlock(); this._buffer.input( textInsertions ); - - // Store the batch as an 'input' batch for the Input.isInput( batch ) check. - this._batches.add( this._buffer.batch ); } ); } } diff --git a/packages/ckeditor5-typing/src/utils/injectunsafekeystrokeshandling.js b/packages/ckeditor5-typing/src/utils/injectunsafekeystrokeshandling.js index 718fac76cdc..f8977fc49f2 100644 --- a/packages/ckeditor5-typing/src/utils/injectunsafekeystrokeshandling.js +++ b/packages/ckeditor5-typing/src/utils/injectunsafekeystrokeshandling.js @@ -110,7 +110,10 @@ export default function injectUnsafeKeystrokesHandling( editor ) { buffer.lock(); - model.enqueueChange( buffer.batch, () => { + const batch = buffer.batch; + inputCommand._batches.add( batch ); + + model.enqueueChange( batch, () => { model.deleteContent( model.document.selection ); } ); diff --git a/packages/ckeditor5-typing/tests/inputcommand.js b/packages/ckeditor5-typing/tests/inputcommand.js index b95cf69a7dd..cf95fd9bcd5 100644 --- a/packages/ckeditor5-typing/tests/inputcommand.js +++ b/packages/ckeditor5-typing/tests/inputcommand.js @@ -12,7 +12,7 @@ import ChangeBuffer from '../src/utils/changebuffer'; import Input from '../src/input'; describe( 'InputCommand', () => { - let editor, model, doc, buffer; + let editor, model, doc, buffer, inputCommand; testUtils.createSinonSandbox(); @@ -23,7 +23,7 @@ describe( 'InputCommand', () => { model = editor.model; doc = model.document; - const inputCommand = new InputCommand( editor, 20 ); + inputCommand = new InputCommand( editor, 20 ); editor.commands.add( 'input', inputCommand ); buffer = inputCommand.buffer; @@ -281,6 +281,28 @@ describe( 'InputCommand', () => { 'z' ); } ); + + it( 'uses typing batch while removing and inserting the content', () => { + expect( inputCommand._batches.has( getCurrentBatch() ), 'batch before typing' ).to.equal( false ); + + model.on( 'deleteContent', () => { + expect( inputCommand._batches.has( getCurrentBatch() ), 'batch when deleting content' ).to.equal( true ); + }, { priority: 'highest' } ); + + model.on( 'insertContent', () => { + expect( inputCommand._batches.has( getCurrentBatch() ), 'batch when inserting content' ).to.equal( true ); + }, { priority: 'lowest' } ); + + setData( model, '[foo]' ); + + editor.execute( 'input', { text: 'bar' } ); + + expect( getData( model ) ).to.equal( 'bar[]' ); + + function getCurrentBatch() { + return editor.model.change( writer => writer.batch ); + } + } ); } ); describe( 'destroy()', () => { diff --git a/packages/ckeditor5-typing/tests/utils/injectunsafekeystrokeshandling.js b/packages/ckeditor5-typing/tests/utils/injectunsafekeystrokeshandling.js index a283a830a31..65a7e179f2c 100644 --- a/packages/ckeditor5-typing/tests/utils/injectunsafekeystrokeshandling.js +++ b/packages/ckeditor5-typing/tests/utils/injectunsafekeystrokeshandling.js @@ -3,10 +3,12 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -import { - keyCodes -} from '@ckeditor/ckeditor5-utils/src/keyboard'; +import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; +import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventdata'; +import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import { keyCodes, getCode } from '@ckeditor/ckeditor5-utils/src/keyboard'; import { isNonTypingKeystroke } from '../../src/utils/injectunsafekeystrokeshandling'; +import Typing from '../../src/typing'; describe( 'unsafe keystroke handling utils', () => { describe( 'isNonTypingKeystroke()', () => { @@ -82,4 +84,45 @@ describe( 'unsafe keystroke handling utils', () => { expect( isNonTypingKeystroke( { keyCode: keyCodes.a, altKey: true } ), 'Alt+a' ).to.be.false; } ); } ); + + describe( 'injectUnsafeKeystrokesHandling()', () => { + let editor, model; + + beforeEach( () => { + return ModelTestEditor.create( { plugins: [ Typing ] } ) + .then( newEditor => { + editor = newEditor; + model = editor.model; + + model.schema.register( 'paragraph', { inheritAllFrom: '$block' } ); + } ); + } ); + + afterEach( () => { + return editor.destroy(); + } ); + + it( 'uses typing batch while removing the content', () => { + const inputCommand = editor.commands.get( 'input' ); + + expect( inputCommand._batches.has( getCurrentBatch() ), 'batch before typing' ).to.equal( false ); + + model.on( 'deleteContent', () => { + expect( inputCommand._batches.has( getCurrentBatch() ), 'batch when deleting content' ).to.equal( true ); + }, { priority: 'highest' } ); + + setData( model, '[foo]' ); + + editor.editing.view.document.fire( 'keydown', new DomEventData( editor.editing.view.document, { + preventDefault: () => {}, + keyCode: getCode( 'A' ) + } ) ); + + expect( getData( model ) ).to.equal( '[]' ); + + function getCurrentBatch() { + return editor.model.change( writer => writer.batch ); + } + } ); + } ); } );