Merge pull request #8739 from ckeditor/i/8401

Fix (link): Removing a link from an image no longer throws an error when link decorators are also present. Closes #8401.

packages/ckeditor5-link/src/linkcommand.js

@@ -187,7 +187,7 @@ export default class LinkCommand extends Command {
 					writer.setSelection( writer.createPositionAfter( linkRange.end.nodeBefore ) );
 				}
 				// If not then insert text node with `linkHref` attribute in place of caret.
-				// However, since selection in collapsed, attribute value will be used as data for text node.
+				// However, since selection is collapsed, attribute value will be used as data for text node.
 				// So, if `href` is empty, do not create text node.
 				else if ( href !== '' ) {
 					const attributes = toMap( selection.getAttributes() );

packages/ckeditor5-link/src/linkimageediting.js

@@ -214,6 +214,13 @@ function downcastImageLinkManualDecorator( manualDecorators, decorator ) {
 			const viewFigure = conversionApi.mapper.toViewElement( data.item );
 			const linkInImage = Array.from( viewFigure.getChildren() ).find( child => child.name === 'a' );
 
+			// The <a> element was removed by the time this converter is executed.
+			// It may happen when the base `linkHref` and decorator attributes are removed
+			// at the same time (see #8401).
+			if ( !linkInImage ) {
+				return;
+			}
+
 			for ( const [ key, val ] of toMap( attributes ) ) {
 				conversionApi.writer.setAttribute( key, val, linkInImage );
 			}

packages/ckeditor5-link/tests/linkimageediting.js

@@ -826,6 +826,81 @@ describe( 'LinkImageEditing', () => {
 					'</p>'
 				);
 			} );
+
+			// See #8401.
+			it( 'should downcast without error if the image already has no link', () => {
+				setModelData( model,
+					'[<image alt="bar" src="sample.jpg"></image>]'
+				);
+
+				editor.execute( 'link', 'https://cksource.com', {
+					linkIsDownloadable: true,
+					linkIsExternal: true,
+					linkIsGallery: true
+				} );
+
+				// Attributes will be removed along with the link, but the downcast will be fired.
+				// The lack of link should not affect the downcasting.
+				expect( () => {
+					editor.execute( 'unlink', 'https://cksource.com', {
+						linkIsDownloadable: true,
+						linkIsExternal: true,
+						linkIsGallery: true
+					} );
+				} ).to.not.throw();
+
+				expect( editor.getData() ).to.equal(
+					'<figure class="image">' +
+							'<img src="sample.jpg" alt="bar">' +
+						'</figure>'
+				);
+			} );
+
+			// See #8401.
+			describe( 'order of model updates', () => {
+				it( 'should not affect converters - base link attributes first', () => {
+					setModelData( model,
+						'[<image src="https://cksource.com"></image>]'
+					);
+
+					model.change( writer => {
+						const ranges = model.schema.getValidRanges( model.document.selection.getRanges(), 'linkIsDownloadable' );
+
+						for ( const range of ranges ) {
+							// The `linkHref` should be processed first - this is the default order of `LinkCommand`.
+							writer.setAttribute( 'linkHref', 'url', range );
+							writer.setAttribute( 'linkIsDownloadable', true, range );
+						}
+					} );
+
+					expect( getModelData( model, { withoutSelection: true } ) ).to.equal(
+						'<image linkHref="url" linkIsDownloadable="true" src="https://cksource.com"></image>'
+					);
+				} );
+
+				it( 'should not affect converters - decorators first', () => {
+					setModelData( model,
+						'[<image src="https://cksource.com"></image>]'
+					);
+
+					model.change( writer => {
+						const ranges = model.schema.getValidRanges( model.document.selection.getRanges(), 'linkIsDownloadable' );
+
+						for ( const range of ranges ) {
+							// Here we force attributes to be set on a model in a different order
+							// to force unusual order of downcast converters down the line.
+							// Normally, the `linkHref` gets processed first, as it is just the first property assigned
+							// to the model by `LinkCommand`.
+							writer.setAttribute( 'linkIsDownloadable', true, range );
+							writer.setAttribute( 'linkHref', 'url', range );
+						}
+					} );
+
+					expect( getModelData( model, { withoutSelection: true } ) ).to.equal(
+						'<image linkHref="url" linkIsDownloadable="true" src="https://cksource.com"></image>'
+					);
+				} );
+			} );
 		} );
 	} );
 } );