Skip to content

Commit

Permalink
Merge pull request #7668 from ckeditor/i/7521
Browse files Browse the repository at this point in the history 
Fix (link): After backspacing into a link, the caret should still stay outside the link. Closes #7521.
  • Loading branch information
@jodator
jodator authored Jul 23, 2020
2 parents 1855df4 + 4df3fa2 commit c175e1c
Show file tree
Hide file tree
Showing 2 changed files with 276 additions and 12 deletions.
108 changes: 96 additions & 12 deletions packages/ckeditor5-link/src/linkediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import LinkCommand from './linkcommand';
import UnlinkCommand from './unlinkcommand';
import ManualDecorator from './utils/manualdecorator';
import findAttributeRange from '@ckeditor/ckeditor5-typing/src/utils/findattributerange';
import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
import { createLinkElement, ensureSafeUrl, getLocalizedDecorators, normalizeDecorators } from './utils';

import '../theme/link.css';
Expand Down Expand Up @@ -116,6 +117,9 @@ export default class LinkEditing extends Plugin {

// Handle typing over the link.
this._enableTypingOverLink();

// Handle removing the content after the link element.
this._handleDeleteContentAfterLink();
}

/**
Expand Down Expand Up @@ -223,8 +227,9 @@ export default class LinkEditing extends Plugin {
const editor = this.editor;
const model = editor.model;
const selection = model.document.selection;
const linkCommand = editor.commands.get( 'link' );

model.on( 'insertContent', () => {
this.listenTo( model, 'insertContent', () => {
const nodeBefore = selection.anchor.nodeBefore;
const nodeAfter = selection.anchor.nodeAfter;

Expand Down Expand Up @@ -291,13 +296,8 @@ export default class LinkEditing extends Plugin {
return;
}

// Make the selection free of link-related model attributes.
// All link-related model attributes start with "link". That includes not only "linkHref"
// but also all decorator attributes (they have dynamic names).
model.change( writer => {
[ ...model.document.selection.getAttributeKeys() ]
.filter( name => name.startsWith( 'link' ) )
.forEach( name => writer.removeSelectionAttribute( name ) );
removeLinkAttributesFromSelection( writer, linkCommand.manualDecorators );
} );
}, { priority: 'low' } );
}
Expand All @@ -315,6 +315,7 @@ export default class LinkEditing extends Plugin {
*/
_enableClickingAfterLink() {
const editor = this.editor;
const linkCommand = editor.commands.get( 'link' );

editor.editing.view.addObserver( MouseObserver );

Expand Down Expand Up @@ -353,11 +354,7 @@ export default class LinkEditing extends Plugin {
// If so, remove the `linkHref` attribute.
if ( position.isTouching( linkRange.start ) || position.isTouching( linkRange.end ) ) {
editor.model.change( writer => {
writer.removeSelectionAttribute( 'linkHref' );

for ( const manualDecorator of editor.commands.get( 'link' ).manualDecorators ) {
writer.removeSelectionAttribute( manualDecorator.id );
}
removeLinkAttributesFromSelection( writer, linkCommand.manualDecorators );
} );
}
} );
Expand Down Expand Up @@ -438,6 +435,93 @@ export default class LinkEditing extends Plugin {
selectionAttributes = null;
}, { priority: 'high' } );
}

/**
* Starts listening to {@link module:engine/model/model~Model#deleteContent} and checks whether
* removing a content right after the "linkHref" attribute.
*
* If so, the selection should not preserve the `linkHref` attribute. However, if
* the {@link module:typing/twostepcaretmovement~TwoStepCaretMovement} plugin is active and
* the selection has the "linkHref" attribute due to overriden gravity (at the end), the `linkHref` attribute should stay untouched.
*
* The purpose of this action is to allow removing the link text and keep the selection outside the link.
*
* See https://github.com/ckeditor/ckeditor5/issues/7521.
*
* @private
*/
_handleDeleteContentAfterLink() {
const editor = this.editor;
const model = editor.model;
const selection = model.document.selection;
const view = editor.editing.view;
const linkCommand = editor.commands.get( 'link' );

// A flag whether attributes `linkHref` attribute should be preserved.
let shouldPreserveAttributes = false;

// A flag whether the `Backspace` key was pressed.
let hasBackspacePressed = false;

// Detect pressing `Backspace`.
this.listenTo( view.document, 'delete', ( evt, data ) => {
hasBackspacePressed = data.domEvent.keyCode === keyCodes.backspace;
}, { priority: 'high' } );

// Before removing the content, check whether the selection is inside a link or at the end of link but with 2-SCM enabled.
// If so, we want to preserve link attributes.
this.listenTo( model, 'deleteContent', () => {
// Reset the state.
shouldPreserveAttributes = false;

const position = selection.getFirstPosition();
const linkHref = selection.getAttribute( 'linkHref' );

if ( !linkHref ) {
return;
}

const linkRange = findAttributeRange( position, 'linkHref', linkHref, model );

// Preserve `linkHref` attribute if the selection is in the middle of the link or
// the selection is at the end of the link and 2-SCM is activated.
shouldPreserveAttributes = linkRange.containsPosition( position ) || linkRange.end.isEqual( position );
}, { priority: 'high' } );

// After removing the content, check whether the current selection should preserve the `linkHref` attribute.
this.listenTo( model, 'deleteContent', () => {
// If didn't press `Backspace`.
if ( !hasBackspacePressed ) {
return;
}

hasBackspacePressed = false;

// Disable the mechanism if inside a link (`<$text url="foo">F[]oo</$text>` or <$text url="foo">Foo[]</$text>`).
if ( shouldPreserveAttributes ) {
return;
}

// Use `model.enqueueChange()` in order to execute the callback at the end of the changes process.
editor.model.enqueueChange( writer => {
removeLinkAttributesFromSelection( writer, linkCommand.manualDecorators );
} );
}, { priority: 'low' } );
}
}

// Make the selection free of link-related model attributes.
// All link-related model attributes start with "link". That includes not only "linkHref"
// but also all decorator attributes (they have dynamic names).
//
// @param {module:engine/model/writer~Writer} writer
// @param {module:utils/collection~Collection} manualDecorators
function removeLinkAttributesFromSelection( writer, manualDecorators ) {
writer.removeSelectionAttribute( 'linkHref' );

for ( const decorator of manualDecorators ) {
writer.removeSelectionAttribute( decorator.id );
}
}

// Checks whether selection's attributes should be copied to the new inserted text.
Expand Down
180 changes: 180 additions & 0 deletions packages/ckeditor5-link/tests/linkediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventd
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 Delete from '@ckeditor/ckeditor5-typing/src/delete';
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 { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
Expand Down Expand Up @@ -1406,4 +1407,183 @@ describe( 'LinkEditing', () => {
};
}
} );

// https://github.com/ckeditor/ckeditor5/issues/7521
describe( 'removing a character before the link element', () => {
let editor;

beforeEach( async () => {
editor = await ClassicTestEditor.create( element, {
plugins: [ Paragraph, LinkEditing, Delete, BoldEditing ],
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 not preserve the `linkHref` attribute when deleting content after the link', () => {
setModelData( model, '<paragraph>Foo <$text linkHref="url">Bar</$text> []</paragraph>' );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'initial state' ).to.equal( false );

view.document.fire( 'delete', new DomEventData( view.document, {
keyCode: keyCodes.backspace,
preventDefault: () => {}
} ) );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'removing space after the link' ).to.equal( false );

view.document.fire( 'delete', new DomEventData( view.document, {
keyCode: keyCodes.backspace,
preventDefault: () => {}
} ) );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'removing a character in the link' ).to.equal( false );
expect( getModelData( model ) ).to.equal( '<paragraph>Foo <$text linkHref="url">Ba</$text>[]</paragraph>' );
} );

it( 'should not preserve the `linkHref` attribute when deleting content after the link (decorators check)', () => {
setModelData( model,
'<paragraph>' +
'This is ' +
'<$text linkIsFoo="true" linkIsBar="true" linkHref="foo">Foo</$text>' +
' []from ' +
'<$text linkHref="bar">Bar</$text>' +
'.' +
'</paragraph>'
);

expect( model.document.selection.hasAttribute( 'linkHref' ), 'initial "linkHref" state' ).to.equal( false );
expect( model.document.selection.hasAttribute( 'linkIsFoo' ), 'initial "linkIsFoo" state' ).to.equal( false );
expect( model.document.selection.hasAttribute( 'linkHref' ), 'initial "linkHref" state' ).to.equal( false );

view.document.fire( 'delete', new DomEventData( view.document, {
keyCode: keyCodes.backspace,
preventDefault: () => {}
} ) );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'removing space after the link ("linkHref")' ).to.equal( false );
expect( model.document.selection.hasAttribute( 'linkIsFoo' ), 'removing space after the link ("linkIsFoo")' ).to.equal( false );
expect( model.document.selection.hasAttribute( 'linkHref' ), 'removing space after the link ("linkHref")' ).to.equal( false );

view.document.fire( 'delete', new DomEventData( view.document, {
keyCode: keyCodes.backspace,
preventDefault: () => {}
} ) );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'removing a character the link ("linkHref")' ).to.equal( false );
expect( model.document.selection.hasAttribute( 'linkIsFoo' ), 'removing a character the link ("linkIsFoo")' ).to.equal( false );
expect( model.document.selection.hasAttribute( 'linkHref' ), 'removing a character the link ("linkHref")' ).to.equal( false );

expect( getModelData( model ) ).to.equal(
'<paragraph>' +
'This is ' +
'<$text linkHref="foo" linkIsBar="true" linkIsFoo="true">Fo</$text>' +
'[]from ' +
'<$text linkHref="bar">Bar</$text>' +
'.' +
'</paragraph>'
);
} );

it( 'should preserve the `linkHref` attribute when deleting content while the selection is at the end of the link', () => {
setModelData( model, '<paragraph>Foo <$text linkHref="url">Bar []</$text></paragraph>' );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'initial state' ).to.equal( true );

view.document.fire( 'delete', new DomEventData( view.document, {
keyCode: keyCodes.backspace,
preventDefault: () => {}
} ) );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'removing space after the link' ).to.equal( true );

view.document.fire( 'delete', new DomEventData( view.document, {
keyCode: keyCodes.backspace,
preventDefault: () => {}
} ) );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'removing a character in the link' ).to.equal( true );
expect( getModelData( model ) ).to.equal( '<paragraph>Foo <$text linkHref="url">Ba[]</$text></paragraph>' );
} );

it( 'should preserve the `linkHref` attribute when deleting content while the selection is inside the link', () => {
setModelData( model, '<paragraph>Foo <$text linkHref="url">A long URLLs[] description</$text></paragraph>' );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'initial state' ).to.equal( true );

view.document.fire( 'delete', new DomEventData( view.document, {
keyCode: keyCodes.backspace,
preventDefault: () => {}
} ) );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'removing space after the link' ).to.equal( true );

view.document.fire( 'delete', new DomEventData( view.document, {
keyCode: keyCodes.backspace,
preventDefault: () => {}
} ) );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'removing a character in the link' ).to.equal( true );
expect( getModelData( model ) ).to.equal( '<paragraph>Foo <$text linkHref="url">A long URL[] description</$text></paragraph>' );
} );

it( 'should do nothing if there is no `linkHref` attribute', () => {
setModelData( model, '<paragraph>Foo <$text bold="true">Bolded.</$text> []Bar</paragraph>' );

view.document.fire( 'delete', new DomEventData( view.document, {
keyCode: keyCodes.backspace,
preventDefault: () => {}
} ) );

view.document.fire( 'delete', new DomEventData( view.document, {
keyCode: keyCodes.backspace,
preventDefault: () => {}
} ) );

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

it( 'should preserve the `linkHref` attribute when deleting content using "Delete" key', () => {
setModelData( model, '<paragraph>Foo <$text linkHref="url">Bar</$text>[ ]</paragraph>' );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'initial state' ).to.equal( false );

view.document.fire( 'delete', new DomEventData( view.document, {
keyCode: keyCodes.delete,
preventDefault: () => {}
}, { direction: 'forward' } ) );

expect( getModelData( model ) ).to.equal( '<paragraph>Foo <$text linkHref="url">Bar[]</$text></paragraph>' );

expect( model.document.selection.hasAttribute( 'linkHref' ), 'removing space after the link' ).to.equal( true );
} );
} );
} );

0 comments on commit c175e1c

Please sign in to comment.