932520 - Defect - HTML5 drag and drop does not work in metroFx

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Browser, defect, P2)

Description

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Based on bug 881989 comment 9 it appears that we do not support HTML5 drag and drop, and it's causing issues in gmail and outlook.com

Comment 1

[Rodrigo Silveira [:rsilveira] [:rodms]]

Test site: http://html5demos.com/drag

Comment 2

[Rodrigo Silveira [:rsilveira] [:rodms]]

Adjusting point estimate to p=5.

Comment 3

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: html5dnd.zip

Code from html5 with logging.

Comment 4

[Rodrigo Silveira [:rsilveira] [:rodms]]

Assigning to Tim who

Comment 5

[Rodrigo Silveira [:rsilveira] [:rodms]]

... kindly offered to take this as he's been working on DnD on desktop recently.

The native drag and drop method is returning DRAGDROP_ACTION_NONE at http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsDragService.cpp#277. Tim suspects we need to hook up drop target on our window.

Comment 6

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: 932520.patch

Tested this patch on the following:
  - HTML5 drag and drop (used the html5rocks demo)
  - Dragging from the URL bar to text input
  - Dragging from MetroFx to desktop apps
  - (!) Dragging from desktop apps to MetroFx

Only the last item (marked with a '!') didn't seem to work properly. I'll investigate, but we may want to just land this patch and file a new bug. Will post back to this bug when I've investigated.

Comment 7

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Comment on attachment 831934 [details] [diff] [review]
932520.patch

I did some further testing using some of the pre-installed immersive apps: They don't seem to support drag and drop to/from other applications at all.

In fact, dragging over any immersive app (besides Firefox, presumably) causes the cursor to change to the "you can't drop here" cursor permanently, regardless of where you move the drag later and whether you can in fact perform a drop.

Let's get this change in, and I'll file follow-ups for the following:
  - Support dragging from other apps to MetroFx
  - Dragging over a metro app causes the cursor to change permanently to the "drop disabled" cursor

Note that we'll have to investigate each of those issues since it might be possible that there are OS limitations causing them.

Comment 8

[Jim Mathies [:jimm]]

Comment on attachment 831934 [details] [diff] [review]
932520.patch

looks good!

Comment 9

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

I'm seeing a mochitest-metro failure with this patch applied. I'll figure out what's going on there before pushing

Comment 10

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

No test failure building with only this patch applied (I had had other patches applied previously). I'll land on fx-team whenever it opens back up

Comment 11

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Built and re-tested today (ran mochitest-metro locally), but fx-team closed while I was doing that. Will land when it's open again.

Comment 12

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Pushed to inbound:
  https://hg.mozilla.org/integration/fx-team/rev/832f40264880

Comment 13

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

(In reply to Tim Abraldes [:TimAbraldes] [:tabraldes] from comment #12)
> Pushed to inbound:

Err, I meant "pushed to fx-team"

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/832f40264880

Comment 15

[Manuela Muntean [Away]]

While testing this for iteration #19, with latest Nightly from 2013-12-01, on Win 8.1 Pro 64-bit:

1) http://html5demos.com/drag works the same in Desktop and Metro mode

2) I can still reproduce the issue from bug 881989 comment 0, with both Gmail and Outlook

Any thoughts/suggestions? Thanks!

Comment 16

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

bug 881989 and bug 906034 are still open because they were not fixed by the patch in this bug. To verify this bug, just test the html5 drag and drop demos. Thanks!

Comment 17

[Manuela Muntean [Away]]

Verified as fixed based on comment 15 and comment 16. Thanks Tim!