Explorar el Código

Fixed a bug when blockQuote would be applied in context in which it's not allowed.

Piotrek Koszuliński hace 8 años
padre
commit
73ef970786

+ 1 - 0
packages/ckeditor5-block-quote/package.json

@@ -15,6 +15,7 @@
     "@ckeditor/ckeditor5-dev-lint": "^2.0.2",
     "@ckeditor/ckeditor5-editor-classic": "^0.7.2",
     "@ckeditor/ckeditor5-enter": "^0.9.0",
+    "@ckeditor/ckeditor5-image": "^0.5.0",
     "@ckeditor/ckeditor5-list": "^0.6.0",
     "@ckeditor/ckeditor5-paragraph": "^0.7.0",
     "@ckeditor/ckeditor5-presets": "^0.2.0",

+ 24 - 13
packages/ckeditor5-block-quote/src/blockquotecommand.js

@@ -64,6 +64,7 @@ export default class BlockQuoteCommand extends Command {
 	 */
 	_doExecute( options = {} ) {
 		const doc = this.editor.document;
+		const schema = doc.schema;
 		const batch = options.batch || doc.batch();
 		const blocks = Array.from( doc.selection.getSelectedBlocks() );
 
@@ -71,7 +72,13 @@ export default class BlockQuoteCommand extends Command {
 			if ( this.value ) {
 				this._removeQuote( batch, blocks.filter( findQuote ) );
 			} else {
-				this._applyQuote( batch, blocks );
+				const blocksToQuote = blocks.filter( block => {
+					// Already quoted blocks needs to be considered while quoting too
+					// in order to reuse their <bQ> elements.
+					return findQuote( block ) || checkCanBeQuoted( schema, block );
+				} );
+
+				this._applyQuote( batch, blocksToQuote );
 			}
 		} );
 	}
@@ -93,18 +100,7 @@ export default class BlockQuoteCommand extends Command {
 			return false;
 		}
 
-		const isMQAllowed = schema.check( {
-			name: 'blockQuote',
-			inside: Position.createBefore( firstBlock )
-		} );
-		const isBlockAllowed = schema.check( {
-			name: firstBlock.name,
-			attributes: Array.from( firstBlock.getAttributeKeys() ),
-			inside: 'blockQuote'
-		} );
-
-		// Whether <bQ> can wrap the block.
-		return isMQAllowed && isBlockAllowed;
+		return checkCanBeQuoted( schema, firstBlock );
 	}
 
 	/**
@@ -222,3 +218,18 @@ function getRangesOfBlockGroups( blocks ) {
 
 	return ranges;
 }
+
+// Checks whether <bQ> can wrap the block.
+function checkCanBeQuoted( schema, block ) {
+	const isBQAllowed = schema.check( {
+		name: 'blockQuote',
+		inside: Position.createBefore( block )
+	} );
+	const isBlockAllowedInBQ = schema.check( {
+		name: block.name,
+		attributes: Array.from( block.getAttributeKeys() ),
+		inside: 'blockQuote'
+	} );
+
+	return isBQAllowed && isBlockAllowedInBQ;
+}

+ 63 - 0
packages/ckeditor5-block-quote/tests/blockquotecommand.js

@@ -367,6 +367,69 @@ describe( 'BlockQuoteCommand', () => {
 					'<p>y</p>'
 				);
 			} );
+
+			it( 'should not wrap a block which can not be in a quote', () => {
+				// blockQuote is allowed in root, but fooBlock can not be inside blockQuote.
+				doc.schema.registerItem( 'fooBlock', '$block' );
+				doc.schema.disallow( { name: 'fooBlock', inside: 'blockQuote' } );
+				buildModelConverter().for( editor.editing.modelToView )
+					.fromElement( 'fooBlock' )
+					.toElement( 'fooblock' );
+
+				setModelData(
+					doc,
+					'<paragraph>a[bc</paragraph>' +
+					'<fooBlock>xx</fooBlock>' +
+					'<paragraph>de]f</paragraph>'
+				);
+
+				editor.execute( 'blockQuote' );
+
+				expect( getModelData( doc ) ).to.equal(
+					'<blockQuote>' +
+						'<paragraph>a[bc</paragraph>' +
+					'</blockQuote>' +
+					'<fooBlock>xx</fooBlock>' +
+					'<blockQuote>' +
+						'<paragraph>de]f</paragraph>' +
+					'</blockQuote>'
+				);
+			} );
+
+			it( 'should not wrap a block which parent does not allow quote inside itself', () => {
+				// blockQuote is not be allowed in fooWrapper, but fooBlock can be inside blockQuote.
+				doc.schema.registerItem( 'fooWrapper' );
+				doc.schema.registerItem( 'fooBlock', '$block' );
+
+				doc.schema.allow( { name: 'fooWrapper', inside: '$root' } );
+				doc.schema.allow( { name: 'fooBlock', inside: 'fooWrapper' } );
+
+				buildModelConverter().for( editor.editing.modelToView )
+					.fromElement( 'fooWrapper' )
+					.toElement( 'foowrapper' );
+				buildModelConverter().for( editor.editing.modelToView )
+					.fromElement( 'fooBlock' )
+					.toElement( 'fooblock' );
+
+				setModelData(
+					doc,
+					'<paragraph>a[bc</paragraph>' +
+					'<fooWrapper><fooBlock>xx</fooBlock></fooWrapper>' +
+					'<paragraph>de]f</paragraph>'
+				);
+
+				editor.execute( 'blockQuote' );
+
+				expect( getModelData( doc ) ).to.equal(
+					'<blockQuote>' +
+						'<paragraph>a[bc</paragraph>' +
+					'</blockQuote>' +
+					'<fooWrapper><fooBlock>xx</fooBlock></fooWrapper>' +
+					'<blockQuote>' +
+						'<paragraph>de]f</paragraph>' +
+					'</blockQuote>'
+				);
+			} );
 		} );
 
 		describe( 'removing quote', () => {

+ 73 - 1
packages/ckeditor5-block-quote/tests/integration.js

@@ -7,6 +7,8 @@
 
 import BlockQuote from '../src/blockquote';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+import Image from '@ckeditor/ckeditor5-image/src/image';
+import ImageCaption from '@ckeditor/ckeditor5-image/src/imagecaption';
 import List from '@ckeditor/ckeditor5-list/src/list';
 import Enter from '@ckeditor/ckeditor5-enter/src/enter';
 import Delete from '@ckeditor/ckeditor5-typing/src/delete';
@@ -22,7 +24,7 @@ describe( 'BlockQuote', () => {
 		document.body.appendChild( element );
 
 		return ClassicTestEditor.create( element, {
-			plugins: [ BlockQuote, Paragraph, List, Enter, Delete ]
+			plugins: [ BlockQuote, Paragraph, Image, ImageCaption, List, Enter, Delete ]
 		} )
 		.then( newEditor => {
 			editor = newEditor;
@@ -289,4 +291,74 @@ describe( 'BlockQuote', () => {
 			);
 		} );
 	} );
+
+	describe( 'compatibility with images', () => {
+		it( 'does not quote a simple image', () => {
+			const element = document.createElement( 'div' );
+			document.body.appendChild( element );
+
+			// We can't load ImageCaption in this test because it adds <caption> to all images automatically.
+			return ClassicTestEditor.create( element, {
+					plugins: [ BlockQuote, Paragraph, Image ]
+				} )
+				.then( ( editor ) => {
+					setModelData( editor.document,
+						'<paragraph>fo[o</paragraph>' +
+						'<image src="foo.png"></image>' +
+						'<paragraph>b]ar</paragraph>'
+					);
+
+					editor.execute( 'blockQuote' );
+
+					expect( getModelData( editor.document ) ).to.equal(
+						'<blockQuote><paragraph>fo[o</paragraph></blockQuote>' +
+						'<image src="foo.png"></image>' +
+						'<blockQuote><paragraph>b]ar</paragraph></blockQuote>'
+					);
+
+					element.remove();
+					editor.destroy();
+				} );
+		} );
+
+		it( 'does not quote an image with caption', () => {
+			setModelData( doc,
+				'<paragraph>fo[o</paragraph>' +
+				'<image src="foo.png">' +
+					'<caption>xxx</caption>' +
+				'</image>' +
+				'<paragraph>b]ar</paragraph>'
+			);
+
+			editor.execute( 'blockQuote' );
+
+			expect( getModelData( doc ) ).to.equal(
+				'<blockQuote><paragraph>fo[o</paragraph></blockQuote>' +
+				'<image src="foo.png">' +
+					'<caption>xxx</caption>' +
+				'</image>' +
+				'<blockQuote><paragraph>b]ar</paragraph></blockQuote>'
+			);
+		} );
+
+		it( 'does not add an image to existing quote', () => {
+			setModelData( doc,
+				'<paragraph>fo[o</paragraph>' +
+				'<image src="foo.png">' +
+					'<caption>xxx</caption>' +
+				'</image>' +
+				'<blockQuote><paragraph>b]ar</paragraph></blockQuote>'
+			);
+
+			editor.execute( 'blockQuote' );
+
+			expect( getModelData( doc ) ).to.equal(
+				'<blockQuote><paragraph>fo[o</paragraph></blockQuote>' +
+				'<image src="foo.png">' +
+					'<caption>xxx</caption>' +
+				'</image>' +
+				'<blockQuote><paragraph>b]ar</paragraph></blockQuote>'
+			);
+		} );
+	} );
 } );