From fb117a51fce02d67e562b0f9c745970b15b52162 Mon Sep 17 00:00:00 2001 From: Marta Motyczynska Date: Wed, 5 Jul 2023 16:58:03 +0200 Subject: [PATCH 1/3] Do not set width after image upload if the image has width or height set. --- packages/ckeditor5-engine/src/index.ts | 2 +- .../src/imageupload/imageuploadediting.ts | 23 +++++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/ckeditor5-engine/src/index.ts b/packages/ckeditor5-engine/src/index.ts index ae2a3c73452..de1bc11596e 100644 --- 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 index 18f92ae24aa..8628293935b 100644 --- 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 ); } } } From 8cab8c9f98df08197c41a9e723db0f4126e6e4b2 Mon Sep 17 00:00:00 2001 From: Marta Motyczynska Date: Mon, 10 Jul 2023 15:08:06 +0200 Subject: [PATCH 2/3] Tests: do not set image width after image upload if width or height were set previously. --- .../tests/imageupload/imageuploadediting.js | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js index fab20c27cc0..24920821f39 100644 --- a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js +++ b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js @@ -513,6 +513,70 @@ describe( 'ImageUploadEditing', () => { } } ); + it( 'should not modify image width if width was set before server response', async () => { + setModelData( model, '[]foo' ); + + const clipboardHtml = ``; + 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( + '[]foo' + ); + + function timeout( ms ) { + return new Promise( res => setTimeout( res, ms ) ); + } + } ); + + it( 'should not modify image width if height was set before server response', async () => { + setModelData( model, '[]foo' ); + + const clipboardHtml = ``; + 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( + '[]foo' + ); + + function timeout( ms ) { + return new Promise( res => setTimeout( res, ms ) ); + } + } ); + it( 'should support adapter response with the normalized `urls` property', async () => { const file = createNativeFileMock(); setModelData( model, '{}foo bar' ); From ed8a0a1655d3b5149f63e50e7cc4db5e57fef49f Mon Sep 17 00:00:00 2001 From: Marta Motyczynska Date: Tue, 11 Jul 2023 10:09:35 +0200 Subject: [PATCH 3/3] Tests: Refactor. --- .../tests/imageupload/imageuploadediting.js | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js index 24920821f39..ebb1343d6b7 100644 --- a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js +++ b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js @@ -507,10 +507,6 @@ describe( 'ImageUploadEditing', () => { expect( getModelData( model ) ).to.equal( '[]foo bar' ); - - function timeout( ms ) { - return new Promise( res => setTimeout( res, ms ) ); - } } ); it( 'should not modify image width if width was set before server response', async () => { @@ -539,10 +535,6 @@ describe( 'ImageUploadEditing', () => { expect( getModelData( model ) ).to.equal( '[]foo' ); - - function timeout( ms ) { - return new Promise( res => setTimeout( res, ms ) ); - } } ); it( 'should not modify image width if height was set before server response', async () => { @@ -571,10 +563,6 @@ describe( 'ImageUploadEditing', () => { expect( getModelData( model ) ).to.equal( '[]foo' ); - - function timeout( ms ) { - return new Promise( res => setTimeout( res, ms ) ); - } } ); it( 'should support adapter response with the normalized `urls` property', async () => { @@ -1573,3 +1561,7 @@ function base64ToBlob( base64Data ) { return new Blob( byteArrays, { type } ); } + +function timeout( ms ) { + return new Promise( res => setTimeout( res, ms ) ); +}