1162050 - Remove draggesture and dragdrop events

Status: RESOLVED FIXED

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

Description

[Neil Deakin]

These events were left in for compatibility. However it's been long enough and we should just remove these events in favour of the standard event names.

draggesture -> dragstart
dragdrop -> drop

dragexit should be left as is since the standard dragleave event fires after the dragenter event and is less useful.

This involves:
 1. Removing the NS_DRAGDROP_GESTURE and NS_DRAGDROP_DRAGDROP constants from BasicEvents.h
 2. Remove any usage of these event name constants.
 3. Remove the code that fires these events from EventStateManager.cpp
 4. Convert or remove any existing usage of draggesture and dragdrop event listeners
 5. Remove toolkit/content/nsDragAndDrop.js

A quick glance suggested there are no usage in Firefox, however there are some in Seamonkey/Thunderbird. These should be converted over, but that doesn't need to be done here.

Comment 1

[Chirag Bhatia]

Hi Neil,

I can work on this bug. Is there anything I need to test after making my changes in the code?

Comment 2

[Neil Deakin]

Hi, thanks for taking this bug. As far as I know, Firefox doesn't use the events so everything should just work there When you have a patch, we can run through our test suites (try server) just to be sure, and, of course, just run Firefox and try some basic drag and drop operations.

We'll want to give a heads up to Thunderbird and Seamonkey and addon developers with a message describing the change since they may still be dependent on these events.

Comment 3

[Chirag Bhatia]

Attachment: 1162050.patch

Hi Neil,

I've made the changes that you asked. Unfortunately, I think I broke the drag and drop functionality within my build with these changes (tried on bookmarks and links on web pages).

There are multiple changes (attached), I hope I've done them right. Would appreciate your help in figuring out what's breaking the drag-drop functionality in my build.

Comment 4

[Neil Deakin]

Great! I'll take a look at the patch and see if I can see anything that stands out that would cause the issue you are seeing.

Comment 5

[Neil Deakin]

Comment on attachment 8610719 [details] [diff] [review]
1162050.patch

>diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp
>   }
>-  case NS_DRAGDROP_GESTURE:
>-    if (Prefs::ClickHoldContextMenu()) {
>-      // an external drag gesture event came in, not generated internally
>-      // by Gecko. Make sure we get rid of the click-hold timer.
>-      KillClickHoldTimer();
>-    }
>-    break;

I think this needs to be changed to NS_DRAGDROP_START, but I think this is an edge case that doesn't happen in practice.
 

>-      WidgetDragEvent gestureEvent(aEvent->mFlags.mIsTrusted,
>-                                   NS_DRAGDROP_GESTURE, widget);
>-      FillInEventFromGestureDown(&gestureEvent);
>-
>-      startEvent.dataTransfer = gestureEvent.dataTransfer = dataTransfer;
>-      startEvent.inputSource = gestureEvent.inputSource = aEvent->inputSource;
>-

This is likely the source of the problems you are seeing. You want to assign the values to the properties of startEvent, but remove the gestureEvent parts.


-      // Dispatch both the dragstart and draggesture events to the DOM. For
+      // Dispatch both the dragstart event to the DOM. For
       // elements in an editor, only fire the draggesture event so that the
       // editor code can handle it but content doesn't see a dragstart.

We should be able to remove the second sentence here that still refers to draggesture. That comment refers to code that no longer exists.


>       if (status != nsEventStatus_eConsumeNoDefault) {
>-        status = nsEventStatus_eIgnore;
>-        EventDispatcher::Dispatch(targetContent, aPresContext, &gestureEvent,
>-                                  nullptr, &status);
>-        event = &gestureEvent;
>+        status = nsEventStatus_eIgnore;        
>       }

You'll also want to remove this whole block. The status is being reset here only because we need to fire both events. Now that only one event is fired, we can leave the status as is.


>-  case NS_DRAGDROP_DROP:
>-    {
>-      // now fire the dragdrop event, for compatibility with XUL
>-      if (mCurrentTarget && nsEventStatus_eConsumeNoDefault != *aStatus) {
>-        nsCOMPtr<nsIContent> targetContent;
>-        mCurrentTarget->GetContentForEvent(aEvent,
>-                                           getter_AddRefs(targetContent));
>-
>-        nsCOMPtr<nsIWidget> widget = mCurrentTarget->GetNearestWidget();
>-        WidgetDragEvent event(aEvent->mFlags.mIsTrusted,
>-                              NS_DRAGDROP_DRAGDROP, widget);
>-
>-        WidgetMouseEvent* mouseEvent = aEvent->AsMouseEvent();
>-        event.refPoint = mouseEvent->refPoint;
>-        if (mouseEvent->widget) {
>-          event.refPoint += mouseEvent->widget->WidgetToScreenOffset();
>-        }
>-        event.refPoint -= widget->WidgetToScreenOffset();
>-        event.modifiers = mouseEvent->modifiers;
>-        event.buttons = mouseEvent->buttons;
>-        event.inputSource = mouseEvent->inputSource;
>-
>-        nsEventStatus status = nsEventStatus_eIgnore;
>-        nsCOMPtr<nsIPresShell> presShell = mPresContext->GetPresShell();
>-        if (presShell) {
>-          presShell->HandleEventWithTarget(&event, mCurrentTarget,
>-                                           targetContent, &status);
>-        }
>-      }
>-      sLastDragOverFrame = nullptr;
>-      ClearGlobalActiveContent(this);
>-      break;

The last two lines here (sLastDragOverFrame = nullptr;  and   ClearGlobalActiveContent(this) should stay as they relate to the drop event, not the dragdrop event.

>diff --git a/editor/libeditor/nsEditorEventListener.h b/editor/libeditor/nsEditorEventListener.h
>--- a/editor/libeditor/nsEditorEventListener.h
>+++ b/editor/libeditor/nsEditorEventListener.h
>@@ -65,18 +65,17 @@ protected:
>   virtual nsresult MouseUp(nsIDOMMouseEvent* aMouseEvent) { return NS_OK; }
>   virtual nsresult MouseClick(nsIDOMMouseEvent* aMouseEvent);
>   nsresult Focus(nsIDOMEvent* aEvent);
>   nsresult Blur(nsIDOMEvent* aEvent);
>   nsresult DragEnter(nsIDOMDragEvent* aDragEvent);
>   nsresult DragOver(nsIDOMDragEvent* aDragEvent);
>   nsresult DragExit(nsIDOMDragEvent* aDragEvent);
>   nsresult Drop(nsIDOMDragEvent* aDragEvent);
>-  nsresult DragGesture(nsIDOMDragEvent* aDragEvent);
>-
>+  

Remove the extra unneeded whitespace change.


OK, so fix up these cases and submit a new patch and we can try it out. Thanks again.

Comment 6

[Chirag Bhatia]

Attachment: 1162050.2.patch

Hi Neil,

Thanks for the help. I made the changes that you asked (attached a new patch file).

After applying the changes, drag and drop seems to work fine for bookmarks and other elements in the browser UI, but I'm still unable to drag and drop links within web pages.

I also keep getting this warning when I try to drag any links within the web pages:

[Child 5253] WARNING: NS_ENSURE_TRUE(node) failed: file /mnt/extgit/mozilla-central/layout/base/nsDocumentViewer.cpp, line 3530

Not sure if this warning is related to the problem I'm facing.

Any ideas what could be causing the problem?

Comment 7

[Neil Deakin]

> [Child 5253] WARNING: NS_ENSURE_TRUE(node) failed: file
> /mnt/extgit/mozilla-central/layout/base/nsDocumentViewer.cpp, line 3530
> 
> Not sure if this warning is related to the problem I'm facing.
>

Not sure. That code is generally used to handle the copy image context menu command, so it shouldn't be related, but it could be.

I'll take a look next week. Thanks!

Comment 8

[Neil Deakin]

Comment on attachment 8613009 [details] [diff] [review]
1162050.2.patch

>       if (initialDataTransfer)
>         initialDataTransfer->SetDropEffectInt(dropEffect);
>     }
>+    sLastDragOverFrame = nullptr;
>+    ClearGlobalActiveContent(this);
>     break;
>-
>-  case NS_DRAGDROP_DROP:
>-    {

The two lines above here will need to go in the NS_DRAGDROP_DROP section. Otherwise, the dragenter/dragexit events won't fire properly. You can see an issue when dragging a link on the tab bar, as the feedback indciator doesn't disappear.

I don't see any issue with dragging and dropping links. Do you have some more specific steps to reproduce? Does it work in a non-e10s window? (Select 'New None-e10s Window' from the File menu)

Otherwise, when you have a new patch we can submit this patch to our 'try' server to run automated tests, and then mark it for a formal review. I'll post a message about this change in the developer mailing lists.

Comment 9

[Chirag Bhatia]

Attachment: 1162050.3.patch

Hi Neil,

I've made the changes you mentioned and attached the patch.

I also tried a build by reverting my changes, and it turns out, the dragging and dropping of links does not work. So the change has nothing to do with my code (either some other bad patch I applied or might be another bug). It does work in a non-e10s window though (even after I applied my patch).

Comment 10

[Neil Deakin]

Comment on attachment 8614777 [details] [diff] [review]
1162050.3.patch

Look good, but I couldn't get the patch to apply cleanly. You likely need to make sure you update (pull or rebase) from the source and fix up any conflicts first.

Comment 11

[Chirag Bhatia]

Hi Neil,

I did an hg qpop, hg rebase, hg pull --rebase, and then hg import 1162050.3.patch and it got imported without a problem.

Am I doing this wrong, or should I be trying something else? Which file did you get the conflict in?

Comment 12

[Neil Deakin]

Attachment: Remove events patch

I got problems in a few files. However, it looks like the issues are minor, so I just fixed them up manually for me. It may be that you have different whitespace or linebreaks or something like that; that sometimes happens when moving patches between platforms. I attached the patch.

Comment 13

[Chirag Bhatia]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=46d5eb5c88b4

Comment 14

[Chirag Bhatia]

Attachment: 1162050.4.patch

Neil,

The tests do not seem to be showing any problems with our code, although you might want to re-check them since this is the first time I've used try server and its automated tests.

I've taken your patch and added a commit message to it.

Comment 15

[Neil Deakin]

Comment on attachment 8616473 [details] [diff] [review]
1162050.4.patch

The try run looks good to me. The patch also looks good, but we should also remove the nsDragAndDrop.js file (hg remove <the file>)

I posted a messaage about this change to the dev-platform list at https://groups.google.com/forum/#!topic/mozilla.dev.platform/thqN2Umpea0

We'll wait a bit to see if anyone has any concerns and then check it in. This should also give some time for the corresponding Thunderbird and Seamonkey bugs to be fixed. Feel free to help out if you'd like.

Thanks for working on this!

Comment 16

[Chirag Bhatia]

Attachment: 1162050.5.patch

I've updated the patch which now includes the removal of the nsDragAndDrop.js file.

Let me know if any other change is required. :)

Comment 17

[Kohei Yoshino]

Posted the site compatibility doc: https://www.fxsitecompat.com/en-US/docs/2015/legacy-non-standard-drag-events-will-be-removed/

Comment 18

[Neil Deakin]

OK, now that all the dependencies are fixed we can check this in. Chirag, are you available to update the patch for checkin?

Comment 19

[Chirag Bhatia]

Yes, Neil. What updates do we need to make?

Comment 20

[Neil Deakin]

The patch doesn't apply anymore, likely mainly because bug 895274 changed the event names from macros to enumerations. Functionally, the patch should be ok. Thanks!

Comment 21

[Jorg K (CEST = GMT+2)]

Hi, can you please finish this or can I take this over? I'd like to see that landed, preferably early in a release cycle, so we can address any regressions. We've already cut over Thunderbird to using the new events, but we want to make sure that no harm is done when this bug here lands.

What needs to be done? Well the event name changed:
Old: NS_DRAGDROP_GESTURE,  New: eLegacyDragGesture
Old: NS_DRAGDROP_START,    New: eDragStart
Old: NS_DRAGDROP_DROP,     New: eDrop
Old: NS_DRAGDROP_DRAGDROP, New: eLegacyDragDrop

This bug is about the legacy events being removed. You need to adapt your patch accordingly.

Comment 22

[Jimmy Wang (:jimicy) - works on e10s stuff]

Attachment: Bug 1162050 - Remove toolkit/content/nsDragAndDrop.js.

Review commit: https://reviewboard.mozilla.org/r/55924/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55924/

Comment 23

[Jimmy Wang (:jimicy) - works on e10s stuff]

Attachment: Bug 1162050 - Remove instances of eLegacyDragGesture and draggesture.

Review commit: https://reviewboard.mozilla.org/r/55926/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55926/

Comment 24

[Jimmy Wang (:jimicy) - works on e10s stuff]

Attachment: Bug 1162050 - Remove use of eLegacyDragGesture from EventStateManager.cpp.

Review commit: https://reviewboard.mozilla.org/r/55928/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55928/

Comment 25

[Jimmy Wang (:jimicy) - works on e10s stuff]

Attachment: Bug 1162050 - Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp.

Review commit: https://reviewboard.mozilla.org/r/55930/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55930/

Comment 26

[Jimmy Wang (:jimicy) - works on e10s stuff]

Attachment: Bug 1162050 - Remove draggesture and eLegacyDragDrop from comments in EventStateManager.cpp/.h.

Review commit: https://reviewboard.mozilla.org/r/55932/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55932/

Comment 27

[Jimmy Wang (:jimicy) - works on e10s stuff]

Attachment: Bug 1162050 - remove instances of eLegacyDragDrop and dragdrop.

Review commit: https://reviewboard.mozilla.org/r/55934/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55934/

Comment 28

[Jorg K (CEST = GMT+2)]

Assigning to the submitter of the patches ;-)

Comment 29

[Neil Deakin]

Comment on attachment 8757500 [details]
Bug 1162050 - remove instances of eLegacyDragDrop and dragdrop.

https://reviewboard.mozilla.org/r/55934/#review52966

Comment 30

[Neil Deakin]

Comment on attachment 8757499 [details]
Bug 1162050 - Remove draggesture and eLegacyDragDrop from comments in EventStateManager.cpp/.h.

https://reviewboard.mozilla.org/r/55932/#review52968

Comment 31

[Neil Deakin]

Comment on attachment 8757498 [details]
Bug 1162050 - Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp.

https://reviewboard.mozilla.org/r/55930/#review52970

Comment 32

[Neil Deakin]

Comment on attachment 8757497 [details]
Bug 1162050 - Remove use of eLegacyDragGesture from EventStateManager.cpp.

https://reviewboard.mozilla.org/r/55928/#review52964

::: dom/events/EventStateManager.cpp:705
(Diff revision 1)
>      // Flush pending layout changes, so that later mouse move events
>      // will go to the right nodes.
>      FlushPendingEvents(aPresContext);
>      break;
>    }
> -  case eLegacyDragGesture:
> +  case eDragStart:

I don't think the code will ever get called, but we could leave it for now.

Comment 33

[Neil Deakin]

Comment on attachment 8757496 [details]
Bug 1162050 - Remove instances of eLegacyDragGesture and draggesture.

https://reviewboard.mozilla.org/r/55926/#review52962

::: dom/events/DataTransfer.cpp:155
(Diff revision 1)
>    , mDragImageY(aDragImageY)
>  {
>    MOZ_ASSERT(mParent);
>    // The items are copied from aItems into mItems. There is no need to copy
>    // the actual data in the items as the data transfer will be read only. The
> -  // draggesture and dragstart events are the only times when items are
> +  // dragstart event is the only times when items are

only time

Comment 34

[Neil Deakin]

Comment on attachment 8757495 [details]
Bug 1162050 - Remove toolkit/content/nsDragAndDrop.js.

https://reviewboard.mozilla.org/r/55924/#review52972

Comment 35

[Jorg K (CEST = GMT+2)]

Sorry, as far as I can see, there are two review issues.

Also, would you mind landing this next week after the branch date on Monday. That will make is easier to address any issues in the same cycle so we wouldn't need uplifts.

Comment 36

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757495 [details]
Bug 1162050 - Remove toolkit/content/nsDragAndDrop.js.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55924/diff/1-2/

Comment 37

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757496 [details]
Bug 1162050 - Remove instances of eLegacyDragGesture and draggesture.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55926/diff/1-2/

Comment 38

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757497 [details]
Bug 1162050 - Remove use of eLegacyDragGesture from EventStateManager.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55928/diff/1-2/

Comment 39

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757498 [details]
Bug 1162050 - Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55930/diff/1-2/

Comment 40

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757499 [details]
Bug 1162050 - Remove draggesture and eLegacyDragDrop from comments in EventStateManager.cpp/.h.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55932/diff/1-2/

Comment 41

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757500 [details]
Bug 1162050 - remove instances of eLegacyDragDrop and dragdrop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55934/diff/1-2/

Comment 42

[Jorg K (CEST = GMT+2)]

Any movement here? Would be nice to land this well before the next branch date, August 1st.

Comment 43

[Jorg K (CEST = GMT+2)]

Have you checked that the patches still apply?

Comment 44

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757495 [details]
Bug 1162050 - Remove toolkit/content/nsDragAndDrop.js.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55924/diff/2-3/

Comment 45

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757496 [details]
Bug 1162050 - Remove instances of eLegacyDragGesture and draggesture.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55926/diff/2-3/

Comment 46

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757497 [details]
Bug 1162050 - Remove use of eLegacyDragGesture from EventStateManager.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55928/diff/2-3/

Comment 47

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757498 [details]
Bug 1162050 - Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55930/diff/2-3/

Comment 48

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757499 [details]
Bug 1162050 - Remove draggesture and eLegacyDragDrop from comments in EventStateManager.cpp/.h.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55932/diff/2-3/

Comment 49

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757500 [details]
Bug 1162050 - remove instances of eLegacyDragDrop and dragdrop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55934/diff/2-3/

Comment 50

[Jimmy Wang (:jimicy) - works on e10s stuff]

(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #43)
> Have you checked that the patches still apply?

If they rebase properly? Yes. I've just pushed the patches rebased on top of the current mozilla-central

Comment 51

[Jorg K (CEST = GMT+2)]

Thanks, so I understand you refreshed/rebased the patches as required.

Comment 52

[Carsten Book [:Tomcat]]

has conflicts now when trying to checkin so with last merges i guess this needs rebasing

Comment 53

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757495 [details]
Bug 1162050 - Remove toolkit/content/nsDragAndDrop.js.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55924/diff/3-4/

Comment 54

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757496 [details]
Bug 1162050 - Remove instances of eLegacyDragGesture and draggesture.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55926/diff/3-4/

Comment 55

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757497 [details]
Bug 1162050 - Remove use of eLegacyDragGesture from EventStateManager.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55928/diff/3-4/

Comment 56

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757498 [details]
Bug 1162050 - Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55930/diff/3-4/

Comment 57

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757499 [details]
Bug 1162050 - Remove draggesture and eLegacyDragDrop from comments in EventStateManager.cpp/.h.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55932/diff/3-4/

Comment 58

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757500 [details]
Bug 1162050 - remove instances of eLegacyDragDrop and dragdrop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55934/diff/3-4/

Comment 59

[Jimmy Wang (:jimicy) - works on e10s stuff]

(In reply to Carsten Book [:Tomcat] from comment #52)
> has conflicts now when trying to checkin so with last merges i guess this
> needs rebasing

Hi Carsten, could you give it a try now? Rebased again. Thanks!

Comment 60

[Jorg K (CEST = GMT+2)]

No action if you don't put "checkin-needed" or NI.

Comment 61

[Carsten Book [:Tomcat]]

Hi Jimmy i get:

unable to find 'editor/libeditor/nsEditorEventListener.h' for patching

when trying to apply the patches. Maybe because of bug 1260651 ?

Comment 62

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757495 [details]
Bug 1162050 - Remove toolkit/content/nsDragAndDrop.js.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55924/diff/4-5/

Comment 63

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757496 [details]
Bug 1162050 - Remove instances of eLegacyDragGesture and draggesture.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55926/diff/4-5/

Comment 64

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757497 [details]
Bug 1162050 - Remove use of eLegacyDragGesture from EventStateManager.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55928/diff/4-5/

Comment 65

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757498 [details]
Bug 1162050 - Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55930/diff/4-5/

Comment 66

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757499 [details]
Bug 1162050 - Remove draggesture and eLegacyDragDrop from comments in EventStateManager.cpp/.h.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55932/diff/4-5/

Comment 67

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757500 [details]
Bug 1162050 - remove instances of eLegacyDragDrop and dragdrop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55934/diff/4-5/

Comment 68

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d7f3aa49d4d
Remove toolkit/content/nsDragAndDrop.js. r=enn
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8fe3acf2e78
Remove instances of eLegacyDragGesture and draggesture. r=enn
https://hg.mozilla.org/integration/mozilla-inbound/rev/f940ad94cc9d
Remove use of eLegacyDragGesture from EventStateManager.cpp. r=enn
https://hg.mozilla.org/integration/mozilla-inbound/rev/11421fcb32b5
Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp. r=enn
https://hg.mozilla.org/integration/mozilla-inbound/rev/af0d7fedc60f
Remove draggesture from comments in EventStateManager.cpp/.h. r=enn
https://hg.mozilla.org/integration/mozilla-inbound/rev/342274a86c0d
remove instances of eLegacyDragDrop and dragdrop. r=enn

Comment 69

[Carsten Book [:Tomcat]]

caused bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=31527433&repo=mozilla-inbound

Comment 70

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e91dc7e8e43
Backed out changeset 342274a86c0d 
https://hg.mozilla.org/integration/mozilla-inbound/rev/03e6db7e26dd
Backed out changeset af0d7fedc60f 
https://hg.mozilla.org/integration/mozilla-inbound/rev/283c5f11c92f
Backed out changeset 11421fcb32b5 
https://hg.mozilla.org/integration/mozilla-inbound/rev/dff37b2bd729
Backed out changeset f940ad94cc9d 
https://hg.mozilla.org/integration/mozilla-inbound/rev/578c5f187aae
Backed out changeset f8fe3acf2e78 
https://hg.mozilla.org/integration/mozilla-inbound/rev/604213dfaeb1
Backed out changeset 2d7f3aa49d4d for bustage in DataTransfer.cpp

Comment 71

[Jorg K (CEST = GMT+2)]

mFileList was removed in bug 906420 here:
https://hg.mozilla.org/mozilla-central/rev/170fc45fb692#l3.199

Comment 72

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757495 [details]
Bug 1162050 - Remove toolkit/content/nsDragAndDrop.js.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55924/diff/5-6/

Comment 73

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757496 [details]
Bug 1162050 - Remove instances of eLegacyDragGesture and draggesture.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55926/diff/5-6/

Comment 74

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757497 [details]
Bug 1162050 - Remove use of eLegacyDragGesture from EventStateManager.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55928/diff/5-6/

Comment 75

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757498 [details]
Bug 1162050 - Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55930/diff/5-6/

Comment 76

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757499 [details]
Bug 1162050 - Remove draggesture and eLegacyDragDrop from comments in EventStateManager.cpp/.h.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55932/diff/5-6/

Comment 77

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757500 [details]
Bug 1162050 - remove instances of eLegacyDragDrop and dragdrop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55934/diff/5-6/

Comment 78

[Carsten Book [:Tomcat]]

seems has problems to apply:

patching file toolkit/content/jar.mn
Hunk #1 FAILED at 0
1 out of 2 hunks FAILED -- saving rejects to file toolkit/content/jar.mn.rej
patch failed to apply
abort: fix up the working directory and run hg transplant --continue

Comment 79

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757495 [details]
Bug 1162050 - Remove toolkit/content/nsDragAndDrop.js.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55924/diff/6-7/

Comment 80

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757496 [details]
Bug 1162050 - Remove instances of eLegacyDragGesture and draggesture.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55926/diff/6-7/

Comment 81

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757497 [details]
Bug 1162050 - Remove use of eLegacyDragGesture from EventStateManager.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55928/diff/6-7/

Comment 82

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757498 [details]
Bug 1162050 - Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55930/diff/6-7/

Comment 83

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757499 [details]
Bug 1162050 - Remove draggesture and eLegacyDragDrop from comments in EventStateManager.cpp/.h.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55932/diff/6-7/

Comment 84

[Jimmy Wang (:jimicy) - works on e10s stuff]

Comment on attachment 8757500 [details]
Bug 1162050 - remove instances of eLegacyDragDrop and dragdrop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55934/diff/6-7/

Comment 85

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f53947a62ad
Remove toolkit/content/nsDragAndDrop.js. r=enn
https://hg.mozilla.org/integration/mozilla-inbound/rev/c010bad64aed
Remove instances of eLegacyDragGesture and draggesture. r=enn
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe482bb7836a
Remove use of eLegacyDragGesture from EventStateManager.cpp. r=enn
https://hg.mozilla.org/integration/mozilla-inbound/rev/158e4fbc8742
Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp. r=enn
https://hg.mozilla.org/integration/mozilla-inbound/rev/35a52f370139
Remove draggesture and eLegacyDragDrop from comments in EventStateManager.cpp/.h. r=enn
https://hg.mozilla.org/integration/mozilla-inbound/rev/48f290424a78
remove instances of eLegacyDragDrop and dragdrop. r=enn

Comment 86

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5f53947a62ad
https://hg.mozilla.org/mozilla-central/rev/c010bad64aed
https://hg.mozilla.org/mozilla-central/rev/fe482bb7836a
https://hg.mozilla.org/mozilla-central/rev/158e4fbc8742
https://hg.mozilla.org/mozilla-central/rev/35a52f370139
https://hg.mozilla.org/mozilla-central/rev/48f290424a78

Comment 87

[Kohei Yoshino]

Updated the site compat doc: https://www.fxsitecompat.com/en-CA/docs/2016/legacy-non-standard-drag-events-have-been-removed/

Comment 88

[Neil Deakin]

Note that dragexit is in the spec now so any comments regarding it should be updated in documentation.

Comment 89

[Kohei Yoshino]

Thanks Neil. Just saw the spec and updated the compat doc accordingly.

https://developers.whatwg.org/dnd.html#event-dnd-dragexit

Comment 90

[Eric Shepherd [:sheppy]]

Pages updated:

https://developer.mozilla.org/en-US/docs/Web/API/Window/ondragdrop
https://developer.mozilla.org/en-US/Firefox/Releases/50

This wasn’t documented anywhere else outside our Archive area.

Comment 91

[Robert Kaiser]

I'm pretty sure this breaks a number of websites out there and make them stop working properly. I myself have noticed drag and drop being broken on NFL.com Fantasy Football (moving players between positions - there is a fallback that works with multiple clicks) and re:dash (editing/rearranging dashboards, no fallback solution available).

Comment 92

[Kohei Yoshino]

Looks like Tweetdeck is also broken. I can confirm that pics can no longer be attached with a drag & drop. https://input.mozilla.org/en-US/dashboard/response/6077371

Comment 93

[Neil Deakin]

Please file bugs for those incompatible sites in the Tech Evangelism component.