Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #264 from ckeditor/t/258
Browse files Browse the repository at this point in the history 
Other: The image uploading listener for handling `base64/blob` images no longer stops `inputTransformation` event. Closes #263. Closes ckeditor/ckeditor5-paste-from-office#44.
  • Loading branch information
@jodator
jodator committed Jan 22, 2019
2 parents 205f119 + f5a52f3 commit 8c5b4fc
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 62 deletions.
33 changes: 11 additions & 22 deletions src/imageupload/imageuploadediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,36 +101,25 @@ export default class ImageUploadEditing extends Plugin {
this.listenTo( editor.plugins.get( 'Clipboard' ), 'inputTransformation', ( evt, data ) => {
const fetchableImages = Array.from( editor.editing.view.createRangeIn( data.content ) )
.filter( value => isLocalImage( value.item ) && !value.item.getAttribute( 'uploadProcessed' ) )
.map( value => fetchLocalImage( value.item ) );
.map( value => { return { promise: fetchLocalImage( value.item ), imageElement: value.item }; } );

if ( !fetchableImages.length ) {
return;
}

evt.stop();
const writer = new UpcastWriter();

Promise.all( fetchableImages ).then( items => {
const writer = new UpcastWriter();
for ( const fetchableImage of fetchableImages ) {
// Set attribute marking that the image was processed already.
writer.setAttribute( 'uploadProcessed', true, fetchableImage.imageElement );

for ( const item of items ) {
if ( !item.file ) {
// Failed to fetch image or create a file instance, remove image element.
writer.remove( item.image );
} else {
// Set attribute marking the image as processed.
writer.setAttribute( 'uploadProcessed', true, item.image );
const loader = fileRepository.createLoader( fetchableImage.promise );

const loader = fileRepository.createLoader( item.file );

if ( loader ) {
writer.setAttribute( 'src', '', item.image );
writer.setAttribute( 'uploadId', loader.id, item.image );
}
}
if ( loader ) {
writer.setAttribute( 'src', '', fetchableImage.imageElement );
writer.setAttribute( 'uploadId', loader.id, fetchableImage.imageElement );
}

editor.plugins.get( 'Clipboard' ).fire( 'inputTransformation', data );
} );
}
} );
}

Expand Down Expand Up @@ -229,7 +218,7 @@ export default class ImageUploadEditing extends Plugin {
}

// Might be 'aborted'.
if ( loader.status == 'error' ) {
if ( loader.status == 'error' && error ) {
notification.showWarning( error, {
title: t( 'Upload failed' ),
namespace: 'upload'
Expand Down
22 changes: 10 additions & 12 deletions src/imageupload/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,30 +22,28 @@ export function isImageType( file ) {
}

/**
* Creates a promise which fetches the image local source (base64 or blob) and returns as a `File` object.
* Creates a promise which fetches the image local source (base64 or blob) and resolves with a `File` object.
*
* @param {module:engine/view/element~Element} image Image which source to fetch.
* @returns {Promise} A promise which resolves when image source is fetched and converted to `File` instance.
* It resolves with object holding initial image element (as `image`) and its file source (as `file`). If
* the `file` attribute is null, it means fetching failed.
* @returns {Promise.<File>} A promise which resolves when image source is fetched and converted to `File` instance.
* It resolves with a `File` object. If there were any errors during file processing the promise will be rejected.
*/
export function fetchLocalImage( image ) {
return new Promise( resolve => {
return new Promise( ( resolve, reject ) => {
const imageSrc = image.getAttribute( 'src' );

// Fetch works asynchronously and so does not block browser UI when processing data.
fetch( image.getAttribute( 'src' ) )
fetch( imageSrc )
.then( resource => resource.blob() )
.then( blob => {
const mimeType = getImageMimeType( blob, image.getAttribute( 'src' ) );
const mimeType = getImageMimeType( blob, imageSrc );
const ext = mimeType.replace( 'image/', '' );
const filename = `image.${ ext }`;
const file = createFileFromBlob( blob, filename, mimeType );

resolve( { image, file } );
file ? resolve( file ) : reject();
} )
.catch( () => {
// We always resolve a promise so `Promise.all` will not reject if one of many fetch fails.
resolve( { image, file: null } );
} );
.catch( reject );
} );
}

Expand Down
201 changes: 173 additions & 28 deletions tests/imageupload/imageuploadediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import FileRepository from '@ckeditor/ckeditor5-upload/src/filerepository';
import { UploadAdapterMock, createNativeFileMock, NativeFileReaderMock } from '@ckeditor/ckeditor5-upload/tests/_utils/mocks';

import { setData as setModelData, getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
import { getData as getViewData, stringify as stringifyView } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';

import log from '@ckeditor/ckeditor5-utils/src/log';
import env from '@ckeditor/ckeditor5-utils/src/env';
Expand Down Expand Up @@ -661,11 +661,20 @@ describe( 'ImageUploadEditing', () => {
} );

it( 'should not upload and remove image if fetch failed', done => {
editor.plugins.get( 'Clipboard' ).on( 'inputTransformation', () => {
const expected = '<paragraph>[]foo</paragraph>';
const notification = editor.plugins.get( Notification );

expectModel( done, getModelData( model ), expected );
}, { priority: 'low' } );
// Prevent popping up alert window.
notification.on( 'show:warning', evt => {
evt.stop();
}, { priority: 'high' } );

expectData(
'<img src="" uploadId="#loader1_id" uploadProcessed="true"></img>',
'[<image src="" uploadId="#loader1_id" uploadStatus="reading"></image>]<paragraph>foo</paragraph>',
'<paragraph>[]foo</paragraph>',
done,
false
);

setModelData( model, '<paragraph>[]foo</paragraph>' );

Expand All @@ -684,14 +693,29 @@ describe( 'ImageUploadEditing', () => {
} );

it( 'should upload only images which were successfully fetched and remove failed ones', done => {
editor.plugins.get( 'Clipboard' ).on( 'inputTransformation', () => {
const expected = '<paragraph>bar</paragraph>' +
`<image src="" uploadId="${ adapterMocks[ 0 ].loader.id }" uploadStatus="reading"></image>` +
`[<image src="" uploadId="${ adapterMocks[ 1 ].loader.id }" uploadStatus="reading"></image>]` +
'<paragraph>foo</paragraph>';
const notification = editor.plugins.get( Notification );

expectModel( done, getModelData( model ), expected );
}, { priority: 'low' } );
// Prevent popping up alert window.
notification.on( 'show:warning', evt => {
evt.stop();
}, { priority: 'high' } );

const expectedModel = '<paragraph>bar</paragraph>' +
'<image src="" uploadId="#loader1_id" uploadStatus="reading"></image>' +
'<image src="" uploadId="#loader2_id" uploadStatus="reading"></image>' +
'[<image src="" uploadId="#loader3_id" uploadStatus="reading"></image>]' +
'<paragraph>foo</paragraph>';
const expectedFinalModel = '<paragraph>bar</paragraph>' +
'<image src="" uploadId="#loader1_id" uploadStatus="reading"></image>' +
'[<image src="" uploadId="#loader2_id" uploadStatus="reading"></image>]' +
'<paragraph>foo</paragraph>';

expectData(
'',
expectedModel,
expectedFinalModel,
done
);

setModelData( model, '<paragraph>[]foo</paragraph>' );

Expand All @@ -710,7 +734,7 @@ describe( 'ImageUploadEditing', () => {
if ( counter < 3 ) {
return fetch( src );
} else {
return new Promise( ( res, rej ) => rej() );
return Promise.reject();
}
} );

Expand All @@ -722,13 +746,23 @@ describe( 'ImageUploadEditing', () => {

window.File = undefined;

editor.plugins.get( 'Clipboard' ).on( 'inputTransformation', () => {
window.File = fileFn;
const notification = editor.plugins.get( Notification );

const expected = '<paragraph>baz[]foo</paragraph>';
// Prevent popping up alert window.
notification.on( 'show:warning', evt => {
evt.stop();
}, { priority: 'high' } );

expectModel( done, getModelData( model ), expected );
}, { priority: 'low' } );
expectData(
'<img src="" uploadId="#loader1_id" uploadProcessed="true"></img><p>baz</p>',
'<image src="" uploadId="#loader1_id" uploadStatus="reading"></image><paragraph>baz[]foo</paragraph>',
'<paragraph>baz[]foo</paragraph>',
err => {
window.File = fileFn;
done( err );
},
false
);

setModelData( model, '<paragraph>[]foo</paragraph>' );

Expand All @@ -742,19 +776,29 @@ describe( 'ImageUploadEditing', () => {
} );

it( 'should not upload and remove image when `File` constructor is not supported', done => {
const fileFn = window.File;

window.File = function() {
throw new Error( 'Function expected.' ); // Simulating Edge browser behaviour here.
};
if ( isEdgeEnv ) {
// Since on Edge `File` is already stubbed, restore it to it native form so that exception will be thrown.
testUtils.sinon.restore();
// Since all stubs were restored, re-stub `scrollToTheSelection`.
testUtils.sinon.stub( editor.editing.view, 'scrollToTheSelection' ).callsFake( () => {} );
} else {
testUtils.sinon.stub( window, 'File' ).throws( 'Function expected.' );
}

editor.plugins.get( 'Clipboard' ).on( 'inputTransformation', () => {
window.File = fileFn;
const notification = editor.plugins.get( Notification );

const expected = '<paragraph>baz[]foo</paragraph>';
// Prevent popping up alert window.
notification.on( 'show:warning', evt => {
evt.stop();
}, { priority: 'high' } );

expectModel( done, getModelData( model ), expected );
}, { priority: 'low' } );
expectData(
'<p>baz</p><img src="" uploadId="#loader1_id" uploadProcessed="true"></img>',
'<paragraph>baz</paragraph>[<image src="" uploadId="#loader1_id" uploadStatus="reading"></image>]<paragraph>foo</paragraph>',
'<paragraph>baz[]</paragraph><paragraph>foo</paragraph>',
done,
false
);

setModelData( model, '<paragraph>[]foo</paragraph>' );

Expand Down Expand Up @@ -822,8 +866,109 @@ describe( 'ImageUploadEditing', () => {

viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );
} );

it( 'should not show notification when file loader failed with no error', done => {
const notification = editor.plugins.get( Notification );

let notificationsCount = 0;
// Prevent popping up alert window.
notification.on( 'show:warning', evt => {
notificationsCount++;
evt.stop();
}, { priority: 'high' } );

// Check data after paste.
editor.plugins.get( 'Clipboard' ).on( 'inputTransformation', () => {
adapterMocks[ 0 ].loader.file.then( () => {
expect.fail( 'Promise should be rejected.' );
} ).catch( () => {
// Deffer so the promise could be resolved.
setTimeout( () => {
expect( notificationsCount ).to.equal( 0 );
done();
} );
} );
}, { priority: 'low' } );

setModelData( model, '<paragraph>[]foo</paragraph>' );

const clipboardHtml = `<img 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 );

// Stub `fetch` in a way that it always fails.
testUtils.sinon.stub( window, 'fetch' ).callsFake( () => Promise.reject() );

viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );
} );

// 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).
//
// @param {String} expectedClipboardData Expected clipboard data on `inputTransformation` event.
// @param {String} expectedModel Expected model data on `inputTransformation` event.
// @param {String} expectedModelOnFile Expected model data after all `file.loader` promises are fetched.
// @param {Function} doneFn Callback function to be called when all assertions are done or error occures.
// @param {Boolean} [onSuccess=true] If `expectedModelOnFile` data should be validated
// on `loader.file` a promise successful resolution or promise rejection.
function expectData( expectedClipboardData, expectedModel, expectedModelOnFile, doneFn, onSuccess ) {
// Check data after paste.
editor.plugins.get( 'Clipboard' ).on( 'inputTransformation', ( evt, data ) => {
const clipboardData = injectLoaderId( expectedClipboardData || '', adapterMocks );
const modelData = injectLoaderId( expectedModel, adapterMocks );
const finalModelData = injectLoaderId( expectedModelOnFile, adapterMocks );

if ( clipboardData.length ) {
expect( stringifyView( data.content ) ).to.equal( clipboardData );
}
expect( getModelData( model ) ).to.equal( modelData );

if ( onSuccess !== false ) {
adapterMocks[ 0 ].loader.file.then( () => {
// Deffer so the promise could be resolved.
setTimeout( () => {
expectModel( doneFn, getModelData( model ), finalModelData );
} );
} );
} else {
adapterMocks[ 0 ].loader.file.then( () => {
expect.fail( 'The `loader.file` should be rejected.' );
} ).catch( () => {
// Deffer so the promise could be resolved.
setTimeout( () => {
expectModel( doneFn, getModelData( model ), finalModelData );
} );
} );
}
}, { priority: 'low' } );
}
} );

// Replaces '#loaderX_id' parameter in the given string with a loader id. It is used
// so data string could be created before loader is initialized.
//
// @param {String} data String which have 'loader params' replaced.
// @param {Array.<UploadAdapterMock>} adapters Adapters list. Each adapter holds a reference to a loader which id is used.
// @returns {String} Data string with 'loader params' replaced.
function injectLoaderId( data, adapters ) {
let newData = data;

if ( newData.includes( '#loader1_id' ) ) {
newData = newData.replace( '#loader1_id', adapters[ 0 ].loader.id );
}
if ( newData.includes( '#loader2_id' ) ) {
newData = newData.replace( '#loader2_id', adapters[ 1 ].loader.id );
}
if ( newData.includes( '#loader3_id' ) ) {
newData = newData.replace( '#loader3_id', adapters[ 2 ].loader.id );
}

return newData;
}

// Asserts actual and expected model data.
//
// @param {function} done Callback function to be called when assertion is done.
Expand Down

0 comments on commit 8c5b4fc

Please sign in to comment.