From 228311c7adc3753ae509f8f53e0cb02f730b1b0c Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Fri, 8 Nov 2024 10:55:02 +0100 Subject: [PATCH] Fix not working drag & drop on images that are in uploading state. --- .../src/imageupload/imageuploadediting.ts | 74 ++++++++- .../tests/imageupload/imageuploadediting.js | 146 ++++++++++++++++++ 2 files changed, 219 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-image/src/imageupload/imageuploadediting.ts b/packages/ckeditor5-image/src/imageupload/imageuploadediting.ts index c32bb594b20..9ff14cb0fb5 100644 --- a/packages/ckeditor5-image/src/imageupload/imageuploadediting.ts +++ b/packages/ckeditor5-image/src/imageupload/imageuploadediting.ts @@ -16,7 +16,8 @@ import { type Writer, type DataTransfer, type ViewElement, - type NodeAttributes + type NodeAttributes, + type DowncastAttributeEvent } from 'ckeditor5/src/engine.js'; import { Notification } from 'ckeditor5/src/ui.js'; @@ -66,6 +67,14 @@ export default class ImageUploadEditing extends Plugin { */ private readonly _uploadImageElements: Map; + /** + * An internal mapping of {@link module:upload/filerepository~FileLoader#id file loader UIDs} and + * upload responses for handling images dragged during their upload process. When such images are later + * dropped, their original upload IDs no longer exist in the registry (as the original upload completed). + * This map preserves the upload responses to properly handle such cases. + */ + private readonly _uploadedImages = new Map(); + /** * @inheritDoc */ @@ -106,6 +115,13 @@ export default class ImageUploadEditing extends Plugin { key: 'uploadId' }, model: 'uploadId' + } ) + .attributeToAttribute( { + view: { + name: 'img', + key: 'data-ck-upload-id' + }, + model: 'uploadId' } ); // Handle pasted images. @@ -217,6 +233,19 @@ export default class ImageUploadEditing extends Plugin { const loader = fileRepository.loaders.get( uploadId ); if ( !loader ) { + // If the loader does not exist, it means that the image was already uploaded + // and the loader promise was removed from the registry. In that scenario we need + // to restore response object from the internal map. + if ( !isInsertedInGraveyard && this._uploadedImages.has( uploadId ) ) { + // Fire `uploadComplete` to set proper attributes on the image element. + this.fire( 'uploadComplete', { + data: this._uploadedImages.get( uploadId )!, + imageElement: imageElement as Element + } ); + + this._uploadedImages.delete( uploadId ); + } + continue; } @@ -274,12 +303,16 @@ export default class ImageUploadEditing extends Plugin { schema.extend( 'imageBlock', { allowAttributes: [ 'uploadId', 'uploadStatus' ] } ); + + this._registerConverters( 'imageBlock' ); } if ( this.editor.plugins.has( 'ImageInlineEditing' ) ) { schema.extend( 'imageInline', { allowAttributes: [ 'uploadId', 'uploadStatus' ] } ); + + this._registerConverters( 'imageInline' ); } } @@ -360,6 +393,8 @@ export default class ImageUploadEditing extends Plugin { } this.fire( 'uploadComplete', { data, imageElement } ); + + this._uploadedImages.set( loader.id, data ); } ); clean(); @@ -445,6 +480,43 @@ export default class ImageUploadEditing extends Plugin { writer.setAttributes( attributes, image ); } } + + /** + * Registers image upload converters. + * + * @param imageType The type of the image. + */ + private _registerConverters( imageType: 'imageBlock' | 'imageInline' ) { + const { conversion, plugins } = this.editor; + + const fileRepository = plugins.get( FileRepository ); + const imageUtils = plugins.get( ImageUtils ); + + // It sets `data-ck-upload-id` attribute on the view image elements that are not fully uploaded. + // It avoids the situation when image disappears when it's being moved and upload is not finished yet. + // See more: https://github.com/ckeditor/ckeditor5/issues/16967 + conversion.for( 'dataDowncast' ).add( dispatcher => { + dispatcher.on( `attribute:uploadId:${ imageType }`, ( evt, data, conversionApi ) => { + if ( !conversionApi.consumable.test( data.item, evt.name ) ) { + return; + } + + const loader = fileRepository.loaders.get( data.attributeNewValue as string ); + + if ( !loader || !loader.data ) { + return null; + } + + const viewElement = conversionApi.mapper.toViewElement( data.item as Element )!; + const img = imageUtils.findViewImgElement( viewElement ); + + if ( img ) { + conversionApi.consumable.consume( data.item, evt.name ); + conversionApi.writer.setAttribute( 'data-ck-upload-id', loader.id, img ); + } + } ); + } ); + } } /** diff --git a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js index f975eb4f080..7542113d687 100644 --- a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js +++ b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js @@ -1552,6 +1552,138 @@ describe( 'ImageUploadEditing', () => { } ); } ); + describe( 'data downcast conversion of images with uploading state', () => { + it( 'should dump the `data-ck-upload-id` into the data', async () => { + const onDispatch = sinon.spy( ( evt, data, conversionApi ) => { + const wasConsumed = conversionApi.consumable.test( data.item, 'attribute:uploadId:imageInline' ); + + expect( wasConsumed ).to.be.true; + } ); + + editor.conversion.for( 'downcast' ).add( dispatcher => + dispatcher.on( 'attribute:uploadId:imageInline', onDispatch, { priority: 'high' } ) + ); + + setModelData( model, '[]foo' ); + + const file = createNativeFileMock(); + editor.execute( 'uploadImage', { file } ); + loader.file.then( () => nativeReaderMock.mockSuccess( base64Sample ) ); + + await timeout( 50 ); + + const uploadId = adapterMocks[ 0 ].loader.id; + + expect( getModelData( editor.model ) ).to.be.equal( + `[]foo` + ); + + expect( onDispatch ).to.be.calledOnce; + expect( editor.getData() ).to.be.equal( + `

foo

` + ); + } ); + + it( 'should not crash if uploadId of down casted image is not found in loaders repository', async () => { + setModelData( model, '[]foo' ); + + const file = createNativeFileMock(); + editor.execute( 'uploadImage', { file } ); + loader.file.then( () => nativeReaderMock.mockSuccess( base64Sample ) ); + + await timeout( 50 ); + + const uploadId = adapterMocks[ 0 ].loader.id; + + sinon + .stub( fileRepository.loaders, 'get' ) + .withArgs( uploadId ) + .returns( null ); + + expect( getModelData( editor.model ) ).to.be.equal( + `[]foo` + ); + + expect( editor.getData() ).to.be.equal( '

foo

' ); + } ); + + it( 'should not downcast consumed uploadId image attribute', async () => { + editor.conversion.for( 'downcast' ).add( dispatcher => + dispatcher.on( 'attribute:uploadId:imageInline', ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item, 'attribute:uploadId:imageInline' ); + }, { priority: 'high' } ) + ); + + setModelData( model, '[]foo' ); + + const file = createNativeFileMock(); + editor.execute( 'uploadImage', { file } ); + loader.file.then( () => nativeReaderMock.mockSuccess( base64Sample ) ); + + await timeout( 50 ); + + const uploadId = adapterMocks[ 0 ].loader.id; + + expect( getModelData( editor.model ) ).to.be.equal( + `[]foo` + ); + + expect( editor.getData() ).to.be.equal( '

foo

' ); + } ); + + it( 'should restore image from `_uploadedImages` if it was pasted from clipboard', async () => { + setModelData( model, '[]foo' ); + + const file = createNativeFileMock(); + editor.execute( 'uploadImage', { file } ); + loader.file.then( () => nativeReaderMock.mockSuccess( base64Sample ) ); + + await timeout( 50 ); + + // Let's copy image in uploading state. + const uploadId = adapterMocks[ 0 ].loader.id; + expect( getModelData( editor.model ) ).to.be.equal( + `[]foo` + ); + + // Lets check if content of clipboard is correct. + const data = { + dataTransfer: createDataTransfer(), + preventDefault: () => {}, + stopPropagation: () => {} + }; + + viewDocument.fire( 'copy', data ); + expect( data.dataTransfer.getData( 'text/html' ) ).to.equal( `` ); + + // Let's resolve uploading status and ensure that image is loaded. + 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' } ) ); + } ); + + expect( editor.getData() ).to.be.equal( + '

foo

' + ); + + // Make sure it's no longer present in registry, so image upload is completed. + expect( fileRepository.loaders.get( uploadId ) ).to.be.null; + + // Let's paste the image from clipboard, it has upload id, which should be stored in plugin cache. + setModelData( model, 'hello[]' ); + + viewDocument.fire( 'paste', { + dataTransfer: mockDataTransfer( `` ), + preventDefault: () => {}, + stopPropagation: () => {} + } ); + + expect( editor.getData() ).to.be.equal( + '

hello

' + ); + } ); + } ); + // Helper for validating clipboard and model data as a result of a paste operation. This function checks both clipboard // data and model data synchronously (`expectedClipboardData`, `expectedModel`) and then the model data after `loader.file` // promise is resolved (so model state after successful/failed file fetch attempt). @@ -1685,3 +1817,17 @@ function base64ToBlob( base64Data ) { function timeout( ms ) { return new Promise( res => setTimeout( res, ms ) ); } + +function createDataTransfer() { + const store = new Map(); + + return { + setData( type, data ) { + store.set( type, data ); + }, + + getData( type ) { + return store.get( type ); + } + }; +}