1352281 - drag/drop: DataTransfer.types does not list non-string types added via mozSetDataAt()

Status: RESOLVED FIXED

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

Description

[Dave Garrett]

As of Firefox 50, dataTransfer.types does not list "application/x-moz-node" set/get via dataTransfer.mozSetDataAt/mozGetDataAt. This broke drag/drop to reorder user actions in Flagfox (current version 5.1.21). Attempt to drag & drop an item in the Flagfox options only detects the "text/plain" version of the data and thus ends up doing a copy instead of move. However, dataTransfer.mozTypesAt _does_ detect the type and dataTransfer.mozSetDataAt/mozGetDataAt still work. The "types" list is simply not updated to contain the type name. I can thus work around this by checking via mozTypesAt instead, and will switch to checking it if this isn't fixed in time for my next update. (The options menu also has arrow buttons to do reorders, so there's also a user workaround available. Relatively few users create enough custom actions to need this feature, anyway.)

The earliest affected Firefox version is before bug 1298243 landed, so that doesn't appear to be related. (I initially thought the breakage was due to the resulting pointless renaming of a method, however bug 1309970 apparently fixed that with a shim.)

Also tested in current Nightly; still affected there. (testing was done on Windows)

No reduced testcase, as making a clean one would take a fair bit of time, and wouldn't work at all for testing old Firefox versions, due to XPI signing requirements. Testcase via Flagfox is to install version 5.1.21 off of AMO, browse to a page with a flag/icon (e.g. this page), right-click on the flag and select "options", then try to reorder any of the actions in the list via drag/drop. In Firefox 49, the action follows the drag with a dotted line for where to drop. In Firefox 50/52ESR/Nightly, dragging causes a dotted line around the whole actions list box and dropping treats it as a text link/url/bookmark dropped/copied from an external source and thus imports it as a new copy appended to the list. The former is desired behavior; the later is not.

This is chrome-only (I think), so not sure if this is filed in the correct place.

Comment 1

[Andrew Overholt [:overholt]]

Michael has been looking at this stuff and probably knows what we should do here. Will Web Extensions have a similar API?

Comment 2

[Nika Layzell [:nika] (ni? for response)]

(In reply to Andrew Overholt [:overholt] from comment #1)
> Michael has been looking at this stuff and probably knows what we should do
> here. Will Web Extensions have a similar API?

This is a web-visible API which we also use within chrome code. I imagine that this API will still be exposed to Web Extensions.

The problem here is that our mozSetDataAt API supports adding items to the DataTransfer which are not KIND_STRING or KIND_FILE. Internally we call this type KIND_OTHER. I believe that the code in flagFox adds a DOM node to the drag data. 

This was changed in bug 1290688 (part 2) where we started hiding the new image/png file types from DataTransfer.types to match the behavior of chrome IIRC. The check was implemented to only add types to `.types` which are of type KIND_STRING, instead of checking that the type is not KIND_FILE. This is technically standards compliant as there is no way to add a DOM node to a DataTransfer in the standard, but unfortunately broke back-compat with our proprietary moz-* API.

The easiest way to fix this would be to change http://searchfox.org/mozilla-central/rev/b8cce081200129a1307e2a682f121135b5ca1d19/dom/events/DataTransfer.cpp#340 from comparing to `KIND_STRING` to compare against `KIND_FILE`. I don't think this should have negative consequences for standards compliant web pages, so I'll post a patch.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

(In reply to Michael Layzell [:mystor] from comment #2)
> (In reply to Andrew Overholt [:overholt] from comment #1)
> > Michael has been looking at this stuff and probably knows what we should do
> > here. Will Web Extensions have a similar API?
> 
> This is a web-visible API which we also use within chrome code. I imagine
> that this API will still be exposed to Web Extensions.

Correction - we reject all non-system principal consumers from adding non file/string items to DataTransfer. This means that once we're in the webextension world this problem will go away, as webextensions, like web pages, will be forced to either use files or strings exclusively on DataTransfer (Webextensions don't have the system principal). (http://searchfox.org/mozilla-central/rev/b8cce081200129a1307e2a682f121135b5ca1d19/dom/events/DataTransfer.cpp#640-652)

I can still fix this bug for extensions today, but it will not matter once we unship xul extensions. Is this worthwhile doing?

Comment 4

[Andrew Overholt [:overholt]]

Fixing it gives addons like FlagFox a few more releases of working so I'm inclined to say yes (and it's kind of an unintentional regression, right?).

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Attachment: Include KIND_OTHER types in DataTransfer.types

MozReview-Commit-ID: EoqzAydaLea

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8855505 [details] [diff] [review]
Include KIND_OTHER types in DataTransfer.types

Neil is probably a better reviewer for this, so I'm moving the review over to him.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6a130fc4cf5
Include KIND_OTHER types in DataTransfer.types, r=baku

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e6a130fc4cf5