Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Embed images pasted from Word in Safari #1976

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Fixed Issues:
* [#1592](https://github.com/ckeditor/ckeditor-dev/issues/1592): [Image Base](https://ckeditor.com/cke4/addon/imagebase) caption is not visible after paste.
* [#620](https://github.com/ckeditor/ckeditor-dev/issues/620): Fixed: [`forcePasteAsPlainText`](https://docs.ckeditor.com/ckeditor4/latest/api/CKEDITOR_config.html#cfg-forcePasteAsPlainText) will be respected when internal and cross-editor pasting happen.
* [#1467](https://github.com/ckeditor/ckeditor-dev/issues/1467): Fixed: [Table Resize](https://ckeditor.com/cke4/addon/tableresize) resizing coursor appearing in middle of merged cell.
* [#1134](https://github.com/ckeditor/ckeditor-dev/issues/1134): [Safari] Fixed: [Paste from Word](https://ckeditor.com/cke4/addon/pastefromword) does not embed images in Safari browser.

API Changes:

Expand Down
34 changes: 34 additions & 0 deletions core/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -1596,6 +1596,40 @@
return base64string;
},

/**
* Return file type based on first 4 bytes of given file. Currently supported file types: `image/png`, `image/jpeg`, `image/gif`.
*
* @since 4.10.0
* @param {Uint8Array} bytesArray Typed array which will be analysed to obtain file type.
* @returns {String/Null} File type recognized from given typed array or null.
*/
getFileTypeFromHeader: function( bytesArray ) {
var header = '',
fileType = null,
bytesHeader = bytesArray.subarray( 0, 4 );

for ( var i = 0; i < bytesHeader.length; i++ ) {
header += bytesHeader[ i ].toString( 16 );
}

switch ( header ) {
case '89504e47':
fileType = 'image/png';
break;
case '47494638':
fileType = 'image/gif';
break;
case 'ffd8ffe0':
case 'ffd8ffe1':
case 'ffd8ffe2':
case 'ffd8ffe3':
case 'ffd8ffe8':
fileType = 'image/jpeg';
break;
}
return fileType;
},

/**
* A set of functions for operations on styles.
*
Expand Down
74 changes: 53 additions & 21 deletions plugins/ajax/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,46 +51,54 @@
return ( xhr.readyState == 4 && ( ( xhr.status >= 200 && xhr.status < 300 ) || xhr.status == 304 || xhr.status === 0 || xhr.status == 1223 ) );
}

function getResponseText( xhr ) {
if ( checkStatus( xhr ) )
return xhr.responseText;
return null;
}
function getResponse( xhr, type ) {
if ( !checkStatus( xhr ) ) {
return null;
}

function getResponseXml( xhr ) {
if ( checkStatus( xhr ) ) {
var xml = xhr.responseXML;
return new CKEDITOR.xml( xml && xml.firstChild ? xml : xhr.responseText );
switch ( type ) {
case 'text':
return xhr.responseText;
case 'xml':
var xml = xhr.responseXML;
return new CKEDITOR.xml( xml && xml.firstChild ? xml : xhr.responseText );
case 'arraybuffer':
return xhr.response;
default:
return null;
}
return null;
}

function load( url, callback, getResponseFn ) {
function load( url, callback, responseType ) {
var async = !!callback;

var xhr = createXMLHttpRequest();

if ( !xhr )
return null;

if ( async && responseType !== 'text' && responseType !== 'xml' ) {
xhr.responseType = responseType;
}

xhr.open( 'GET', url, async );

if ( async ) {
// TODO: perform leak checks on this closure.
xhr.onreadystatechange = function() {
if ( xhr.readyState == 4 ) {
callback( getResponseFn( xhr ) );
callback( getResponse( xhr, responseType ) );
xhr = null;
}
};
}

xhr.send( null );

return async ? '' : getResponseFn( xhr );
return async ? '' : getResponse( xhr, responseType );
}

function post( url, data, contentType, callback, getResponseFn ) {
function post( url, data, contentType, callback, responseType ) {
var xhr = createXMLHttpRequest();

if ( !xhr )
Expand All @@ -101,7 +109,7 @@
xhr.onreadystatechange = function() {
if ( xhr.readyState == 4 ) {
if ( callback ) {
callback( getResponseFn( xhr ) );
callback( getResponse( xhr, responseType ) );
}
xhr = null;
}
Expand All @@ -128,12 +136,16 @@
* @param {String} url The URL from which the data is loaded.
* @param {Function} [callback] A callback function to be called on
* data load. If not provided, the data will be loaded
* synchronously.
* @returns {String} The loaded data. For asynchronous requests, an
* synchronously. Please notice that only text data might be loaded synchrnously.
* @param {String} [responseType='text'] Defines type of returned data.
* Currently supports: `text`, `xml`, `arraybuffer`. This parameter was added in 4.10.
* @returns {String/null} The loaded data. For asynchronous requests, an
* empty string. For invalid requests, `null`.
*/
load: function( url, callback ) {
return load( url, callback, getResponseText );
load: function( url, callback, responseType ) {
responseType = responseType || 'text';

return load( url, callback, responseType );
},

/**
Expand All @@ -157,7 +169,7 @@
* depending on the `status` of the request.
*/
post: function( url, data, contentType, callback ) {
return post( url, data, contentType, callback, getResponseText );
return post( url, data, contentType, callback, 'text' );
},

/**
Expand All @@ -179,7 +191,27 @@
* empty string. For invalid requests, `null`.
*/
loadXml: function( url, callback ) {
return load( url, callback, getResponseXml );
return load( url, callback, 'xml' );
},

/**
* Converts blob url into base64 string. Conversion is happening asynchronously.
* Currently supported file types: `image/png`, `image/jpeg`, `image/gif`.
*
* @since 4.10.0
* @param {String} blobUrlSrc Address of blob which is going to be converted
* @param {Function} callback Function to execute when blob url is be converted.
* @param {String} callback.dataUri data uri represent transformed blobUrl or empty string file type was unrecognized.
*/
convertBlobUrlToBase64: function( blobUrlSrc, callback ) {
load( blobUrlSrc, function( arrayBuffer ) {
var data = new Uint8Array( arrayBuffer ),
fileType = CKEDITOR.tools.getFileTypeFromHeader( data.subarray( 0, 4 ) ),
base64 = CKEDITOR.tools.convertBytesToBase64( data );

callback( fileType ? 'data:' + fileType + ';base64,' + base64 : '' );

} , 'arraybuffer' );
}
};
} )();
Expand Down
18 changes: 16 additions & 2 deletions plugins/pastefromword/filter/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@

CKEDITOR.cleanWord = function( mswordHtml, editor ) {
var msoListsDetected = Boolean( mswordHtml.match( /mso-list:\s*l\d+\s+level\d+\s+lfo\d+/ ) ),
shapesIds = [];
shapesIds = [],
blobUrls = [];

function shapeTagging( element ) {
// Check if regular or canvas shape (#1088).
Expand Down Expand Up @@ -140,6 +141,10 @@
return false;
}

if ( element.attributes.src && element.attributes.src.match( /^blob:https?:\/\// ) ) {
blobUrls.push( element.attributes.src );
}

},
'p': function( element ) {
element.filterChildren( filter );
Expand Down Expand Up @@ -515,7 +520,16 @@

fragment.writeHtml( writer );

return writer.getHtml();
// If there was blobUrl detected (Paste images from word in Safari browser),
// Then we need to transform those images asynchronously into base64 and replace them in editor.
if ( blobUrls.length ) {
return {
dataValue: writer.getHtml(),
blobUrls: blobUrls
};
} else {
return writer.getHtml();
}
};

/**
Expand Down
52 changes: 50 additions & 2 deletions plugins/pastefromword/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
/* global confirm */

CKEDITOR.plugins.add( 'pastefromword', {
requires: 'clipboard',
requires: 'clipboard,ajax',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it any other way to not require ajax for PfW? It looks a little bit strange. I wonder if FileReader would be an alternative here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've considered using FileTools here. Unfortunately FileTools contains logic responsible for uploading files to the server, there is no methods helping downloading files from given URL, where Ajax has it and only requires to add new file type to support. That's it was chosen here. I could extend FileTools, with new functions, but this seems like new separate task, or hide completely XHR inside PFW and do not rely on Ajax plugin.
Using Ajax seems to be most convenient and time efficient for me.

@mlewand WDYT? Should we require Ajax in PFW or use some solution?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Comandeer what are the downsides of using ajax plugin for this case? Using FileReader just for sake of using it results with more code to worry about.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mlewand it wasn't exactly a downside, I just felt that current use of ajax plugin is inappropriate, but I haven't spend much time on it as whole fix wasn't working for me. You're comment about using plugin in core explains my anxiety ;)

// jscs:disable maximumLineLength
lang: 'af,ar,az,bg,bn,bs,ca,cs,cy,da,de,de-ch,el,en,en-au,en-ca,en-gb,eo,es,es-mx,et,eu,fa,fi,fo,fr,fr-ca,gl,gu,he,hi,hr,hu,id,is,it,ja,ka,km,ko,ku,lt,lv,mk,mn,ms,nb,nl,no,oc,pl,pt,pt-br,ro,ru,si,sk,sl,sq,sr,sr-latn,sv,th,tr,tt,ug,uk,vi,zh,zh-cn', // %REMOVE_LINE_CORE%
// jscs:enable maximumLineLength
Expand Down Expand Up @@ -96,7 +96,16 @@
editor.fire( 'paste', data );
} else if ( !editor.config.pasteFromWordPromptCleanup || ( forceFromWord || confirm( editor.lang.pastefromword.confirmCleanup ) ) ) {

pfwEvtData.dataValue = CKEDITOR.cleanWord( pfwEvtData.dataValue, editor );
var filteredData = CKEDITOR.cleanWord( pfwEvtData.dataValue, editor );

if ( typeof filteredData === 'string' ) {
pfwEvtData.dataValue = filteredData;
} else if ( typeof filteredData === 'object' ) {
pfwEvtData.dataValue = filteredData.dataValue;
handleBlobs( filteredData.blobUrls, function() {
editor.fire( 'saveSnapshot' );
} );
}

editor.fire( 'afterPasteFromWord', pfwEvtData );

Expand Down Expand Up @@ -167,6 +176,45 @@
}
}
}

// Method takes blob list (Array of Strings) and when finish processing it, then run callback method.
// 1. Remove blob duplicates (if exists).
// 2. Count amount of URLs to process.
// 3. For each blobUrl calculate its base64 value and store it in map blobUrl as a key and base64 as a value.
// 4. If process last blobUrl run replaceBlobUrlsInEditor and after that run callback function.
function handleBlobs( blobs, callback ) {
var arrayTools = CKEDITOR.tools.array,
blobUrlsToProcess = removeDuplicates( blobs ),
blobUrlsToBase64Map = {},
amountOfBlobsToProcess = blobUrlsToProcess.length;

arrayTools.forEach( blobUrlsToProcess, function( blobUrl ) {
CKEDITOR.ajax.convertBlobUrlToBase64( blobUrl, function( base64 ) {
blobUrlsToBase64Map[ blobUrl ] = base64;
amountOfBlobsToProcess--;
if ( amountOfBlobsToProcess === 0 ) {
replaceBlobUrlsInEditor( blobUrlsToBase64Map );
callback();
}
}, this );
}, this );

function removeDuplicates( arr ) {
return arrayTools.filter( arr, function( item, index ) {
return index === arrayTools.indexOf( arr, item );
} );
}

function replaceBlobUrlsInEditor( map ) {
for ( var blob in map ) {
var nodeList = editor.editable().find( 'img[src="' + blob + '"]' ).toArray();
arrayTools.forEach( nodeList, function( element ) {
element.setAttribute( 'src', map[ blob ] );
element.setAttribute( 'data-cke-saved-src', map[ blob ] );
}, this );
}
}
}
}

} );
Expand Down
52 changes: 51 additions & 1 deletion tests/core/tools.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* bender-tags: editor */
/* bender-ckeditor-plugins: ajax */

( function() {
'use strict';
Expand Down Expand Up @@ -856,7 +857,56 @@
CKEDITOR.tools.array.forEach( testCases, function( test ) {
assert.areSame( test.base64, CKEDITOR.tools.convertBytesToBase64( test.bytes ) );
} );
}
},

// #1134
'test getFileTypeFromHeader': function() {
if ( typeof Uint8Array !== 'function' ) {
assert.ignore();
}
var test_cases = [
{
input: '89504e47',
output: 'image/png'
},
{
input: '47494638',
output: 'image/gif'
},
{
input: 'ffd8ffe0',
output: 'image/jpeg'
},
{
input: 'ffd8ffe1',
output: 'image/jpeg'
},
{
input: 'ffd8ffe2',
output: 'image/jpeg'
},
{
input: 'ffd8ffe3',
output: 'image/jpeg'
},
{
input: 'ffd8ffe8',
output: 'image/jpeg'
},
{
input: '12345678',
output: null
},
{
input: 'ff',
output: null
}
];

CKEDITOR.tools.array.forEach( test_cases, function( test ) {
var header = CKEDITOR.tools.getFileTypeFromHeader( Uint8Array.from( CKEDITOR.tools.convertHexStringToBytes( test.input ) ) );
assert.areEqual( test.output, header, 'There is problem for test case with input: ' + test.input );
} );
}
} );
} )();
Loading