Skip to content

Commit

Permalink
No longer crash when aborting image that is in graveyard.
Browse files Browse the repository at this point in the history
  • Loading branch information
Mati365 committed Nov 18, 2024
1 parent 3424ecb commit 7b5e9f7
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,13 @@ export default class ImageUploadEditing extends Plugin {

// Permanently remove image from insertion batch.
model.enqueueChange( { isUndoable: false }, writer => {
writer.remove( imageUploadElements.get( loader.id )! );
const node = imageUploadElements.get( loader.id )!;

// Handle situation when the image ha been removed and then `abort` exception was thrown.
// See: https://github.com/cksource/ckeditor5-commercial/issues/6817
if ( node && node.root.rootName !== '$graveyard' ) {
writer.remove( node );
}
} );

clean();
Expand Down
35 changes: 35 additions & 0 deletions packages/ckeditor5-image/tests/imageupload/imageuploadediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { setData as setModelData, getData as getModelData } from '@ckeditor/cked
import { getData as getViewData, stringify as stringifyView } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js';

import Notification from '@ckeditor/ckeditor5-ui/src/notification/notification.js';
import Writer from '@ckeditor/ckeditor5-engine/src/model/writer.js';
import { downcastImageAttribute } from '../../src/image/converters.js';
import { assertCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils.js';

Expand Down Expand Up @@ -1550,6 +1551,40 @@ describe( 'ImageUploadEditing', () => {
} );
} );
} );

it( 'should not remove image when it is already in graveyard', done => {
const notification = editor.plugins.get( Notification );
const file = createNativeFileMock();

notification.on( 'show:warning', evt => {
evt.stop();
}, { priority: 'high' } );

setModelData( model, '<paragraph>[]foo bar</paragraph>' );
editor.execute( 'uploadImage', { file } );

const image = doc.getRoot().getChild( 0 ).getChild( 0 );

editor.execute( 'undo' );

const removeMock = sinon.stub( Writer.prototype, 'remove' ).callThrough();

// Simulate change to trigger image upload error handling
const imagePlugin = editor.plugins.get( 'ImageUploadEditing' );
imagePlugin._uploadImageElements.set( loader.id, image );

// Throw aborted exception to simulate loader error
loader.status = 'aborted';

model.document.once( 'change', () => {
tryExpect( done, () => {
expect( image.root.rootName ).to.equal( '$graveyard' );
sinon.assert.notCalled( removeMock );
} );
} );

loader.file.then( () => nativeReaderMock.mockError( 'Upload error.' ) );
} );
} );

// Helper for validating clipboard and model data as a result of a paste operation. This function checks both clipboard
Expand Down

0 comments on commit 7b5e9f7

Please sign in to comment.