Merge pull request #163 from ckeditor/t/162

Other: Removed LinkElement, using custom properties instead. Closes #162.

src/link.js

@@ -11,7 +11,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import ClickObserver from '@ckeditor/ckeditor5-engine/src/view/observer/clickobserver';
 import Range from '@ckeditor/ckeditor5-engine/src/view/range';
 import LinkEngine from './linkengine';
-import LinkElement from './linkelement';
+import { isLinkElement } from './utils';
 import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon';
 
 import clickOutsideHandler from '@ckeditor/ckeditor5-ui/src/bindings/clickoutsidehandler';
@@ -323,15 +323,15 @@ export default class Link extends Plugin {
 	}
 
 	/**
-	 * Returns the {@link module:link/linkelement~LinkElement} under
+	 * Returns the link {@link module:engine/view/attributeelement~AttributeElement} under
 	 * the {@link module:engine/view/document~Document editing view's} selection or `null`
 	 * if there is none.
 	 *
-	 * **Note**: For a non–collapsed selection the `LinkElement` is only returned when **fully**
+	 * **Note**: For a non–collapsed selection the link element is only returned when **fully**
 	 * selected and the **only** element within the selection boundaries.
 	 *
 	 * @private
-	 * @returns {module:link/linkelement~LinkElement|null}
+	 * @returns {module:engine/view/attributeelement~AttributeElement|null}
 	 */
 	_getSelectedLinkElement() {
 		const selection = this.editor.editing.view.selection;
@@ -340,7 +340,7 @@ export default class Link extends Plugin {
 			return findLinkElementAncestor( selection.getFirstPosition() );
 		} else {
 			// The range for fully selected link is usually anchored in adjacent text nodes.
-			// Trim it to get closer to the actual LinkElement.
+			// Trim it to get closer to the actual link element.
 			const range = selection.getFirstRange().getTrimmed();
 			const startLink = findLinkElementAncestor( range.start );
 			const endLink = findLinkElementAncestor( range.end );
@@ -349,7 +349,7 @@ export default class Link extends Plugin {
 				return null;
 			}
 
-			// Check if the LinkElement is fully selected.
+			// Check if the link element is fully selected.
 			if ( Range.createIn( startLink ).getTrimmed().isEqual( range ) ) {
 				return startLink;
 			} else {
@@ -359,11 +359,11 @@ export default class Link extends Plugin {
 	}
 }
 
-// Returns a `LinkElement` if there's one among the ancestors of the provided `Position`.
+// Returns a link element if there's one among the ancestors of the provided `Position`.
 //
 // @private
 // @param {module:engine/view/position~Position} View position to analyze.
-// @returns {module:link/linkelement~LinkElement|null} LinkElement at the position or null.
+// @returns {module:engine/view/attributeelement~AttributeElement|null} Link element at the position or null.
 function findLinkElementAncestor( position ) {
-	return position.getAncestors().find( ancestor => ancestor instanceof LinkElement );
+	return position.getAncestors().find( ancestor => isLinkElement( ancestor ) );
 }

src/linkengine.js

@@ -10,9 +10,9 @@
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';
 import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter';
-import LinkElement from './linkelement';
 import LinkCommand from './linkcommand';
 import UnlinkCommand from './unlinkcommand';
+import { createLinkElement } from './utils';
 
 /**
  * The link engine feature.
@@ -36,14 +36,7 @@ export default class LinkEngine extends Plugin {
 		// Build converter from model to view for data and editing pipelines.
 		buildModelConverter().for( data.modelToView, editing.modelToView )
 			.fromAttribute( 'linkHref' )
-			.toElement( linkHref => {
-				const linkElement = new LinkElement( 'a', { href: linkHref } );
-
-				// https://github.com/ckeditor/ckeditor5-link/issues/121
-				linkElement.priority = 5;
-
-				return linkElement;
-			} );
+			.toElement( linkHref => createLinkElement( linkHref ) );
 
 		// Build converter from view to model for data pipeline.
 		buildViewConverter().for( data.viewToModel )

src/utils.js

@@ -0,0 +1,38 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/**
+ * @module link/utils
+ */
+
+import AttributeElement from '@ckeditor/ckeditor5-engine/src/view/attributeelement';
+
+const linkElementSymbol = Symbol( 'linkElement' );
+
+/**
+ * Returns `true` if a given view node is the link element.
+ *
+ * @param {module:engine/view/node~Node} node
+ * @return {Boolean}
+ */
+export function isLinkElement( node ) {
+	return node.is( 'attributeElement' ) && !!node.getCustomProperty( linkElementSymbol );
+}
+
+/**
+ * Creates link {@link module:engine/view/attributeelement~AttributeElement} with provided `href` attribute.
+ *
+ * @param {String} href
+ * @return {module:engine/view/attributeelement~AttributeElement}
+ */
+export function createLinkElement( href ) {
+	const linkElement = new AttributeElement( 'a', { href } );
+	linkElement.setCustomProperty( linkElementSymbol, true );
+
+	// https://github.com/ckeditor/ckeditor5-link/issues/121
+	linkElement.priority = 5;
+
+	return linkElement;
+}

tests/link.js

@@ -606,14 +606,14 @@ describe( 'Link', () => {
 				sinon.assert.calledWithExactly( spy );
 			} );
 
-			it( 'should open when selection exclusively encloses a LinkElement (#1)', () => {
+			it( 'should open when selection exclusively encloses a link element (#1)', () => {
 				setModelData( editor.model, '[<$text linkHref="url">foo</$text>]' );
 
 				observer.fire( 'click', { target: {} } );
 				sinon.assert.calledWithExactly( spy );
 			} );
 
-			it( 'should open when selection exclusively encloses a LinkElement (#2)', () => {
+			it( 'should open when selection exclusively encloses a link element (#2)', () => {
 				setModelData( editor.model, '<$text linkHref="url">[foo]</$text>' );
 
 				observer.fire( 'click', { target: {} } );
@@ -627,35 +627,35 @@ describe( 'Link', () => {
 				sinon.assert.notCalled( spy );
 			} );
 
-			it( 'should not open when selection is non-collapsed and doesn\'t enclose a LinkElement (#1)', () => {
+			it( 'should not open when selection is non-collapsed and doesn\'t enclose a link element (#1)', () => {
 				setModelData( editor.model, '<$text linkHref="url">f[o]o</$text>' );
 
 				observer.fire( 'click', { target: {} } );
 				sinon.assert.notCalled( spy );
 			} );
 
-			it( 'should not open when selection is non-collapsed and doesn\'t enclose a LinkElement (#2)', () => {
+			it( 'should not open when selection is non-collapsed and doesn\'t enclose a link element (#2)', () => {
 				setModelData( editor.model, '<$text linkHref="url">[fo]o</$text>' );
 
 				observer.fire( 'click', { target: {} } );
 				sinon.assert.notCalled( spy );
 			} );
 
-			it( 'should not open when selection is non-collapsed and doesn\'t enclose a LinkElement (#3)', () => {
+			it( 'should not open when selection is non-collapsed and doesn\'t enclose a link element (#3)', () => {
 				setModelData( editor.model, '<$text linkHref="url">f[oo]</$text>' );
 
 				observer.fire( 'click', { target: {} } );
 				sinon.assert.notCalled( spy );
 			} );
 
-			it( 'should not open when selection is non-collapsed and doesn\'t enclose a LinkElement (#4)', () => {
+			it( 'should not open when selection is non-collapsed and doesn\'t enclose a link element (#4)', () => {
 				setModelData( editor.model, 'ba[r<$text linkHref="url">foo]</$text>' );
 
 				observer.fire( 'click', { target: {} } );
 				sinon.assert.notCalled( spy );
 			} );
 
-			it( 'should not open when selection is non-collapsed and doesn\'t enclose a LinkElement (#5)', () => {
+			it( 'should not open when selection is non-collapsed and doesn\'t enclose a link element (#5)', () => {
 				setModelData( editor.model, 'ba[r<$text linkHref="url">foo</$text>]' );
 
 				observer.fire( 'click', { target: {} } );

tests/linkengine.js

@@ -5,13 +5,13 @@
 
 import LinkEngine from '../src/linkengine';
 import LinkCommand from '../src/linkcommand';
-import LinkElement from '../src/linkelement';
 import UnlinkCommand from '../src/unlinkcommand';
 
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 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 buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';
 
@@ -99,10 +99,11 @@ describe( 'LinkEngine', () => {
 			expect( getViewData( editor.editing.view, { withoutSelection: true } ) ).to.equal( '<p><a href="url">foo</a>bar</p>' );
 		} );
 
-		it( 'should convert to `LinkElement` instance', () => {
+		it( 'should convert to link element instance', () => {
 			setModelData( model, '<paragraph><$text linkHref="url">foo</$text>bar</paragraph>' );
 
-			expect( editor.editing.view.getRoot().getChild( 0 ).getChild( 0 ) ).to.be.instanceof( LinkElement );
+			const element = editor.editing.view.getRoot().getChild( 0 ).getChild( 0 );
+			expect( isLinkElement( element ) ).to.be.true;
 		} );
 
 		// https://github.com/ckeditor/ckeditor5-link/issues/121

tests/utils.js

@@ -0,0 +1,43 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import AttributeElement from '@ckeditor/ckeditor5-engine/src/view/attributeelement';
+import ContainerElement from '@ckeditor/ckeditor5-engine/src/view/containerelement';
+import Text from '@ckeditor/ckeditor5-engine/src/view/text';
+
+import { createLinkElement, isLinkElement } from '../src/utils';
+
+describe( 'utils', () => {
+	describe( 'isLinkElement', () => {
+		it( 'should return true for elements created by createLinkElement', () => {
+			const element = createLinkElement( 'http://ckeditor.com' );
+
+			expect( isLinkElement( element ) ).to.be.true;
+		} );
+
+		it( 'should return false for other AttributeElements', () => {
+			expect( isLinkElement( new AttributeElement( 'a' ) ) ).to.be.false;
+		} );
+
+		it( 'should return false for ContainerElements', () => {
+			expect( isLinkElement( new ContainerElement( 'p' ) ) ).to.be.false;
+		} );
+
+		it( 'should return false for text nodes', () => {
+			expect( isLinkElement( new Text( 'foo' ) ) ).to.be.false;
+		} );
+	} );
+
+	describe( 'createLinkElement', () => {
+		it( 'should create link AttributeElement', () => {
+			const element = createLinkElement( 'http://cksource.com' );
+
+			expect( isLinkElement( element ) ).to.be.true;
+			expect( element.priority ).to.equal( 5 );
+			expect( element.getAttribute( 'href' ) ).to.equal( 'http://cksource.com' );
+			expect( element.name ).to.equal( 'a' );
+		} );
+	} );
+} );