From 65a1f5a5eda3a481ded9d343ea711c0f841308c2 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Fri, 23 Jun 2017 16:12:41 +0200 Subject: [PATCH 1/2] Changed: "on-abort" fixer should permanently remove image placeholder. --- src/imageuploadengine.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/imageuploadengine.js b/src/imageuploadengine.js index c2e233a..03b4747 100644 --- a/src/imageuploadengine.js +++ b/src/imageuploadengine.js @@ -151,9 +151,9 @@ export default class ImageUploadEngine extends Plugin { clean(); - // Remove image from insertion batch. + // Permanently remove image from insertion batch. doc.enqueueChanges( () => { - batch.remove( imageElement ); + batch.remove( imageElement, true ); } ); } ); From 2354bde519685baaa751a7433ff2701f11ed7881 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Wed, 5 Jul 2017 13:07:03 +0200 Subject: [PATCH 2/2] Fix: correctly integrated upload with undo. --- src/imageuploadengine.js | 23 +++++++++---------- tests/imageuploadengine.js | 47 +++++++++++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/src/imageuploadengine.js b/src/imageuploadengine.js index 03b4747..78d2326 100644 --- a/src/imageuploadengine.js +++ b/src/imageuploadengine.js @@ -53,7 +53,7 @@ export default class ImageUploadEngine extends Plugin { } } ); - doc.on( 'change', ( evt, type, data, batch ) => { + doc.on( 'change', ( evt, type, data ) => { // Listen on document changes and: // * start upload process when image with `uploadId` attribute is inserted, // * abort upload process when image `uploadId` attribute is removed. @@ -68,7 +68,7 @@ export default class ImageUploadEngine extends Plugin { if ( loader ) { if ( type === 'insert' && loader.status == 'idle' ) { - this.load( loader, batch, imageElement ); + this.load( loader, imageElement ); } if ( type === 'remove' ) { @@ -88,10 +88,9 @@ export default class ImageUploadEngine extends Plugin { * * @protected * @param {module:upload/filerepository~FileLoader} loader - * @param {module:engine/model/batch~Batch} batch * @param {module:engine/model/element~Element} imageElement */ - load( loader, batch, imageElement ) { + load( loader, imageElement ) { const editor = this.editor; const t = editor.locale.t; const doc = editor.document; @@ -99,7 +98,7 @@ export default class ImageUploadEngine extends Plugin { const notification = editor.plugins.get( Notification ); doc.enqueueChanges( () => { - batch.setAttribute( imageElement, 'uploadStatus', 'reading' ); + doc.batch( 'transparent' ).setAttribute( imageElement, 'uploadStatus', 'reading' ); } ); loader.read() @@ -112,15 +111,15 @@ export default class ImageUploadEngine extends Plugin { editor.editing.view.render(); doc.enqueueChanges( () => { - batch.setAttribute( imageElement, 'uploadStatus', 'uploading' ); + doc.batch( 'transparent' ).setAttribute( imageElement, 'uploadStatus', 'uploading' ); } ); return promise; } ) .then( data => { doc.enqueueChanges( () => { - batch.setAttribute( imageElement, 'uploadStatus', 'complete' ); - batch.setAttribute( imageElement, 'src', data.original ); + doc.batch( 'transparent' ).setAttribute( imageElement, 'uploadStatus', 'complete' ); + doc.batch( 'transparent' ).setAttribute( imageElement, 'src', data.original ); // Srcset attribute for responsive images support. const srcsetAttribute = Object.keys( data ) @@ -134,7 +133,7 @@ export default class ImageUploadEngine extends Plugin { .join( ', ' ); if ( srcsetAttribute != '' ) { - batch.setAttribute( imageElement, 'srcset', srcsetAttribute ); + doc.batch( 'transparent' ).setAttribute( imageElement, 'srcset', srcsetAttribute ); } } ); @@ -153,14 +152,14 @@ export default class ImageUploadEngine extends Plugin { // Permanently remove image from insertion batch. doc.enqueueChanges( () => { - batch.remove( imageElement, true ); + doc.batch( 'transparent' ).remove( imageElement ); } ); } ); function clean() { doc.enqueueChanges( () => { - batch.removeAttribute( imageElement, 'uploadId' ); - batch.removeAttribute( imageElement, 'uploadStatus' ); + doc.batch( 'transparent' ).removeAttribute( imageElement, 'uploadId' ); + doc.batch( 'transparent' ).removeAttribute( imageElement, 'uploadStatus' ); } ); fileRepository.destroyLoader( loader ); diff --git a/tests/imageuploadengine.js b/tests/imageuploadengine.js index 9fb3cc6..5202109 100644 --- a/tests/imageuploadengine.js +++ b/tests/imageuploadengine.js @@ -10,6 +10,7 @@ import ImageEngine from '@ckeditor/ckeditor5-image/src/image/imageengine'; import ImageUploadEngine from '../src/imageuploadengine'; import ImageUploadCommand from '../src/imageuploadcommand'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import UndoEngine from '@ckeditor/ckeditor5-undo/src/undoengine'; import DataTransfer from '@ckeditor/ckeditor5-clipboard/src/datatransfer'; import FileRepository from '../src/filerepository'; import { AdapterMock, createNativeFileMock, NativeFileReaderMock } from './_utils/mocks'; @@ -33,7 +34,7 @@ describe( 'ImageUploadEngine', () => { } ); return ClassicTestEditor.create( { - plugins: [ ImageEngine, ImageUploadEngine, Paragraph ] + plugins: [ ImageEngine, ImageUploadEngine, Paragraph, UndoEngine ] } ) .then( newEditor => { editor = newEditor; @@ -235,6 +236,50 @@ describe( 'ImageUploadEngine', () => { sinon.assert.calledOnce( abortSpy ); } ); + it( 'image should be permanently removed if it is removed by user during upload', done => { + const file = createNativeFileMock(); + const notification = editor.plugins.get( Notification ); + setModelData( document, '{}foo bar' ); + + // Prevent popping up alert window. + notification.on( 'show:warning', evt => { + evt.stop(); + }, { priority: 'high' } ); + + editor.execute( 'imageUpload', { file } ); + + document.once( 'changesDone', () => { + // This is called after "manual" remove. + document.once( 'changesDone', () => { + // This is called after attributes are removed. + let undone = false; + + document.once( 'changesDone', () => { + if ( !undone ) { + undone = true; + + // This is called after abort remove. + expect( getModelData( document ) ).to.equal( '[]foo bar' ); + + editor.execute( 'undo' ); + + // Expect that the image has not been brought back. + expect( getModelData( document ) ).to.equal( '[]foo bar' ); + + done(); + } + } ); + } ); + } ); + + const image = document.getRoot().getChild( 0 ); + document.enqueueChanges( () => { + const batch = document.batch(); + + batch.remove( image ); + } ); + } ); + it( 'should create responsive image if server return multiple images', done => { const file = createNativeFileMock(); setModelData( document, '{}foo bar' );