8
0
Просмотр исходного кода

Chanded linking behaviour: Update existing link instead of inserting new one.

Oskar Wrobel 9 лет назад
Родитель
Сommit
64d164a868

+ 79 - 0
packages/ckeditor5-link/src/findLinkRange.js

@@ -0,0 +1,79 @@
+/**
+ * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import Range from '../engine/model/range.js';
+import Position from '../engine/model/position.js';
+
+/**
+ * Walk backward and forward from start position node by node as long as they have the same `linkHref` attribute value and return
+ * {@link engine.model.Range Range} with founded link.
+ *
+ * @param {engine.model.Position} position Start position.
+ * @param {String} value `linkHref` attribute value.
+ * @returns {engine.model.Range} Link range.
+ */
+export default function findLinkRange( position, value ) {
+	return new Range( _findBackwardBound( position, value ), _findForwardBound( position, value ) );
+}
+
+// Walk backward node by node as long as they have the same `linkHref` attribute value and return position just before last matched node.
+//
+// @param {engine.model.Position} position Start position.
+// @param {String} value `linkHref` attribute value.
+// @returns {engine.model.Position} Position just before last matched node.
+function _findBackwardBound( position, value ) {
+	// Get node before position. When position is inside text node then start searching from text node.
+	let node = position.textNode === null ? position.nodeBefore : position.textNode;
+
+	// When node doesn't match it means we are at the beginning of link.
+	if ( !node || node.getAttribute( 'linkHref' ) != value ) {
+		return position;
+	}
+
+	let lastNode = node;
+
+	while ( node ) {
+		// When node doesn't match it means that previous node is the last matches node and we need to return position before it.
+		if ( node.getAttribute( 'linkHref' ) != value ) {
+			return Position.createBefore( lastNode );
+		}
+
+		lastNode = node;
+		node = node.previousSibling;
+	}
+
+	// We reach the bound.
+	return Position.createBefore( lastNode );
+}
+
+// Walk forward node by node as long as they have the same `linkHref` attribute value and return position just after last matched node.
+//
+// @param {engine.model.Position} position Start position.
+// @param {String} value `linkHref` attribute value.
+// @returns {engine.model.Position} Position just after last matched node.
+function _findForwardBound( position, value ) {
+	// Get node after position. When position is inside text node then start searching from text node.
+	let node = position.textNode === null ? position.nodeAfter : position.textNode;
+
+	// When node doesn't match it means we are at the end of link.
+	if ( !node || node.getAttribute( 'linkHref' ) != value ) {
+		return position;
+	}
+
+	let lastNode = node;
+
+	while ( node ) {
+		// When node doesn't match it means that previous node is the last matches node and we need to return position after it.
+		if ( node.getAttribute( 'linkHref' ) != value ) {
+			return Position.createAfter( lastNode );
+		}
+
+		lastNode = node;
+		node = node.nextSibling;
+	}
+
+	// We reach the bound.
+	return Position.createAfter( lastNode );
+}

+ 20 - 7
packages/ckeditor5-link/src/linkcommand.js

@@ -8,6 +8,7 @@ import Text from '../engine/model/text.js';
 import Range from '../engine/model/range.js';
 import getSchemaValidRanges from '../core/command/helpers/getschemavalidranges.js';
 import isAttributeAllowedInSelection from '../core/command/helpers/isattributeallowedinselection.js';
+import findLinkRange from './findLinkRange.js';
 
 /**
  * The link command. It is used by the {@link Link.Link link feature}.
@@ -54,9 +55,11 @@ export default class LinkCommand extends Command {
 	 * When selection is non-collapsed then `linkHref` attribute will be applied to nodes inside selection, but only to
 	 * this nodes where `linkHref` attribute is allowed (disallowed nodes will be omitted).
 	 *
-	 * When selection is collapsed then new {@link engine.model.Text Text node} with `linkHref` attribute will be inserted
-	 * in place of caret, but only if such an element is allowed in this place. _data of inserted text will be equal
-	 * to `href` parameter. Selection will be updated to wrap just inserted text node.
+	 * When selection is collapsed and is not inside text with `linkHref` attribute then new {@link engine.model.Text Text node} with
+	 * `linkHref` attribute will be inserted in place of caret, but only if such an element is allowed in this place. _data of inserted
+	 * text will be equal to `href` parameter. Selection will be updated to wrap just inserted text node.
+	 *
+	 * When selection is collapsed and is inside text with `linkHref` attribute then attribute value will be updated.
 	 *
 	 * @protected
 	 * @param {String} href Link destination.
@@ -69,19 +72,29 @@ export default class LinkCommand extends Command {
 			// Keep it as one undo step.
 			const batch = document.batch();
 
+			// If selection is collapsed then update selected link or insert new one at the place of caret.
 			if ( selection.isCollapsed ) {
 				const position = selection.getFirstPosition();
 				const parent = position.parent;
 
-				// Insert Text node with linkHref attribute if is allowed in parent.
-				if ( document.schema.check( { name: '$text', attributes: 'linkHref', inside: parent.name } ) ) {
+				// When selection is inside text with `linkHref` attribute.
+				if ( selection.hasAttribute( 'linkHref' ) ) {
+					// Then update `linkHref` value.
+					const linkRange = findLinkRange( selection.getFirstPosition(), selection.getAttribute( 'linkHref' ) );
+
+					batch.setAttribute( linkRange, 'linkHref', href );
+
+					// Create new range wrapping changed link.
+					selection.setRanges( [ linkRange ] );
+
+				// If not then insert text node with `linkHref` attribute in place of caret.
+				} else if ( document.schema.check( { name: '$text', attributes: 'linkHref', inside: parent.name } ) ) {
 					const node = new Text( href, { linkHref: href } );
 
 					batch.insert( position, node );
 
-					// Create new range wrapping just created node.
+					// Create new range wrapping created node.
 					selection.setRanges( [ Range.createOn( node ) ] );
-					selection.setAttribute( 'linkHref', href );
 				}
 			} else {
 				// If selection has non-collapsed ranges, we change attribute on nodes inside those ranges

+ 10 - 39
packages/ckeditor5-link/src/unlinkcommand.js

@@ -4,6 +4,7 @@
  */
 
 import Command from '../core/command/command.js';
+import findLinkRange from './findLinkRange.js';
 
 /**
  * The unlink command. It is used by the {@link Link.Link link feature}.
@@ -15,9 +16,9 @@ export default class UnlinkCommand extends Command {
 	/**
 	 * Executes the command.
 	 *
-	 * When selection is collapsed then walk through each stick node with `linkHref` attribute and remove this attribute.
+	 * When selection is collapsed then remove `linkHref` attribute from each stick node with the same `linkHref` attribute value.
 	 *
-	 * If selection is non-collapsed then remove `linkHref` from each node in selected range.
+	 * When selection is non-collapsed then remove `linkHref` from each node in selected ranges.
 	 *
 	 * @protected
 	 */
@@ -26,46 +27,16 @@ export default class UnlinkCommand extends Command {
 		const selection = document.selection;
 
 		document.enqueueChanges( () => {
+			// Get ranges to unlink.
+			const rangesToUnlink = selection.isCollapsed ?
+				[ findLinkRange( selection.getFirstPosition(), selection.getAttribute( 'linkHref' ) ) ] : selection.getRanges();
+
 			// Keep it as one undo step.
 			const batch = document.batch();
 
-			// When selection is collapsed we are removing `linkHref` attribute from every stick sibling with the same attribute value.
-			if ( selection.isCollapsed ) {
-				const linkValue = selection.getAttribute( 'linkHref' );
-				const position = selection.getFirstPosition();
-				let sibling;
-
-				// Get node on the right side of the selection. When selection is inside TextNode then get this node.
-				sibling = position.textNode === null ? position.nodeAfter : position.textNode;
-
-				// Walk forward and remove `linkHref` attribute from each stick element with the same attribute value.
-				while ( sibling ) {
-					if ( sibling.getAttribute( 'linkHref' ) == linkValue ) {
-						batch.removeAttribute( sibling, 'linkHref' );
-						sibling = sibling.nextSibling;
-					} else {
-						sibling = null;
-					}
-				}
-
-				// Get node on the left side of the selection. When selection is inside TextNode then get node just after
-				// TextNode because `linkHref` attribute has been already removed from TextNode during forward walking.
-				sibling = position.textNode === null ? position.nodeBefore : position.textNode.previousSibling;
-
-				// Walk backward and remove `linkHref` attribute from each stick element with the same attribute value.
-				while ( sibling ) {
-					if ( sibling.getAttribute( 'linkHref' ) == linkValue ) {
-						batch.removeAttribute( sibling, 'linkHref' );
-						sibling = sibling.previousSibling;
-					} else {
-						sibling = null;
-					}
-				}
-			} else {
-				// When selection is non-collapsed then we are removing `linkHref` attribute from each node inside the ranges.
-				for ( let range of selection.getRanges() ) {
-					batch.removeAttribute( range, 'linkHref' );
-				}
+			// Remove `linkHref` attribute from specified ranges.
+			for ( let range of rangesToUnlink ) {
+				batch.removeAttribute( range, 'linkHref' );
 			}
 		} );
 	}

+ 126 - 0
packages/ckeditor5-link/tests/findlinkrange.js

@@ -0,0 +1,126 @@
+/**
+ * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import findLinkRange from '/ckeditor5/link/findlinkrange.js';
+import Document from '/ckeditor5/engine/model/document.js';
+import Range from '/ckeditor5/engine/model/range.js';
+import Position from '/ckeditor5/engine/model/position.js';
+import { setData } from '/tests/engine/_utils/model.js';
+
+describe( 'findLinkRange', () => {
+	let document, root;
+
+	beforeEach( () => {
+		document = new Document();
+		root = document.createRoot();
+		document.schema.allow( { name: '$text', inside: '$root' } );
+		document.schema.registerItem( 'p', '$block' );
+	} );
+
+	it( 'should find link range searching from the center of the link #1', () => {
+		setData( document, '<$text linkHref="url">foobar</$text>' );
+
+		const startPosition = new Position( root, [ 3 ] );
+		const result = findLinkRange( startPosition, 'url' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 0, root, 6 ) ) ).to.true;
+	} );
+
+	it( 'should find link range searching from the center of the link #2', () => {
+		setData( document, 'abc <$text linkHref="url">foobar</$text> abc' );
+
+		const startPosition = new Position( root, [ 7 ] );
+		const result = findLinkRange( startPosition, 'url' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 4, root, 10 ) ) ).to.true;
+	} );
+
+	it( 'should find link range searching from the beginning of the link #1', () => {
+		setData( document, '<$text linkHref="url">foobar</$text>' );
+
+		const startPosition = new Position( root, [ 0 ] );
+		const result = findLinkRange( startPosition, 'url' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 0, root, 6 ) ) ).to.true;
+	} );
+
+	it( 'should find link range searching from the beginning of the link #2', () => {
+		setData( document, 'abc <$text linkHref="url">foobar</$text> abc' );
+
+		const startPosition = new Position( root, [ 4 ] );
+		const result = findLinkRange( startPosition, 'url' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 4, root, 10 ) ) ).to.true;
+	} );
+
+	it( 'should find link range searching from the end of the link #1', () => {
+		setData( document, '<$text linkHref="url">foobar</$text>' );
+
+		const startPosition = new Position( root, [ 6 ] );
+		const result = findLinkRange( startPosition, 'url' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 0, root, 6 ) ) ).to.true;
+	} );
+
+	it( 'should find link range searching from the end of the link #2', () => {
+		setData( document, 'abc <$text linkHref="url">foobar</$text> abc' );
+
+		const startPosition = new Position( root, [ 10 ] );
+		const result = findLinkRange( startPosition, 'url' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 4, root, 10 ) ) ).to.true;
+	} );
+
+	it( 'should find link range when link stick to other link searching from the center of the link', () => {
+		setData( document, '<$text linkHref="other">abc</$text><$text linkHref="url">foobar</$text><$text linkHref="other">abc</$text>' );
+
+		const startPosition = new Position( root, [ 6 ] );
+		const result = findLinkRange( startPosition, 'url' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 3, root, 9 ) ) ).to.true;
+	} );
+
+	it( 'should find link range when link stick to other link searching from the beginning of the link', () => {
+		setData( document, '<$text linkHref="other">abc</$text><$text linkHref="url">foobar</$text><$text linkHref="other">abc</$text>' );
+
+		const startPosition = new Position( root, [ 3 ] );
+		const result = findLinkRange( startPosition, 'url' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 3, root, 9 ) ) ).to.true;
+	} );
+
+	it( 'should find link range when link stick to other link searching from the end of the link', () => {
+		setData( document, '<$text linkHref="other">abc</$text><$text linkHref="url">foobar</$text><$text linkHref="other">abc</$text>' );
+
+		const startPosition = new Position( root, [ 9 ] );
+		const result = findLinkRange( startPosition, 'url' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( Range.createFromParentsAndOffsets( root, 3, root, 9 ) ) ).to.true;
+	} );
+
+	it( 'should find link range only inside current parent', () => {
+		setData(
+			document,
+			'<p><$text linkHref="url">foobar</$text></p>' +
+			'<p><$text linkHref="url">foobar</$text></p>' +
+			'<p><$text linkHref="url">foobar</$text></p>'
+		);
+
+		const startPosition = new Position( root, [ 1, 3 ] );
+		const result = findLinkRange( startPosition, 'url' );
+
+		expect( result ).to.instanceOf( Range );
+		expect( result.isEqual( new Range( new Position( root, [ 1, 0 ] ), new Position( root, [ 1, 6 ] ) ) ) ).to.true;
+	} );
+} );

+ 17 - 23
packages/ckeditor5-link/tests/linkcommand.js

@@ -20,7 +20,7 @@ describe( 'LinkCommand', () => {
 				// Allow text in $root.
 				document.schema.allow( { name: '$text', inside: '$root' } );
 
-				// Allow text with linkHref attribute in paragraph.
+				// Allow text with `linkHref` attribute in paragraph.
 				document.schema.registerItem( 'p', '$block' );
 				document.schema.allow( { name: '$text', attributes: 'linkHref', inside: '$root' } );
 			} );
@@ -32,13 +32,13 @@ describe( 'LinkCommand', () => {
 
 	describe( 'value', () => {
 		describe( 'collapsed selection', () => {
-			it( 'should be equal attribute value when selection is placed inside element with linkHref attribute', () => {
+			it( 'should be equal attribute value when selection is placed inside element with `linkHref` attribute', () => {
 				setData( document, `<$text linkHref="url">foo[]bar</$text>` );
 
 				expect( command.value ).to.equal( 'url' );
 			} );
 
-			it( 'should be undefined when selection is placed inside element without linkHref attribute', () => {
+			it( 'should be undefined when selection is placed inside element without `linkHref` attribute', () => {
 				setData( document, `<$text bold="true">foo[]bar</$text>` );
 
 				expect( command.value ).to.undefined;
@@ -46,13 +46,13 @@ describe( 'LinkCommand', () => {
 		} );
 
 		describe( 'non-collapsed selection', () => {
-			it( 'should be equal attribute value when selection contains only elements with linkHref attribute', () => {
+			it( 'should be equal attribute value when selection contains only elements with `linkHref` attribute', () => {
 				setData( document, 'fo[<$text linkHref="url">ob</$text>]ar' );
 
 				expect( command.value ).to.equal( 'url' );
 			} );
 
-			it( 'should be undefined when selection contains not only elements with linkHref attribute', () => {
+			it( 'should be undefined when selection contains not only elements with `linkHref` attribute', () => {
 				setData( document, 'f[o<$text linkHref="url">ob</$text>]ar' );
 
 				expect( command.value ).to.undefined;
@@ -62,7 +62,7 @@ describe( 'LinkCommand', () => {
 
 	describe( '_doExecute', () => {
 		describe( 'non-collapsed selection', () => {
-			it( 'should set linkHref attribute to selected text', () => {
+			it( 'should set `linkHref` attribute to selected text', () => {
 				setData( document, 'f[ooba]r' );
 
 				expect( command.value ).to.undefined;
@@ -73,7 +73,7 @@ describe( 'LinkCommand', () => {
 				expect( command.value ).to.equal( 'url' );
 			} );
 
-			it( 'should set linkHref attribute to selected text when text already has attributes', () => {
+			it( 'should set `linkHref` attribute to selected text when text already has attributes', () => {
 				setData( document, 'f[o<$text bold="true">oba]r</$text>' );
 
 				expect( command.value ).to.undefined;
@@ -88,7 +88,7 @@ describe( 'LinkCommand', () => {
 				);
 			} );
 
-			it( 'should overwrite existing linkHref attribute when selected text wraps text with linkHref attribute', () => {
+			it( 'should overwrite existing `linkHref` attribute when selected text wraps text with `linkHref` attribute', () => {
 				setData( document, 'f[o<$text linkHref="other url">o</$text>ba]r' );
 
 				expect( command.value ).to.undefined;
@@ -99,7 +99,7 @@ describe( 'LinkCommand', () => {
 				expect( command.value ).to.equal( 'url' );
 			} );
 
-			it( 'should split text and overwrite attribute value when selection is inside text with linkHref attribute', () => {
+			it( 'should split text and overwrite attribute value when selection is inside text with `linkHref` attribute', () => {
 				setData( document, 'f<$text linkHref="other url">o[ob]a</$text>r' );
 
 				expect( command.value ).to.equal( 'other url' );
@@ -117,7 +117,7 @@ describe( 'LinkCommand', () => {
 			} );
 
 			it(
-				'should overwrite linkHref attribute of selected text only, when selection start inside text with linkHref attribute',
+				'should overwrite `linkHref` attribute of selected text only, when selection start inside text with `linkHref` attribute',
 			() => {
 				setData( document, 'f<$text linkHref="other url">o[o</$text>ba]r' );
 
@@ -129,7 +129,7 @@ describe( 'LinkCommand', () => {
 				expect( command.value ).to.equal( 'url' );
 			} );
 
-			it( 'should overwrite linkHref attribute of selected text only, when selection end inside text with linkHref attribute', () => {
+			it( 'should overwrite `linkHref` attribute of selected text only, when selection end inside text with `linkHref` attribute', () => {
 				setData( document, 'f[o<$text linkHref="other url">ob]a</$text>r' );
 
 				expect( command.value ).to.undefined;
@@ -140,7 +140,7 @@ describe( 'LinkCommand', () => {
 				expect( command.value ).to.equal( 'url' );
 			} );
 
-			it( 'should set linkHref attribute to selected text when text is split by $block element', () => {
+			it( 'should set `linkHref` attribute to selected text when text is split by $block element', () => {
 				setData( document, '<p>f[oo</p><p>ba]r</p>' );
 
 				expect( command.value ).to.undefined;
@@ -152,7 +152,7 @@ describe( 'LinkCommand', () => {
 				expect( command.value ).to.equal( 'url' );
 			} );
 
-			it( 'should set linkHref attribute only to allowed elements and omit disallowed', () => {
+			it( 'should set `linkHref` attribute only to allowed elements and omit disallowed', () => {
 				// Disallow text in img.
 				document.schema.registerItem( 'img', '$block' );
 				document.schema.disallow( { name: '$text', attributes: 'linkHref', inside: 'img' } );
@@ -170,7 +170,7 @@ describe( 'LinkCommand', () => {
 		} );
 
 		describe( 'collapsed selection', () => {
-			it( 'should insert text with linkHref attribute and data equal to href', () => {
+			it( 'should insert text with `linkHref` attribute, text data equal to href and select new link', () => {
 				setData( document, 'foo[]bar' );
 
 				command._doExecute( 'url' );
@@ -178,21 +178,15 @@ describe( 'LinkCommand', () => {
 				expect( getData( document ) ).to.equal( 'foo[<$text linkHref="url">url</$text>]bar' );
 			} );
 
-			it(
-				'should insert text with linkHref attribute and data equal to href when selection is inside text with linkHref attribute',
-			() => {
+			it( 'should update `linkHref` attribute and select whole link when selection is inside text with `linkHref` attribute', () => {
 				setData( document, '<$text linkHref="other url">foo[]bar</$text>' );
 
 				command._doExecute( 'url' );
 
-				expect( getData( document ) ).to.equal(
-					'<$text linkHref="other url">foo</$text>' +
-					'[<$text linkHref="url">url</$text>]' +
-					'<$text linkHref="other url">bar</$text>'
-				);
+				expect( getData( document ) ).to.equal( '[<$text linkHref="url">foobar</$text>]' );
 			} );
 
-			it( 'should not insert text with linkHref attribute when is not allowed in parent', () => {
+			it( 'should not insert text with `linkHref` attribute when is not allowed in parent', () => {
 				document.schema.disallow( { name: '$text', attributes: 'linkHref', inside: 'p' } );
 				setData( document, '<p>foo[]bar</p>' );