Merge pull request #14556 from ckeditor/ck/14531-keep-image-sizes-aft…

…er-image-upload Task (image): The image width should not be changed after uploading if the width or height was previously set. Closes #14531.
ckeditor · Jul 12, 2023 · 5f60808 · 5f60808
2 parents aec7511 + c2be094
commit 5f60808
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 9 deletions.
diff --git a/packages/ckeditor5-engine/src/index.ts b/packages/ckeditor5-engine/src/index.ts
@@ -92,7 +92,7 @@ export type { Marker } from './model/markercollection';
 export type { default as Batch } from './model/batch';
 export type { default as Differ, DiffItem, DiffItemAttribute, DiffItemInsert, DiffItemRemove } from './model/differ';
 export type { default as Item } from './model/item';
-export type { default as Node } from './model/node';
+export type { default as Node, NodeAttributes } from './model/node';
 export type { default as RootElement } from './model/rootelement';
 export type {
 	default as Schema,

diff --git a/packages/ckeditor5-image/src/imageupload/imageuploadediting.ts b/packages/ckeditor5-image/src/imageupload/imageuploadediting.ts
@@ -9,7 +9,15 @@
 
 import { Plugin, type Editor } from 'ckeditor5/src/core';
 
-import { UpcastWriter, type Element, type Item, type Writer, type DataTransfer, type ViewElement } from 'ckeditor5/src/engine';
+import {
+	UpcastWriter,
+	type Element,
+	type Item,
+	type Writer,
+	type DataTransfer,
+	type ViewElement,
+	type NodeAttributes
+} from 'ckeditor5/src/engine';
 
 import { Notification } from 'ckeditor5/src/ui';
 import { ClipboardPipeline, type ViewDocumentClipboardInputEvent } from 'ckeditor5/src/clipboard';
@@ -398,10 +406,15 @@ export default class ImageUploadEditing extends Plugin {
 			.join( ', ' );
 
 		if ( srcsetAttribute != '' ) {
-			writer.setAttributes( {
-				srcset: srcsetAttribute,
-				width: maxWidth
-			}, image );
+			const attributes: NodeAttributes = {
+				srcset: srcsetAttribute
+			};
+
+			if ( !image.hasAttribute( 'width' ) && !image.hasAttribute( 'height' ) ) {
+				attributes.width = maxWidth;
+			}
+
+			writer.setAttributes( attributes, image );
 		}
 	}
 }

diff --git a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js
@@ -507,10 +507,62 @@ describe( 'ImageUploadEditing', () => {
 		expect( getModelData( model ) ).to.equal(
 			'<paragraph>[<imageInline height="96" src="/assets/sample.png" width="96"></imageInline>]foo bar</paragraph>'
 		);
+	} );
 
-		function timeout( ms ) {
-			return new Promise( res => setTimeout( res, ms ) );
-		}
+	it( 'should not modify image width if width was set before server response', async () => {
+		setModelData( model, '<paragraph>[]foo</paragraph>' );
+
+		const clipboardHtml = `<img width="50" src=${ base64Sample } />`;
+		const dataTransfer = mockDataTransfer( clipboardHtml );
+
+		const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) );
+		const targetViewRange = editor.editing.mapper.toViewRange( targetRange );
+
+		viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );
+
+		await new Promise( res => {
+			model.document.once( 'change', res );
+			loader.file.then( () => nativeReaderMock.mockSuccess( base64Sample ) );
+		} );
+
+		await new Promise( res => {
+			model.document.once( 'change', res, { priority: 'lowest' } );
+			loader.file.then( () => adapterMocks[ 0 ].mockSuccess( { default: '/assets/sample.png', 800: 'image-800.png' } ) );
+		} );
+
+		await timeout( 100 );
+
+		expect( getModelData( model ) ).to.equal(
+			'[<imageBlock src="/assets/sample.png" srcset="image-800.png 800w" width="50"></imageBlock>]<paragraph>foo</paragraph>'
+		);
+	} );
+
+	it( 'should not modify image width if height was set before server response', async () => {
+		setModelData( model, '<paragraph>[]foo</paragraph>' );
+
+		const clipboardHtml = `<img height="50" src=${ base64Sample } />`;
+		const dataTransfer = mockDataTransfer( clipboardHtml );
+
+		const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) );
+		const targetViewRange = editor.editing.mapper.toViewRange( targetRange );
+
+		viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );
+
+		await new Promise( res => {
+			model.document.once( 'change', res );
+			loader.file.then( () => nativeReaderMock.mockSuccess( base64Sample ) );
+		} );
+
+		await new Promise( res => {
+			model.document.once( 'change', res, { priority: 'lowest' } );
+			loader.file.then( () => adapterMocks[ 0 ].mockSuccess( { default: '/assets/sample.png', 800: 'image-800.png' } ) );
+		} );
+
+		await timeout( 100 );
+
+		expect( getModelData( model ) ).to.equal(
+			'[<imageBlock height="50" src="/assets/sample.png" srcset="image-800.png 800w"></imageBlock>]<paragraph>foo</paragraph>'
+		);
 	} );
 
 	it( 'should support adapter response with the normalized `urls` property', async () => {
@@ -1509,3 +1561,7 @@ function base64ToBlob( base64Data ) {
 
 	return new Blob( byteArrays, { type } );
 }
+
+function timeout( ms ) {
+	return new Promise( res => setTimeout( res, ms ) );
+}