Merge pull request #11531 from ckeditor/ck/11530-image-src-consumption

Fix (image): The image upcast converter should consume the src attribute. Closes #11530.

packages/ckeditor5-image/src/image/utils.js

@@ -54,18 +54,20 @@ export function createBlockImageViewElement( writer ) {
  * @returns {module:engine/view/matcher~MatcherPattern}
  */
 export function getImgViewElementMatcher( editor, matchImageType ) {
-	if ( editor.plugins.has( 'ImageInlineEditing' ) !== editor.plugins.has( 'ImageBlockEditing' ) ) {
-		return { name: 'img' };
-	}
-
 	const imageUtils = editor.plugins.get( 'ImageUtils' );
+	const areBothImagePluginsLoaded = editor.plugins.has( 'ImageInlineEditing' ) && editor.plugins.has( 'ImageBlockEditing' );
 
 	return element => {
-		// Check if view element is an `img`.
+		// Check if the matched view element is an <img>.
 		if ( !imageUtils.isInlineImageView( element ) ) {
 			return null;
 		}
 
+		// If just one of the plugins is loaded (block or inline), it will match all kinds of images.
+		if ( !areBothImagePluginsLoaded ) {
+			return getPositiveMatchPattern( element );
+		}
+
 		// The <img> can be standalone, wrapped in <figure>...</figure> (ImageBlock plugin) or
 		// wrapped in <figure><a>...</a></figure> (LinkImage plugin).
 		const imageType = element.findAncestor( imageUtils.isBlockImageView ) ? 'imageBlock' : 'imageInline';
@@ -74,8 +76,21 @@ export function getImgViewElementMatcher( editor, matchImageType ) {
 			return null;
 		}
 
-		return { name: true };
+		return getPositiveMatchPattern( element );
 	};
+
+	function getPositiveMatchPattern( element ) {
+		const pattern = {
+			name: true
+		};
+
+		// This will trigger src consumption (See https://github.com/ckeditor/ckeditor5/issues/11530).
+		if ( element.hasAttribute( 'src' ) ) {
+			pattern.attributes = [ 'src' ];
+		}
+
+		return pattern;
+	}
 }
 
 /**

packages/ckeditor5-image/tests/image/imageblockediting.js

@@ -290,6 +290,14 @@ describe( 'ImageBlockEditing', () => {
 					.to.equal( '' );
 			} );
 
+			it( 'should consume the src attribute on <img>', () => {
+				editor.data.upcastDispatcher.on( 'element:img', ( evt, data, conversionApi ) => {
+					expect( conversionApi.consumable.test( data.viewItem, { attributes: 'src' } ) ).to.be.false;
+				}, { priority: 'low' } );
+
+				editor.setData( '<figure class="image"><img src="/assets/sample.png" alt="alt text" /></figure>' );
+			} );
+
 			it( 'should dispatch conversion for nested elements', () => {
 				const conversionSpy = sinon.spy();
 				editor.data.upcastDispatcher.on( 'element:figcaption', conversionSpy );

packages/ckeditor5-image/tests/image/imageinlineediting.js

@@ -278,6 +278,14 @@ describe( 'ImageInlineEditing', () => {
 					.to.equal( '<paragraph></paragraph>' );
 			} );
 
+			it( 'should consume the src attribute on <img>', () => {
+				editor.data.upcastDispatcher.on( 'element:img', ( evt, data, conversionApi ) => {
+					expect( conversionApi.consumable.test( data.viewItem, { attributes: 'src' } ) ).to.be.false;
+				}, { priority: 'low' } );
+
+				editor.setData( '<p><img src="/assets/sample.png" alt="alt text" /></p>' );
+			} );
+
 			it( 'should dispatch conversion for nested elements', () => {
 				const conversionSpy = sinon.spy();
 				editor.data.upcastDispatcher.on( 'element:img', conversionSpy );

packages/ckeditor5-image/tests/image/utils.js

@@ -128,42 +128,75 @@ describe( 'image utils', () => {
 	} );
 
 	describe( 'getImgViewElementMatcher()', () => {
-		let editor;
+		describe( 'when one of the image editing plugins is not loaded', () => {
+			let editor;
 
-		beforeEach( async () => {
-			editor = await VirtualTestEditor.create( {
-				plugins: [ ImageUtils, ImageEditing ]
-			} );
+			beforeEach( async () => {
+				editor = await VirtualTestEditor.create( {
+					plugins: [ ImageUtils, ImageEditing ]
+				} );
 
-			imageUtils = editor.plugins.get( 'ImageUtils' );
-		} );
+				imageUtils = editor.plugins.get( 'ImageUtils' );
 
-		afterEach( async () => {
-			editor.destroy();
-		} );
+				writer = new UpcastWriter( editor.editing.view.document );
+			} );
 
-		describe( 'when one of the image editing plugins is not loaded', () => {
-			const returnValue = {
-				name: 'img'
-			};
+			afterEach( async () => {
+				editor.destroy();
+			} );
 
 			it( 'should return a matcher pattern for an img element if ImageBlockEditing plugin is not loaded', () => {
 				sinon.stub( editor.plugins, 'has' ).callsFake( pluginName => pluginName !== 'ImageBlockEditing' );
 
-				expect( getImgViewElementMatcher( editor, 'imageBlock' ) ).to.eql( returnValue );
-				expect( getImgViewElementMatcher( editor, 'imageInline' ) ).to.eql( returnValue );
+				element = writer.createElement( 'img', { src: 'sample.jpg' } );
+				writer.appendChild( element, writer.createElement( 'figure', { class: 'image' } ) );
+
+				expect( getImgViewElementMatcher( editor, 'imageBlock' )( element ) ).to.deep.equal( {
+					name: true,
+					attributes: [ 'src' ]
+				} );
+
+				expect( getImgViewElementMatcher( editor, 'imageInline' )( element ) ).to.deep.equal( {
+					name: true,
+					attributes: [ 'src' ]
+				} );
+			} );
+
+			it( 'should return a matcher pattern for an img element if ImageInlineEditing plugin is not loaded', () => {
+				sinon.stub( editor.plugins, 'has' ).callsFake( pluginName => pluginName !== 'ImageInlineEditing' );
+
+				element = writer.createElement( 'img', { src: 'sample.jpg' } );
+				writer.appendChild( element, writer.createElement( 'figure', { class: 'image' } ) );
+
+				expect( getImgViewElementMatcher( editor, 'imageBlock' )( element ) ).to.deep.equal( {
+					name: true,
+					attributes: [ 'src' ]
+				} );
+
+				expect( getImgViewElementMatcher( editor, 'imageInline' )( element ) ).to.deep.equal( {
+					name: true,
+					attributes: [ 'src' ]
+				} );
 			} );
 
-			it( 'should return a matcher patter for an img element if ImageInlineEditing plugin is not loaded', () => {
+			it( 'should not include "src" in the matcher pattern if the image has no "src"', () => {
 				sinon.stub( editor.plugins, 'has' ).callsFake( pluginName => pluginName !== 'ImageInlineEditing' );
 
-				expect( getImgViewElementMatcher( editor, 'imageBlock', editor ) ).to.eql( returnValue );
-				expect( getImgViewElementMatcher( editor, 'imageInline' ) ).to.eql( returnValue );
+				element = writer.createElement( 'img' );
+				writer.appendChild( element, writer.createElement( 'figure', { class: 'image' } ) );
+
+				expect( getImgViewElementMatcher( editor, 'imageBlock' )( element ) ).to.deep.equal( {
+					name: true
+				} );
+
+				expect( getImgViewElementMatcher( editor, 'imageInline' )( element ) ).to.deep.equal( {
+					name: true
+				} );
 			} );
 		} );
 
 		describe( 'when both image editing plugins are loaded', () => {
-			let matcherPattern, editorElement;
+			let editor, matcherPattern, editorElement;
 
 			beforeEach( async () => {
 				editorElement = document.createElement( 'div' );
@@ -184,7 +217,7 @@ describe( 'image utils', () => {
 			} );
 
 			describe( 'the returned matcherPattern function', () => {
-				describe( 'for the "image" type requested', () => {
+				describe( 'for the "imageBlock" type requested', () => {
 					beforeEach( () => {
 						matcherPattern = getImgViewElementMatcher( editor, 'imageBlock' );
 					} );
@@ -225,6 +258,16 @@ describe( 'image utils', () => {
 						element = writer.createElement( 'img', { src: 'sample.jpg' } );
 						writer.appendChild( element, writer.createElement( 'figure', { class: 'image' } ) );
 
+						expect( matcherPattern( element ) ).to.deep.equal( {
+							name: true,
+							attributes: [ 'src' ]
+						} );
+					} );
+
+					it( 'should not include "src" in the matcher pattern if the image has no "src"', () => {
+						element = writer.createElement( 'img' );
+						writer.appendChild( element, writer.createElement( 'figure', { class: 'image' } ) );
+
 						expect( matcherPattern( element ) ).to.deep.equal( {
 							name: true
 						} );
@@ -261,7 +304,8 @@ describe( 'image utils', () => {
 						element = writer.createElement( 'img', { src: 'sample.jpg' } );
 
 						expect( matcherPattern( element ) ).to.deep.equal( {
-							name: true
+							name: true,
+							attributes: [ 'src' ]
 						} );
 					} );
 
@@ -273,6 +317,15 @@ describe( 'image utils', () => {
 						);
 
 						expect( matcherPattern( fragment.selection.getSelectedElement() ) ).to.deep.equal( {
+							name: true,
+							attributes: [ 'src' ]
+						} );
+					} );
+
+					it( 'should not include "src" in the matcher pattern if the image has no "src"', () => {
+						element = writer.createElement( 'img' );
+
+						expect( matcherPattern( element ) ).to.deep.equal( {
 							name: true
 						} );
 					} );