Opened 3 years ago

Closed 3 years ago

Last modified 21 months ago

#13501 closed New Feature (fixed)

Add config setting to allow default filename for paste uploads

Reported by: Steve James Owned by: Szymon Kupś
Priority: Normal Milestone: CKEditor 4.5.3
Component: Core : Pasting Version: 4.5.0
Keywords: Cc:


Per your source code for paste uploads (upload repository):

If not set and the second parameter is a Base64 data string, then the 
file name will be created based on the MIME type by replacing '/' with 
'.', for example for `image/png` the file name will be `image.png`.

I just discovered this in via my upload handler... What if a user uploads a file actually named image.png? (not that far fetched) How am I supposed to discriminate between that case and the case where CKEditor used the default name?

It would be better to allow establishing the default name via a config setting. Then, I could set something like ~_noname_~ + extension that I could easily detect.

Change History (12)

comment:1 Changed 3 years ago by Piotrek Koszuliński

Keywords: paste upload with filename default removed
Status: newconfirmed
Version: 4.5.2 (GitHub - master)4.5.0

Makes sense.

comment:2 Changed 3 years ago by Szymon Kupś

Owner: set to Szymon Kupś
Status: confirmedassigned

comment:3 Changed 3 years ago by Szymon Kupś

Status: assignedreview

comment:4 Changed 3 years ago by Steve James

I looked at the code for this...

I was expecting to see logic along the lines of fetch the default from the config or set it as 'image' as a fallback. e.g., something like

var mimeParts,defaultFileName = editor.config.fileTools_defaultFileName || 'image';

unless 'image' is already the fallback in config.js. (I did not see that, though.). However, the code as written does not have the

|| 'image'

or other fallback in it, so you have a potential issue if the config value is not set.

Thus, the value from the config is an override, since you cannot reliably depend on developers always setting this value. A reasonable approach might be to just put fileTools_defaultFileName setting in your default config.js with a value of 'image'. That way, it is documented and you know you have a value, but it can be overridden with a better value.

comment:5 Changed 3 years ago by Szymon Kupś

This code checks if defaultFileName is present. If not - it leaves creating file name from MIME type as it was before introducing this fix. So for image/png MIME type file name will be image.png and so on.

mimeParts = this.file.type.split( '/' );

if ( defaultFileName ) {
	mimeParts[ 0 ] = defaultFileName;

this.fileName = mimeParts.join( '.' );

comment:6 Changed 3 years ago by Steve James

Ok, sorry, I missed that. Thx.

comment:7 Changed 3 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.5.3

comment:8 Changed 3 years ago by Piotrek Koszuliński

Resolution: fixed
Status: reviewclosed

Merged to master with git:9c406a0.

I needed to update one test file though. There's a bit more tests to run together with the filetools plugin (uploadimage, uploadwidget, etc). Always make sure to find related plugins or simply run all plugins tests.

comment:9 Changed 3 years ago by Steve James

Is there also a related doc update needed?

Last edited 3 years ago by Steve James (previous) (diff)

comment:10 Changed 3 years ago by Piotrek Koszuliński

There are API docs for the new feature and they will be available on before the release. But we may also need to mention something in the guides, although I'm not sure if have a good place for this yet.

comment:11 Changed 3 years ago by Piotr Jasiun

I have added comment to the guild about this option:

Last edited 3 years ago by Piotr Jasiun (previous) (diff)

comment:12 Changed 21 months ago by Marcin K.

Not working in Firefox, but it could be Firefox fault. Firefox set own name - in polish language is "obraz.png"

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy