The default bug view has changed. See this FAQ.

Remove draggesture and dragdrop events

RESOLVED FIXED in Firefox 50

Status

()

Core
Drag and Drop
RESOLVED FIXED
2 years ago
4 months ago

People

(Reporter: Neil Deakin, Assigned: jimicy, Mentored)

Tracking

({addon-compat, dev-doc-complete, site-compat})

unspecified
mozilla50
addon-compat, dev-doc-complete, site-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(6 attachments, 6 obsolete attachments)

58 bytes, text/x-review-board-request
Neil Deakin
: review+
Details | Review
58 bytes, text/x-review-board-request
Neil Deakin
: review+
Details | Review
58 bytes, text/x-review-board-request
Neil Deakin
: review+
Details | Review
58 bytes, text/x-review-board-request
Neil Deakin
: review+
Details | Review
58 bytes, text/x-review-board-request
Neil Deakin
: review+
Details | Review
58 bytes, text/x-review-board-request
Neil Deakin
: review+
Details | Review
(Reporter)

Description

2 years ago
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

2 years ago
Hi Neil,

I can work on this bug. Is there anything I need to test after making my changes in the code?
Flags: needinfo?(enndeakin)
(Reporter)

Comment 2

2 years ago
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.
Flags: needinfo?(enndeakin)

Comment 3

2 years ago
Created attachment 8610719 [details] [diff] [review]
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.
Flags: needinfo?(enndeakin)
(Reporter)

Comment 4

2 years ago
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.
Flags: needinfo?(enndeakin)
(Reporter)

Updated

2 years ago
Assignee: nobody → chiragbhatia2006
Status: NEW → ASSIGNED
(Reporter)

Comment 5

2 years ago
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

2 years ago
Created attachment 8613009 [details] [diff] [review]
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?
Attachment #8610719 - Attachment is obsolete: true
Flags: needinfo?(enndeakin)
(Reporter)

Comment 7

2 years ago
> [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!
Flags: needinfo?(enndeakin)
(Reporter)

Comment 8

2 years ago
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

2 years ago
Created attachment 8614777 [details] [diff] [review]
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).
Attachment #8613009 - Attachment is obsolete: true
Attachment #8614777 - Flags: review?(enndeakin)
(Reporter)

Comment 10

2 years ago
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.
Attachment #8614777 - Flags: review?(enndeakin) → review+

Comment 11

2 years ago
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?
Flags: needinfo?(enndeakin)
(Reporter)

Comment 12

2 years ago
Created attachment 8615982 [details] [diff] [review]
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.
Flags: needinfo?(enndeakin)
(Reporter)

Updated

2 years ago
Keywords: addon-compat, dev-doc-needed
(Reporter)

Updated

2 years ago
Blocks: 1171979
(Reporter)

Updated

2 years ago
Blocks: 1171980

Comment 13

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

Updated

2 years ago
See Also: → bug 1171980

Comment 14

2 years ago
Created attachment 8616473 [details] [diff] [review]
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.
Attachment #8614777 - Attachment is obsolete: true
Attachment #8616473 - Flags: review?(enndeakin)
(Reporter)

Comment 15

2 years ago
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!
Attachment #8616473 - Flags: review?(enndeakin) → review+

Updated

2 years ago
Keywords: site-compat

Comment 16

2 years ago
Created attachment 8620427 [details] [diff] [review]
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. :)
Attachment #8616473 - Attachment is obsolete: true
Attachment #8620427 - Flags: review?(enndeakin)
(Reporter)

Updated

2 years ago
Attachment #8620427 - Flags: review?(enndeakin) → review+

Updated

2 years ago
Blocks: 1174601
Blocks: 1186516
Posted the site compatibility doc: https://www.fxsitecompat.com/en-US/docs/2015/legacy-non-standard-drag-events-will-be-removed/
(Reporter)

Comment 18

a year ago
OK, now that all the dependencies are fixed we can check this in. Chirag, are you available to update the patch for checkin?
Flags: needinfo?(chiragbhatia2006)

Comment 19

a year ago
Yes, Neil. What updates do we need to make?
Flags: needinfo?(chiragbhatia2006) → needinfo?(enndeakin)
(Reporter)

Comment 20

a year ago
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!
Flags: needinfo?(enndeakin)

Comment 21

11 months ago
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.
Flags: needinfo?(chiragbhatia2006)
Created attachment 8757495 [details]
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/
Created attachment 8757496 [details]
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/
Created attachment 8757497 [details]
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/
Created attachment 8757498 [details]
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/
Created attachment 8757499 [details]
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/
Created attachment 8757500 [details]
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

10 months ago
Assigning to the submitter of the patches ;-)
Assignee: chiragbhatia2006 → jimmyw22
Flags: needinfo?(chiragbhatia2006)
Attachment #8757495 - Flags: review?(enndeakin)
Attachment #8757496 - Flags: review?(enndeakin)
Attachment #8757497 - Flags: review?(enndeakin)
Attachment #8757498 - Flags: review?(enndeakin)
Attachment #8757499 - Flags: review?(enndeakin)
Attachment #8757500 - Flags: review?(enndeakin)
(Reporter)

Comment 29

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

https://reviewboard.mozilla.org/r/55934/#review52966
Attachment #8757500 - Flags: review?(enndeakin) → review+
(Reporter)

Comment 30

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

https://reviewboard.mozilla.org/r/55932/#review52968
Attachment #8757499 - Flags: review?(enndeakin) → review+
(Reporter)

Comment 31

10 months ago
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
Attachment #8757498 - Flags: review?(enndeakin) → review+
(Reporter)

Comment 32

10 months ago
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.
Attachment #8757497 - Flags: review?(enndeakin) → review+
(Reporter)

Comment 33

10 months ago
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
Attachment #8757496 - Flags: review?(enndeakin) → review+
(Reporter)

Comment 34

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

https://reviewboard.mozilla.org/r/55924/#review52972
Attachment #8757495 - Flags: review?(enndeakin) → review+

Updated

10 months ago
Attachment #8620427 - Attachment is obsolete: true

Updated

10 months ago
Attachment #8615982 - Attachment is obsolete: true

Updated

10 months ago
Keywords: checkin-needed

Comment 35

10 months ago
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.
Flags: needinfo?(kohei.yoshino)
Keywords: checkin-needed

Updated

10 months ago
Flags: needinfo?(kohei.yoshino)
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/
Attachment #8757495 - Attachment description: MozReview Request: Bug 1162050 - Remove toolkit/content/nsDragAndDrop.js → Bug 1162050 - Remove toolkit/content/nsDragAndDrop.js
Attachment #8757496 - Attachment description: MozReview Request: Bug 1162050 - Remove instances of eLegacyDragGesture and draggesture → Bug 1162050 - Remove instances of eLegacyDragGesture and draggesture
Attachment #8757497 - Attachment description: MozReview Request: Bug 1162050 - Remove use of eLegacyDragGesture from EventStateManager.cpp → Bug 1162050 - Remove use of eLegacyDragGesture from EventStateManager.cpp
Attachment #8757498 - Attachment description: MozReview Request: Bug 1162050 - Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp → Bug 1162050 - Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp
Attachment #8757499 - Attachment description: MozReview Request: Bug 1162050 - Remove draggesture from comments in EventStateManager.cpp/.h → Bug 1162050 - Remove draggesture from comments in EventStateManager.cpp/.h
Attachment #8757500 - Attachment description: MozReview Request: Bug 1162050 - remove instances of eLegacyDragDrop and dragdrop → Bug 1162050 - remove instances of eLegacyDragDrop and dragdrop
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 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 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 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 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

9 months ago
Any movement here? Would be nice to land this well before the next branch date, August 1st.
Flags: needinfo?(enndeakin)
Keywords: checkin-needed

Comment 43

9 months ago
Have you checked that the patches still apply?
Flags: needinfo?(enndeakin)
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 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 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 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 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 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/
(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

9 months ago
Thanks, so I understand you refreshed/rebased the patches as required.
has conflicts now when trying to checkin so with last merges i guess this needs rebasing
Flags: needinfo?(jimmyw22)

Updated

9 months ago
Keywords: checkin-needed
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 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 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 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 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 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/
(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!
Flags: needinfo?(jimmyw22)

Comment 60

9 months ago
No action if you don't put "checkin-needed" or NI.
Keywords: checkin-needed
Hi Jimmy i get:

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

when trying to apply the patches. Maybe because of bug 1260651 ?
Flags: needinfo?(jimmyw22)

Updated

9 months ago
Keywords: checkin-needed
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 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 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 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 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 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

9 months ago
Pushed by cbook@mozilla.com:
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
caused bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=31527433&repo=mozilla-inbound

Comment 70

9 months ago
Backout by cbook@mozilla.com:
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

9 months ago
mFileList was removed in bug 906420 here:
https://hg.mozilla.org/mozilla-central/rev/170fc45fb692#l3.199
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/
Attachment #8757499 - Attachment description: Bug 1162050 - Remove draggesture from comments in EventStateManager.cpp/.h → Bug 1162050 - Remove draggesture and eLegacyDragDrop from comments in EventStateManager.cpp/.h
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 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 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 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 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/
Flags: needinfo?(jimmyw22)
Keywords: checkin-needed
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
Flags: needinfo?(jimmyw22)
Keywords: checkin-needed
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/
Attachment #8757495 - Attachment description: Bug 1162050 - Remove toolkit/content/nsDragAndDrop.js → Bug 1162050 - Remove toolkit/content/nsDragAndDrop.js.
Attachment #8757496 - Attachment description: Bug 1162050 - Remove instances of eLegacyDragGesture and draggesture → Bug 1162050 - Remove instances of eLegacyDragGesture and draggesture.
Attachment #8757497 - Attachment description: Bug 1162050 - Remove use of eLegacyDragGesture from EventStateManager.cpp → Bug 1162050 - Remove use of eLegacyDragGesture from EventStateManager.cpp.
Attachment #8757498 - Attachment description: Bug 1162050 - Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp → Bug 1162050 - Remove the dragdrop event code that is not related to the drop event in from EventStateManager.cpp.
Attachment #8757499 - Attachment description: Bug 1162050 - Remove draggesture and eLegacyDragDrop from comments in EventStateManager.cpp/.h → Bug 1162050 - Remove draggesture and eLegacyDragDrop from comments in EventStateManager.cpp/.h.
Attachment #8757500 - Attachment description: Bug 1162050 - remove instances of eLegacyDragDrop and dragdrop → Bug 1162050 - remove instances of eLegacyDragDrop and dragdrop.
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 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 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 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 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/
Flags: needinfo?(jimmyw22)
Keywords: checkin-needed

Comment 85

9 months ago
Pushed by cbook@mozilla.com:
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
Keywords: checkin-needed

Comment 86

9 months ago
bugherder
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
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Updated the site compat doc: https://www.fxsitecompat.com/en-CA/docs/2016/legacy-non-standard-drag-events-have-been-removed/
(Reporter)

Comment 88

9 months ago
Note that dragexit is in the spec now so any comments regarding it should be updated in documentation.
Thanks Neil. Just saw the spec and updated the compat doc accordingly.

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

Updated

8 months ago
Depends on: 1286902
Depends on: 1291500
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.
Keywords: dev-doc-needed → dev-doc-complete

Comment 91

5 months ago
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).
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
(Reporter)

Comment 93

4 months ago
Please file bugs for those incompatible sites in the Tech Evangelism component.
You need to log in before you can comment on or make changes to this bug.