Closed Bug 1812611 Opened 1 year ago Closed 1 year ago

Drag and Drop dataTransfer.files populated when some types of Image element dropped

Categories

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

Firefox 109
defect

Tracking

()

VERIFIED FIXED
111 Branch
Tracking Status
firefox-esr102 --- unaffected
firefox109 --- wontfix
firefox110 + verified
firefox111 + verified

People

(Reporter: services, Assigned: tschuster, NeedInfo)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/109.0

Steps to reproduce:

After update from Firefox 108 to 109, a drop action between elements in a Vaadin web application no longer worked. After investigation, it appears the problem is application is interpreting drop as a FileDrop (file upload) rather than a regular DOM element drop, because the dataTransfer object is populating files array. See example jsfiddle:
https://jsfiddle.net/ksgvyhjm/2/

Issue is happening in Firefox on Linux and Windows desktops.

Actual results:

Dragging an image within the page to a drop target triggers a drop event containing files in the dataTransfer.files array, even though there is no actual usable file path. In the example, the dragged image with a jpeg source is actually reported as as having filename "image.png" and type "image/png", neither of which is actually true.
Dragging the other image on the page does not populate the files array.
Dragging a file from the OS file system does report the file information as expected.

Expected results:

Dragging image elements within the page to the drop target within the same page should not be populating the dataTransfer.files list.

Component: Untriaged → DOM: Copy & Paste and Drag & Drop
Product: Firefox → Core

This is an intentional change from bug 1437126.

Regressed by: 1437126

Is there a way to define a draggable img so that it will not be added to the Files as a pseudo-file?

For example, the breaking case on the existing application is where a visual thumbnail/icon image is being dragged to a grid control as part of a database record edit form. The target should not respond to drops of files from the OS in this case, and would not expect the dragged visual element to be reported as a file.

Is there a way to define a draggable img so that it will not be added to the Files as a pseudo-file?

Not really. You could prevent it from being dragged in the first place though. You can also check event.dataTransfer.mozSourceNode !== null on the drop handler.

We might consider not putting the image data into DataTransfer for a drag&drop on the same. That seems like a hack to me though, when it works (differently) dragging across from another tab.

Agree that treating image drag from same page differently from other tab would be a terrible hack.

Unfortunately, the 1437126 fix has broken apps built on top of Vaadin 8 drag and drop, which may be a sizeable code base, and I fear the easiest solution is people will just switch to using Chrome which doesn't seem to do the populating Files for dragged IMG.

needinfoing evilpie for ideas of how to not break Vaadin 8.

It seems to me that Vaadin 8 itself has been EOLed and apps are supposed to upgrade. As these things go, it seems quite possible that the old version will continue to be deployed out there after EOL.

Flags: needinfo?(evilpies)

I tried to come up with something that let's us detect same page drags, but somehow that turned out to be more difficult than I expected. DataTransfer::mIsExternal doesn't seem to work for this. The closest match I have found is nsContentUtils::GetDragSession() returning null? I produce a somewhat ugly patch for this. Maybe Nika has some ideas.

Flags: needinfo?(evilpies) → needinfo?(nika)
See Also: → CVE-2023-25741

FWIW, I am working around this issue by constructing the draggable image as an empty div with the background-image css set to the required image. That seems to be the only semantic way that I can declare my 'intent' that a drop target should not interpret the dragged element as an image file.

The W3C spec seems to permit the Files array in the dataTransfer to refer to actual OS files or dynamically created / in-memory objects, as long as they have a 'filename' and a data stream.

The use case addressed in 1437126 appears just as valid as the issue highlighted here.. It is a benefit to the user to be able to drag an image element as if it were a file in cases where they want to submit that image data to a web form.

It feels like the ideal solution would be some way to declare that a draggable element should/should not be presented as a file (i.e. something cleaner than using background image approach).. perhaps there is a way to do this already in the spec but I have missed it?

Thank you Tom S. I appreciate your prompt and earnest efforts with this issue.

(In reply to Tom S [:evilpie] from comment #6)

I tried to come up with something that let's us detect same page drags, but somehow that turned out to be more difficult than I expected. DataTransfer::mIsExternal doesn't seem to work for this. The closest match I have found is nsContentUtils::GetDragSession() returning null? I produce a somewhat ugly patch for this. Maybe Nika has some ideas.

I'm not sure I love the approach of making same-page drags dramatically different from cross-page drags in e.g. what values are added to the drag session.

I'm a bit curious what behaviour other browsers like chrome have in this situation around dragging an image element. It seems desirable that we align with their behaviour in general to be as compatible as possible.

Flags: needinfo?(nika)

Testing drop event using the original JSFiddle example in Chrome 109.0.5414.74 :

Drag the corgi IMG on same page to target:
dataTransfer.files is empty

Drag the corgi IMG from a different Chromium tab:
dataTransfer.files[0]
lastModified: 1675200212654
lastModifiedDate: Tue Jan 31 2023 16:23:32 GMT-0500 (Eastern Standard Time) {}
name: "Pembroke-Welsh-Corgi-320x240.jpg"
size: 53026
type: "image/jpeg"
webkitRelativePath: ""

Drag an actual copy of the corgi jpg on disk from OS file manager (modifiedDate is on-disk file time):
dataTransfer.files[0]
lastModified: 1675199955773
lastModifiedDate: Tue Jan 31 2023 16:19:15 GMT-0500 (Eastern Standard Time) {}
name: "Pembroke-Welsh-Corgi-320x240.jpg"
size: 53026
type: "image/jpeg"
webkitRelativePath: ""

Drag the corgi IMG to Chrome from the page opened in Firefox (size reported as 0):

lastModified: 1675200413596
lastModifiedDate: Tue Jan 31 2023 16:26:53 GMT-0500 (Eastern Standard Time) {}
name: "Pembroke-Welsh-Corgi-320x240.jpeg"
size: 0
type: "image/jpeg"
webkitRelativePath: ""

Assignee: nobody → tschuster
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

We didn't support for dragging images as files for almost 10 years, so probably nobody depends on this terribly. This patch should also be easy to uplift to fix the other regressions.

Set release status flags based on info from the regressing bug 1437126

Since Firefox 109, access to the DataTransfer#files became extremely slow/expensive. This was reported by CKEditor 5 user as a regression in https://github.com/ckeditor/ckeditor5/issues/13366.

(In reply to j.niegowski from comment #14)

Since Firefox 109, access to the DataTransfer#files became extremely slow/expensive. This was reported by CKEditor 5 user as a regression in https://github.com/ckeditor/ckeditor5/issues/13366.

Thanks for the report. Please file a new bug for this (in the same component as this one) as that will make it easier to track multiple regressions.

Flags: needinfo?(j.niegowski)
Pushed by tschuster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/eb43450f5957
Disable image dragging as files by default again. r=nika
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 111 Branch
Attachment #9314699 - Attachment is obsolete: true

Please request uplift to our last beta, thanks!

Flags: needinfo?(tschuster)

Comment on attachment 9315376 [details]
Bug 1812611 - Disable image dragging as files by default again. r?nika

Beta/Release Uplift Approval Request

  • User impact if declined: Performance, usability issues on different websites. Other regressions.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): We disable a new feature that was only on in one release.
  • String changes made/needed:
  • Is Android affected?: Unknown
Flags: needinfo?(tschuster)
Attachment #9315376 - Flags: approval-mozilla-beta?

Comment on attachment 9315376 [details]
Bug 1812611 - Disable image dragging as files by default again. r?nika

Approved for our last 110 beta, thanks.

Attachment #9315376 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Reproduced the initial issue described in comment 0 using an old Nightly build from 2023-01-26, verified using the test case from comment 0 that this is no longer an issue using latest Nightly build from today and latest Beta build 110.0b9 across platforms (Windows 10, macOS 13 and Ubuntu 22.04).

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: