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

Broken integration between uploadimage and clipboard plugin #5333

Closed
Comandeer opened this issue Sep 21, 2022 · 1 comment · Fixed by #5364
Closed

Broken integration between uploadimage and clipboard plugin #5333

Comandeer opened this issue Sep 21, 2022 · 1 comment · Fixed by #5364
Assignees
Labels
plugin:clipboard The plugin which probably causes the issue. plugin:uploadimage The plugin which probably causes the issue. plugin:uploadwidget The plugin which probably causes the issue. regression This issue is a regression. status:confirmed An issue confirmed by the development team. support:2 An issue reported by a commercially licensed client. type:bug A bug.
Milestone

Comments

@Comandeer
Copy link
Member

Comandeer commented Sep 21, 2022

Type of report

Bug

Provide detailed reproduction steps (if any)

  1. Open https://ckeditor.com/docs/ckeditor4/latest/examples/fileupload.html#uploading-dropped-and-pasted-images
  2. Drag and drop any image into the editor.
  3. Wait for the upload to finish and then check the source of the editor.

Expected result

The image has preserved its original name.

Actual result

The image has a generic name in the format of image-<datetime>-<no>.

Other details

  • Browser: N/A
  • OS: N/A
  • CKEditor version: 4.17.0+
  • Installed CKEditor plugins: clipboard, uploadimage, uploadwidget

Similar to #4874 but much more subtle.

Due to how our clipboard logic works, handling images inside the clipboard plugin prevents the uploadimage/uploadwidget plugins from getting image files from the clipboard. This causes the uploadimage to use its fallback logic:

// Handle images which are not available in the dataTransfer.
// This means that we need to read them from the <img src="data:..."> elements.
editor.on( 'paste', function( evt ) {
// For performance reason do not parse data if it does not contain img tag and data attribute.
if ( !evt.data.dataValue.match( /<img[\s\S]+data:/i ) ) {
return;
}
var data = evt.data,
// Prevent XSS attacks.
tempDoc = document.implementation.createHTMLDocument( '' ),
temp = new CKEDITOR.dom.element( tempDoc.body ),
imgs, img, i;
// Without this isReadOnly will not works properly.
temp.data( 'cke-editable', 1 );
temp.appendHtml( data.dataValue );
imgs = temp.find( 'img' );
for ( i = 0; i < imgs.count(); i++ ) {
img = imgs.getItem( i );
// Assign src once, as it might be a big string, so there's no point in duplicating it all over the place.
var imgSrc = img.getAttribute( 'src' ),
// Image have to contain src=data:...
isDataInSrc = imgSrc && imgSrc.substring( 0, 5 ) == 'data:',
isRealObject = img.data( 'cke-realelement' ) === null;
// We are not uploading images in non-editable blocs and fake objects (https://dev.ckeditor.com/ticket/13003).
if ( isDataInSrc && isRealObject && !img.data( 'cke-upload-id' ) && !img.isReadOnly( 1 ) ) {
// Note that normally we'd extract this logic into a separate function, but we should not duplicate this string, as it might
// be large.
var imgFormat = imgSrc.match( /image\/([a-z]+?);/i ),
loader;
imgFormat = ( imgFormat && imgFormat[ 1 ] ) || 'jpg';
loader = editor.uploadRepository.create( imgSrc, getUniqueImageFileName( imgFormat ) );
loader.upload( uploadUrl );
fileTools.markElement( img, 'uploadimage', loader.id );
fileTools.bindNotifications( editor, loader );
}
}
data.dataValue = temp.getHtml();
} );

The solution introduced in the #4874 (setting the config.clipboard_handleImages to false) works but the broken behavior seems to work as long as the original name of the file is not important. It's so deceiving that we missed it for several releases in our docs… Due to that, I think we should revisit the issue and think about better integration between the clipboard and uploadimage/uploadwidget plugins.

@Comandeer Comandeer added type:bug A bug. status:confirmed An issue confirmed by the development team. plugin:clipboard The plugin which probably causes the issue. plugin:uploadimage The plugin which probably causes the issue. plugin:uploadwidget The plugin which probably causes the issue. labels Sep 21, 2022
@martynawierzbicka martynawierzbicka added the support:2 An issue reported by a commercially licensed client. label Sep 21, 2022
@Comandeer Comandeer added the regression This issue is a regression. label Sep 22, 2022
@CKEditorBot
Copy link
Collaborator

Closed in #5364

@CKEditorBot CKEditorBot added this to the 4.20.1 milestone Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin:clipboard The plugin which probably causes the issue. plugin:uploadimage The plugin which probably causes the issue. plugin:uploadwidget The plugin which probably causes the issue. regression This issue is a regression. status:confirmed An issue confirmed by the development team. support:2 An issue reported by a commercially licensed client. type:bug A bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants