Merge pull request #132 from ckeditor/t/85

Fix: Linking an entire image should not be possible. Closes #85.

package.json

@@ -14,6 +14,7 @@
     "@ckeditor/ckeditor5-editor-classic": "^0.7.3",
     "@ckeditor/ckeditor5-enter": "^0.9.1",
     "@ckeditor/ckeditor5-heading": "^0.9.1",
+    "@ckeditor/ckeditor5-image": "^0.6.0",
     "@ckeditor/ckeditor5-paragraph": "^0.8.0",
     "@ckeditor/ckeditor5-typing": "^0.9.1",
     "@ckeditor/ckeditor5-undo": "^0.8.1",

src/linkcommand.js

@@ -33,7 +33,7 @@ export default class LinkCommand extends Command {
 		const doc = this.editor.document;
 
 		this.value = doc.selection.getAttribute( 'linkHref' );
-		this.isEnabled = doc.schema.checkAttributeInSelection( doc.selection, 'linkHref' );
+		this.isEnabled = this._checkEnabled();
 	}
 
 	/**
@@ -94,4 +94,22 @@ export default class LinkCommand extends Command {
 			}
 		} );
 	}
+
+	/**
+	 * Checks whether the command can be enabled in the current context.
+	 *
+	 * @private
+	 * @returns {Boolean} Whether the command should be enabled.
+	 */
+	_checkEnabled() {
+		const doc = this.editor.document;
+		const selectedElement = doc.selection.getSelectedElement();
+
+		// https://github.com/ckeditor/ckeditor5-link/issues/85
+		if ( selectedElement && selectedElement.is( 'image' ) ) {
+			return false;
+		}
+
+		return doc.schema.checkAttributeInSelection( doc.selection, 'linkHref' );
+	}
 }

tests/integration.js

@@ -0,0 +1,153 @@
+/**
+ * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* global document */
+
+import Link from '../src/link';
+import Image from '@ckeditor/ckeditor5-image/src/image';
+import ImageCaption from '@ckeditor/ckeditor5-image/src/imagecaption';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+
+import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
+import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+
+describe( 'Link', () => {
+	let editor, doc, element, linkCommand, unlinkCommand;
+
+	beforeEach( () => {
+		element = document.createElement( 'div' );
+		document.body.appendChild( element );
+
+		return ClassicTestEditor.create( element, {
+			plugins: [ Paragraph, Image, ImageCaption, Link ]
+		} )
+		.then( newEditor => {
+			editor = newEditor;
+			doc = editor.document;
+
+			linkCommand = editor.commands.get( 'link' );
+			unlinkCommand = editor.commands.get( 'unlink' );
+		} );
+	} );
+
+	afterEach( () => {
+		element.remove();
+
+		return editor.destroy();
+	} );
+
+	describe( 'compatibility with images', () => {
+		it( 'does not link a caption–less image', () => {
+			element = document.createElement( 'div' );
+			document.body.appendChild( element );
+
+			return ClassicTestEditor.create( element, {
+				plugins: [ Paragraph, Image, Link ]
+			} )
+			.then( newEditor => {
+				editor = newEditor;
+				doc = editor.document;
+
+				linkCommand = editor.commands.get( 'link' );
+				unlinkCommand = editor.commands.get( 'unlink' );
+
+				setModelData( doc,
+					'<paragraph>fo[o</paragraph>' +
+					'<image src="foo.png"></image>' +
+					'<paragraph>b]ar</paragraph>'
+				);
+
+				editor.execute( 'link', 'url' );
+
+				expect( getModelData( doc ) ).to.equal(
+					'<paragraph>fo[<$text linkHref="url">o</$text></paragraph>' +
+					'<image src="foo.png"></image>' +
+					'<paragraph><$text linkHref="url">b</$text>]ar</paragraph>'
+				);
+
+				expect( linkCommand.isEnabled ).to.be.true;
+				expect( linkCommand.value ).to.equal( 'url' );
+				expect( unlinkCommand.isEnabled ).to.be.true;
+
+				element.remove();
+
+				return editor.destroy();
+			} );
+		} );
+
+		it( 'links the image caption text if selection contains more than an image', () => {
+			setModelData( doc,
+				'<paragraph>fo[o</paragraph>' +
+				'<image src="foo.png">' +
+					'<caption>abc</caption>' +
+				'</image>' +
+				'<paragraph>baz</paragraph>' +
+				'<image src="bar.png">' +
+					'<caption>abc</caption>' +
+				'</image>' +
+				'<paragraph>b]ar</paragraph>'
+			);
+
+			editor.execute( 'link', 'url' );
+
+			expect( getModelData( doc ) ).to.equal(
+				'<paragraph>fo[<$text linkHref="url">o</$text></paragraph>' +
+				'<image src="foo.png">' +
+					'<caption><$text linkHref="url">abc</$text></caption>' +
+				'</image>' +
+				'<paragraph><$text linkHref="url">baz</$text></paragraph>' +
+				'<image src="bar.png">' +
+					'<caption><$text linkHref="url">abc</$text></caption>' +
+				'</image>' +
+				'<paragraph><$text linkHref="url">b</$text>]ar</paragraph>'
+			);
+
+			expect( linkCommand.isEnabled ).to.be.true;
+			expect( linkCommand.value ).to.equal( 'url' );
+			expect( unlinkCommand.isEnabled ).to.be.true;
+		} );
+
+		it( 'links the image caption text if selection is in the image caption', () => {
+			setModelData( doc,
+				'<paragraph>foo</paragraph>' +
+				'<image src="foo.png">' +
+					'<caption>a[b]c</caption>' +
+				'</image>' +
+				'<paragraph>bar</paragraph>'
+			);
+
+			editor.execute( 'link', 'url' );
+
+			expect( getModelData( doc ) ).to.equal(
+				'<paragraph>foo</paragraph>' +
+				'<image src="foo.png">' +
+					'<caption>a[<$text linkHref="url">b</$text>]c</caption>' +
+				'</image>' +
+				'<paragraph>bar</paragraph>'
+			);
+
+			expect( linkCommand.isEnabled ).to.be.true;
+			expect( linkCommand.value ).to.equal( 'url' );
+			expect( unlinkCommand.isEnabled ).to.be.true;
+		} );
+
+		// https://github.com/ckeditor/ckeditor5-link/issues/85
+		it( 'is disabled when only the image is the only selected element', () => {
+			setModelData( doc, '[<image src="foo.png"></image>]' );
+			expect( linkCommand.isEnabled ).to.be.false;
+
+			setModelData( doc, '[<image src="foo.png"><caption>abc</caption></image>]' );
+			expect( linkCommand.isEnabled ).to.be.false;
+
+			setModelData( doc,
+				'[<image src="foo.png">' +
+					'<caption><$text linkHref="url">abc</$text></caption>' +
+				'</image>]'
+			);
+			expect( linkCommand.isEnabled ).to.be.false;
+			expect( unlinkCommand.isEnabled ).to.be.false;
+		} );
+	} );
+} );