When adding an image in a page, if an attachment with a similar name already exists, the attachment is replaced by the new image (with an version increment).
But, for images pasted in CKEditor, the user does not control the image name.
The image name is a localized and browser dependent constant (e.g., image.png on Chrome, or grafik.png in a German Firefox).
Consequently, if a user paste several images to the same document, the last one will override the previous one (or if an image.png image already exists).
The current approach to address this issue is to check if the image is named image.png and to replace it with ${TIMESTAMP}-${RANDOM}.png.
But, as explained above, this is not accurate as for German Firefox, the file name is grafik.png.
You can see the Draft PR where I propose an approach to more accurately identify if an image originates from a paste operation (using beforePaste/afterPaste event listeners).
This PR is also proposing a behavioral change. The pasted filename is now transformed to ${FILENAME}-${TIMESTAMP}-${RANDOM}.png where FILENAME is the original name of the pasted file (e.g, image or grafik).
This change is not technically required and we could keep using the ${TIMESTAMP}-${RANDOM}.png pattern, but I feel like having the original filename could make for better attachments names.
So the question is, WDYT of changing the filename pattern:
This is not always true. It depends very much on the tool used to copy the image. Sometimes you do have a proper image file name. The question is more if we agree to have a different behaviour on paste versus drop / upload:
drop / upload: overwrite existing images / attachments, if the file name matches
paste: never overwrite existing images / attachments
I’m not fully convinced on my side but I don’t have a better solution and the current approach is clearly not good enough as we had many complains from users. So +1 to try this out.
I have another option in mind. At paste time, prompt the user to set a filename manually.
Either all the time, or by checking first if the new file name conflicts with an existing attachment.
We could check for the conflict on the beforePaste event, and either change the name automatically (e.g., with an incremental suffix), or open a prompt so that the user could manually edit the name.
I’m note fully sure of how strong it would be regarding:
the temporary attachment store (which would need to be checked in case of multiple paste in the same edit session)
a document edited by several users at the same time, with several uploading images with the same name
Note that this approach could even be generalize to all ways to upload attachments, since conflicts can also occur there too.
You’d probably need to perform the check for both existing attachments and attachments in the temporary store, but that should be doable I think.
It would be a standard edition conflict, but right now I’m not sure we’re giving much options for fixing those in case of uploads. Could be interesting to check actually.
+0 for this
IMO we’d need to have some UI in the editor to manipulate the added attachments in current editing session, e.g. to also allow canceling an attachment before saving (see: Loading...). So I’d go for the option you suggested with an automatic incremental suffix, and then provide in this future UI the capability to rename the attachments before actually performing the save.
Something that wasn’t clear in my proposal is that pasted files also include drag-and-dropped files from CKEditor point of view.
Meaning that any file drag-and-dropped in the editor is now renamed with the ${TIMESTAMP}-${RANDOM}.png pattern (see Loading...).
Do you thinks this is a correct behavior?
If not, I can see several options:
revert to the fix (and get back the ‘grafik’ bug)
hardcode ‘grafik’ in the codebase as this is the only instance we know of
offer an configuration to define a set of known ‘paste’ filenames, and only those would be replaced with the pattern (note that in this case, the drag-and-drop or upload of ‘grafik.png’ or ‘image.png’ would still be replaced by ${TIMESTAMP}-${RANDOM}.png)
update the pattern to what I proposed initially to ${FILENAME}-${TIMESTAMP}-${RANDOM}.png so that it is less confusing as the initial name of the drag-and-dropped file is still present in the resulting file
What do you this is the best option:
for 15.5-rc-1 (where XWIKI-20932 has not been released yet)
for 14.10.12 where the issue has been raised by @ilie.andriuta
Thanks for pointing this out, there is actually a event.data.method with a value specific to the method of insertion:
‘drop’ for drag-and drop
‘paste’ for pasting
This makes the fix much more obvious.
I propose instead of what I proposed above to keep the legacy naming, but to only apply it when event.data.method is paste (i.e., drag-and-dropped files wouldn’t be renamed)