Merge pull request #15135 from ckeditor/ck/15093-fix-the-url-editing-…

…of-the-responsive-image

Fix (image): Remove outdated image attributes when an image is replaced by a URL. Closes #15093.

packages/ckeditor5-ckbox/src/ckboxediting.ts

@@ -23,12 +23,13 @@ import {
  type ViewElement,
  type Writer
 } from 'ckeditor5/src/engine';
-import { CKEditorError, logError } from 'ckeditor5/src/utils';
+import { CKEditorError, logError, type DecoratedMethodEvent } from 'ckeditor5/src/utils';
 
 import type { CKBoxAssetDefinition } from './ckboxconfig';
 
 import CKBoxCommand from './ckboxcommand';
 import CKBoxUploadAdapter from './ckboxuploadadapter';
+import type { ReplaceImageSourceCommand } from '@ckeditor/ckeditor5-image';
 
 const DEFAULT_CKBOX_THEME_NAME = 'lark';
 
@@ -318,6 +319,18 @@ export default class CKBoxEditing extends Plugin {
  }
  }
  } );
+
+ const replaceImageSourceCommand = editor.commands.get( 'replaceImageSource' );
+
+ if ( replaceImageSourceCommand ) {
+ this.listenTo<DecoratedMethodEvent<ReplaceImageSourceCommand, 'cleanupImage'>>(
+ replaceImageSourceCommand,
+ 'cleanupImage',
+ ( _, [ writer, image ] ) => {
+ writer.removeAttribute( 'ckboxImageId', image );
+ }
+ );
+ }
  }
 
  /**

packages/ckeditor5-ckbox/tests/ckboxediting.js

@@ -34,7 +34,7 @@ import TokenMock from '@ckeditor/ckeditor5-cloud-services/tests/_utils/tokenmock
 const CKBOX_API_URL = 'https://upload.example.com';
 
 describe( 'CKBoxEditing', () => {
- let editor, model, view, originalCKBox;
+ let editor, model, view, originalCKBox, replaceImageSourceCommand;
 
  testUtils.createSinonSandbox();
 
@@ -50,6 +50,7 @@ describe( 'CKBoxEditing', () => {
  }
  } );
 
+ replaceImageSourceCommand = editor.commands.get( 'replaceImageSource' );
  model = editor.model;
  view = editor.editing.view;
  } );
@@ -2056,6 +2057,23 @@ describe( 'CKBoxEditing', () => {
  } );
  } );
  } );
+
+ it( 'should remove ckboxImageId attribute on image replace', () => {
+ const schema = model.schema;
+ schema.extend( 'imageBlock', { allowAttributes: 'ckboxImageId' } );
+
+ setModelData( model, `[<imageBlock
+ ckboxImageId="id"
+ ></imageBlock>]` );
+
+ const element = model.document.selection.getSelectedElement();
+
+ expect( element.getAttribute( 'ckboxImageId' ) ).to.equal( 'id' );
+
+ replaceImageSourceCommand.execute( { source: 'bar/foo.jpg' } );
+
+ expect( element.getAttribute( 'ckboxImageId' ) ).to.be.undefined;
+ } );
 } );
 
 function createTestEditor( config = {} ) {

packages/ckeditor5-ckbox/tests/manual/ckbox.js

@@ -8,6 +8,7 @@
 import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
 import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
 import ImageUpload from '@ckeditor/ckeditor5-image/src/imageupload';
+import ImageInsert from '@ckeditor/ckeditor5-image/src/imageinsert';
 import LinkImageEditing from '@ckeditor/ckeditor5-link/src/linkimageediting';
 import PictureEditing from '@ckeditor/ckeditor5-image/src/pictureediting';
 import CloudServices from '@ckeditor/ckeditor5-cloud-services/src/cloudservices';
@@ -16,14 +17,15 @@ import CKBox from '../../src/ckbox';
 
 ClassicEditor
  .create( document.querySelector( '#editor' ), {
- plugins: [ ArticlePluginSet, PictureEditing, ImageUpload, LinkImageEditing, CloudServices, CKBox ],
+ plugins: [ ArticlePluginSet, PictureEditing, ImageUpload, LinkImageEditing, ImageInsert, CloudServices, CKBox ],
  toolbar: [
  'heading',
  '|',
  'bold',
  'italic',
  'link',
  'insertTable',
+ 'insertImage',
  '|',
  'undo',
  'redo',

packages/ckeditor5-image/src/image/replaceimagesourcecommand.ts

@@ -3,8 +3,9 @@
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
 
-import { Command } from 'ckeditor5/src/core';
+import { Command, type Editor } from 'ckeditor5/src/core';
 import type ImageUtils from '../imageutils';
+import type { Writer, Element } from 'ckeditor5/src/engine';
 
 /**
  * @module image/image/replaceimagesourcecommand
@@ -22,6 +23,12 @@ import type ImageUtils from '../imageutils';
 export default class ReplaceImageSourceCommand extends Command {
  declare public value: string | null;
 
+ constructor( editor: Editor ) {
+ super( editor );
+
+ this.decorate( 'cleanupImage' );
+ }
+
  /**
  * @inheritDoc
  */
@@ -43,10 +50,42 @@ export default class ReplaceImageSourceCommand extends Command {
  */
  public override execute( options: { source: string } ): void {
  const image = this.editor.model.document.selection.getSelectedElement()!;
+ const imageUtils: ImageUtils = this.editor.plugins.get( 'ImageUtils' );
+
  this.editor.model.change( writer => {
  writer.setAttribute( 'src', options.source, image );
- writer.removeAttribute( 'srcset', image );
- writer.removeAttribute( 'sizes', image );
+
+ this.cleanupImage( writer, image );
+
+ imageUtils.setImageNaturalSizeAttributes( image );
  } );
  }
+
+ /**
+ * Cleanup image attributes that are not relevant to the new source.
+ *
+ * Removed attributes are: 'srcset', 'sizes', 'sources', 'width', 'height', 'alt'.
+ *
+ * This method is decorated, to allow custom cleanup logic.
+ * For example, to remove 'myImageId' attribute after 'src' has changed:
+ *
+ * ```ts
+ * replaceImageSourceCommand.on( 'cleanupImage', ( eventInfo, [ writer, image ] ) => {
+ * writer.removeAttribute( 'myImageId', image );
+ * } );
+ * ```
+ */
+ public cleanupImage( writer: Writer, image: Element ): void {
+ writer.removeAttribute( 'srcset', image );
+ writer.removeAttribute( 'sizes', image );
+
+ /**
+ * In case responsive images some attributes should be cleaned up.
+ * Check: https://github.com/ckeditor/ckeditor5/issues/15093
+ */
+ writer.removeAttribute( 'sources', image );
+ writer.removeAttribute( 'width', image );
+ writer.removeAttribute( 'height', image );
+ writer.removeAttribute( 'alt', image );
+ }
 }

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

@@ -3,6 +3,8 @@
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
 
+/* global setTimeout */
+
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
@@ -44,6 +46,59 @@ describe( 'ReplaceImageSourceCommand', () => {
 
  expect( element.getAttribute( 'src' ) ).to.equal( 'bar/foo.jpg' );
  } );
+
+ it( 'should clean up some attributes in responsive image', () => {
+ setModelData( model, `[<imageBlock
+ src="foo/bar.jpg"
+ width="100"
+ height="200"
+ myCustomId="id"
+ alt="Example image"
+ sources="[{srcset:'url', sizes:'100vw, 1920px', type: 'image/webp'}]"
+ ></imageBlock>]` );
+
+ const element = model.document.selection.getSelectedElement();
+
+ expect( element.getAttribute( 'src' ) ).to.equal( 'foo/bar.jpg' );
+ expect( element.getAttribute( 'sources' ) ).to.equal( '[{srcset:\'url\', sizes:\'100vw, 1920px\', type: \'image/webp\'}]' );
+ expect( element.getAttribute( 'width' ) ).to.equal( 100 );
+ expect( element.getAttribute( 'height' ) ).to.equal( 200 );
+ expect( element.getAttribute( 'alt' ) ).to.equal( 'Example image' );
+ expect( element.getAttribute( 'myCustomId' ) ).to.equal( 'id' );
+
+ command.on( 'cleanupImage', ( eventInfo, [ writer, image ] ) => {
+ writer.removeAttribute( 'myCustomId', image );
+ } );
+ command.execute( { source: 'bar/foo.jpg' } );
+
+ expect( element.getAttribute( 'src' ) ).to.equal( 'bar/foo.jpg' );
+ expect( element.getAttribute( 'sources' ) ).to.be.undefined;
+ expect( element.getAttribute( 'width' ) ).to.be.undefined;
+ expect( element.getAttribute( 'height' ) ).to.be.undefined;
+ expect( element.getAttribute( 'alt' ) ).to.be.undefined;
+ expect( element.getAttribute( 'myCustomId' ) ).to.be.undefined;
+ } );
+
+ it( 'should set width and height on replaced image', done => {
+ setModelData( model, `[<imageBlock
+ src="foo/bar.jpg"
+ width="100"
+ height="200"
+ myCustomId="id"
+ alt="Example image"
+ sources="[{srcset:'url', sizes:'100vw, 1920px', type: 'image/webp'}]"
+ ></imageBlock>]` );
+
+ const element = model.document.selection.getSelectedElement();
+
+ command.execute( { source: '/assets/sample.png' } );
+
+ setTimeout( () => {
+ expect( element.getAttribute( 'width' ) ).to.equal( 96 );
+ expect( element.getAttribute( 'height' ) ).to.equal( 96 );
+ done();
+ }, 100 );
+ } );
  } );
 
  describe( 'refresh()', () => {