Merge pull request #76 from ckeditor/t/73

Fix: It should not be possible to apply a heading to an image. Closes #73.

package.json

@@ -15,6 +15,7 @@
     "@ckeditor/ckeditor5-dev-lint": "^3.0.0",
     "@ckeditor/ckeditor5-enter": "^0.9.1",
     "@ckeditor/ckeditor5-editor-classic": "^0.7.3",
+    "@ckeditor/ckeditor5-image": "^0.6.0",
     "@ckeditor/ckeditor5-typing": "^0.9.1",
     "@ckeditor/ckeditor5-undo": "^0.8.1",
     "eslint-config-ckeditor5": "^1.0.0",

src/headingcommand.js

@@ -88,8 +88,12 @@ export default class HeadingCommand extends Command {
 
 		document.enqueueChanges( () => {
 			const batch = options.batch || document.batch();
+			const blocks = Array.from( document.selection.getSelectedBlocks() )
+				.filter( block => {
+					return checkCanBecomeHeading( block, this.modelElement, document.schema );
+				} );
 
-			for ( const block of document.selection.getSelectedBlocks() ) {
+			for ( const block of blocks ) {
 				// When removing applied option.
 				if ( shouldRemove ) {
 					if ( block.is( this.modelElement ) ) {
@@ -123,15 +127,27 @@ export default class HeadingCommand extends Command {
 	 * @inheritDoc
 	 */
 	_checkEnabled() {
-		const block = first( this.editor.document.selection.getSelectedBlocks() );
+		const document = this.editor.document;
+		const block = first( document.selection.getSelectedBlocks() );
 
-		return !!block && this.editor.document.schema.check( {
-			name: this.modelElement,
-			inside: Position.createBefore( block )
-		} );
+		return !!block && checkCanBecomeHeading( block, this.modelElement, document.schema );
 	}
 }
 
+// Checks whether the given block can be replaced by a specific heading.
+//
+// @private
+// @param {module:engine/model/element~Element} block A block to be tested.
+// @param {module:heading/headingcommand~HeadingCommand#modelElement} heading Command element name in the model.
+// @param {module:engine/model/schema~Schema} schema The schema of the document.
+// @returns {Boolean}
+function checkCanBecomeHeading( block, heading, schema ) {
+	return schema.check( {
+		name: heading,
+		inside: Position.createBefore( block )
+	} );
+}
+
 /**
  * Heading option descriptor.
  *

tests/headingcommand.js

@@ -118,6 +118,31 @@ describe( 'HeadingCommand', () => {
 			expect( command.value ).to.be.true;
 		} );
 
+		// https://github.com/ckeditor/ckeditor5-heading/issues/73
+		it( 'should not rename blocks which cannot become headings', () => {
+			document.schema.registerItem( 'restricted' );
+			document.schema.allow( { name: 'restricted', inside: '$root' } );
+			document.schema.disallow( { name: 'heading1', inside: 'restricted' } );
+
+			document.schema.registerItem( 'fooBlock', '$block' );
+			document.schema.allow( { name: 'fooBlock', inside: 'restricted' } );
+
+			setData(
+				document,
+				'<paragraph>a[bc</paragraph>' +
+				'<restricted><fooBlock></fooBlock></restricted>' +
+				'<paragraph>de]f</paragraph>'
+			);
+
+			commands.heading1._doExecute();
+
+			expect( getData( document ) ).to.equal(
+				'<heading1>a[bc</heading1>' +
+				'<restricted><fooBlock></fooBlock></restricted>' +
+				'<heading1>de]f</heading1>'
+			);
+		} );
+
 		describe( 'custom options', () => {
 			it( 'should use provided batch', () => {
 				const batch = editor.document.batch();

tests/headingengine.js

@@ -8,15 +8,14 @@ import HeadingCommand from '../src/headingcommand';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import ParagraphCommand from '@ckeditor/ckeditor5-paragraph/src/paragraphcommand';
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
-import Enter from '@ckeditor/ckeditor5-enter/src/enter';
 import { getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
 describe( 'HeadingEngine', () => {
 	let editor, document;
 
 	beforeEach( () => {
 		return VirtualTestEditor.create( {
-			plugins: [ Enter, HeadingEngine ]
+			plugins: [ HeadingEngine ]
 		} )
 		.then( newEditor => {
 			editor = newEditor;
@@ -75,25 +74,6 @@ describe( 'HeadingEngine', () => {
 		expect( editor.getData() ).to.equal( '<h4>foobar</h4>' );
 	} );
 
-	it( 'should make enter command insert a defaultOption block if selection ended at the end of heading block', () => {
-		editor.setData( '<h2>foobar</h2>' );
-		document.selection.collapse( document.getRoot().getChild( 0 ), 'end' );
-
-		editor.execute( 'enter' );
-
-		expect( getData( document ) ).to.equal( '<heading1>foobar</heading1><paragraph>[]</paragraph>' );
-	} );
-
-	it( 'should not alter enter command if selection not ended at the end of heading block', () => {
-		// This test is to fill code coverage.
-		editor.setData( '<h2>foobar</h2>' );
-		document.selection.collapse( document.getRoot().getChild( 0 ), 3 );
-
-		editor.execute( 'enter' );
-
-		expect( getData( document ) ).to.equal( '<heading1>foo</heading1><heading1>[]bar</heading1>' );
-	} );
-
 	it( 'should not blow up if there\'s no enter command in the editor', () => {
 		return VirtualTestEditor.create( {
 			plugins: [ HeadingEngine ]
@@ -120,7 +100,7 @@ describe( 'HeadingEngine', () => {
 				];
 
 				return VirtualTestEditor.create( {
-					plugins: [ Enter, HeadingEngine ],
+					plugins: [ HeadingEngine ],
 					heading: {
 						options
 					}

tests/integration.js

@@ -0,0 +1,83 @@
+/**
+ * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* global document */
+
+import Heading from '../src/heading.js';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+
+import Enter from '@ckeditor/ckeditor5-enter/src/enter';
+import Image from '@ckeditor/ckeditor5-image/src/image';
+import ImageCaption from '@ckeditor/ckeditor5-image/src/imagecaption';
+
+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( 'Heading integration', () => {
+	let editor, doc, element;
+
+	beforeEach( () => {
+		element = document.createElement( 'div' );
+		document.body.appendChild( element );
+
+		return ClassicTestEditor.create( element, {
+			plugins: [ Paragraph, Heading, Enter, Image, ImageCaption ]
+		} )
+		.then( newEditor => {
+			editor = newEditor;
+			doc = editor.document;
+		} );
+	} );
+
+	afterEach( () => {
+		element.remove();
+
+		return editor.destroy();
+	} );
+
+	describe( 'with the enter key', () => {
+		it( 'should make the "enter" command insert a default heading block if the selection ended at the end of a heading block', () => {
+			editor.setData( '<h2>foobar</h2>' );
+			doc.selection.collapse( doc.getRoot().getChild( 0 ), 'end' );
+
+			editor.execute( 'enter' );
+
+			expect( getModelData( doc ) ).to.equal( '<heading1>foobar</heading1><paragraph>[]</paragraph>' );
+		} );
+
+		it( 'should not alter the "enter" command if selection not ended at the end of a heading block', () => {
+			// This test is to fill code coverage.
+			editor.setData( '<h2>foobar</h2>' );
+			doc.selection.collapse( doc.getRoot().getChild( 0 ), 3 );
+
+			editor.execute( 'enter' );
+
+			expect( getModelData( doc ) ).to.equal( '<heading1>foo</heading1><heading1>[]bar</heading1>' );
+		} );
+	} );
+
+	describe( 'with the image feature', () => {
+		// https://github.com/ckeditor/ckeditor5-heading/issues/73
+		it( 'should not destroy the image when a selection converted to a heading', () => {
+			setModelData( editor.document,
+				'<paragraph>fo[o</paragraph>' +
+				'<image src="foo.png">' +
+					'<caption>xxx</caption>' +
+				'</image>' +
+				'<paragraph>b]ar</paragraph>'
+			);
+
+			editor.execute( 'heading1' );
+
+			expect( getModelData( doc ) ).to.equal(
+				'<heading1>fo[o</heading1>' +
+				'<image src="foo.png">' +
+					'<caption>xxx</caption>' +
+				'</image>' +
+				'<heading1>b]ar</heading1>'
+			);
+		} );
+	} );
+} );