Bug #81812
closed
Drag and drop file upload is not working in DCE file upload fields (ie. fields using flexforms)
Added by Giannis Economou over 7 years ago. Updated about 6 years ago.
100%
Description
When using DCE with a file upload field, users cannot upload files by using drag and drop in their content elements.
This bug might exist on all similarly defined flexform fields (not only on fields defined by DCE).
Files
| dropzone-insertion.png (60.6 KB) dropzone-insertion.png | insertAfter() of "dropzone" with id having dots in its value | Giannis Economou, 2017-07-06 14:05 | |
| dropzone-insertion-dropzone-target.png (52.1 KB) dropzone-insertion-dropzone-target.png | Giannis Economou, 2017-07-06 15:02 |
Updated by Gerrit Code Review over 7 years ago
- Status changed from New to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/53416
Updated by Markus Klein over 7 years ago
Can you please add more information here, what the code is that is inserted into DOM, so reviewers can understand by reading.
For testing it would be nice to have either an example within the ext:styleguide extension, or by providing a demo-setup/export here. (t3d)
Updated by Giannis Economou over 7 years ago
- File dropzone-insertion.png dropzone-insertion.png added
Markus Klein wrote:
Can you please add more information here, what the code is that is inserted into DOM, so reviewers can understand by reading.
For testing it would be nice to have either an example within the ext:styleguide extension, or by providing a demo-setup/export here. (t3d)
I am providing a quick screenshot (attached here) and some extra explanations for the review.
You will see that the "dropzone" div is inserted (by DragUploader.js) after the div above (shown also the in screenshot).
The div above, might have a value that includes dots (which is a CSS selector - special character, coming from flexforms).
Example div id value shown also in the screenshot:
div id="data-8-tt_content-NEW595e24ef50fb3309295065-pi_flexform---data---sheet.tabGeneral---lDEF---settings.mediaEntryImages---vDEF"
In such cases, "dropzone" insertion fails and files drag and drop is not functioning.
To have functional drag and drop on such cases, the insertion of "dropzone" should be successful, so we need to properly escape those special characters in the above div id for the insertAfter() to work.
This happens in DragUploader.js in https://review.typo3.org/53416.
Updated by Gerrit Code Review over 7 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/53416
Updated by Gerrit Code Review over 7 years ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/53416
Updated by Markus Klein over 7 years ago
What does data-dropzone-target hold for the case you posted in the screenshot?
Updated by Giannis Economou over 7 years ago
Markus Klein wrote:
What does
data-dropzone-targethold for the case you posted in the screenshot?
data-dropzone-target holds the id (selector) where the insertion of the dropzone will be done by DragUploader.js (after or before the div that gets selected using data-dropzone-target value).
So in first screenshot earlier, the data-dropzone-target had value:
"#data-8-tt_content-NEW595e24ef50fb3309295065-pi_flexform---data---sheet.tabGeneral---lDEF---settings.mediaEntryImages---vDEF"
Accoding to DragUploader.js code, the dropzone will be inserted after (or before) this targeted div.
Without escaping the specials chars in data-dropzone-target value it is never inserted in such cases (because not found when dots are present, for example).
As an extra, here is quick screenshot where both data-dropzone-target and the targeted id are visible at the same time.
Updated by Markus Klein over 7 years ago
Okay everything clear now.
BUT: IMO the fix must be to write a valid selector to the data-dropzone-target attribute. Consider the case (maybe currently not needed but still) where the selector should be something like .something---234-sheet\.tabGeneral-234.theClass. If we do the escaping within JS we would also escape the last dot, which would be wrong.
So that's why I think the correct escaping must happen upon generation of the data attribute in PHP.
Updated by Gerrit Code Review over 7 years ago
Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/53416
Updated by Giannis Economou over 7 years ago
Markus Klein wrote:
Okay everything clear now.
BUT: IMO the fix must be to write a valid selector to the
data-dropzone-targetattribute. Consider the case (maybe currently not needed but still) where the selector should be something like.something---234-sheet\.tabGeneral-234.theClass. If we do the escaping within JS we would also escape the last dot, which would be wrong.
So that's why I think the correct escaping must happen upon generation of the data attribute in PHP.
You are right, I understand.
I assumed that target div id naming is following a specific pattern that is agreed upon already by the way it works (where cases like the one you mention is not possible).
I pushed the javascript code changes you suggested for sake of completeness (regardless of the luck of the patch).
Thank you.
Updated by Giannis Economou over 7 years ago
In any case, since we talk about future scenarios (currently not needed), I don't see how PHP can benefit the escaping situation more that javascript. I mean, if we talk about unknown future scenarios they still would need a code update (but it will be in PHP).
What I understand is that architectural wise you might prefer this, to handle such data preparation cases in PHP. Especially if similar preparations for other javascript data are already done only in PHP.
Updated by Benni Mack almost 7 years ago
- Sprint Focus set to On Location Sprint
Updated by Gerrit Code Review almost 7 years ago
Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/53416
Updated by Gerrit Code Review almost 7 years ago
Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/53416
Updated by Gerrit Code Review almost 7 years ago
Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/53416
Updated by Gerrit Code Review almost 7 years ago
Patch set 1 for branch TYPO3_8-7 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/54825
Updated by Gerrit Code Review almost 7 years ago
Patch set 2 for branch TYPO3_8-7 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/54825
Updated by Giannis Economou almost 7 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 6708d691034633cf02b1a7e32d79072f07339db7.
Updated by Georg Ringer almost 7 years ago
- Related to Bug #83747: "Select & upload files" does not work for FAL in flexform added