497498 - drag-leave signals are not received due to processing X events before returning from the drag-motion handler

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Karl Tomlinson (:karlt)]

On receiving an XdndPosition event (or similar for another protocol) GTK sends
a drag-motion signal to the appropriate widget and waits for the signal
handler to return indicating whether the widget will handle drag events.

Mozilla runs arbitrary JavaScript during handling the drag-motion signal which
often starts a nested event loop.

During the nested event loop, GTK often processes the subsequent XdndLeave
event, but as the widget has not yet indicated interest in drag events
(through the drag-motion handler), GTK does not send a drag-leave signal to
the widget.

A common cause of nested event loops could be addressed through bug 497494,
but I assume the JavaScript can start other event loops through an alert or
what else?

Comment 1

[Karl Tomlinson (:karlt)]

As well as the increased CPU usage, this is also noticeable in that once this happens no drags (tabs, text selections, ...) can be initiated.

Comment 2

[Karl Tomlinson (:karlt)]

The right thing to do here is probably to reply to the drag-motion signal immediately, after scheduling an event to dispatch the NS_DRAGDROP_OVER and NS_DRAGDROP_ENTER.  That may need some accounting to cancel the scheduled event if the drag-leave signal is received on the same widget.

Comment 3

[Karl Tomlinson (:karlt)]

Since bug 545714 landed, the nested event loop of bug 497494 is always being run on the first drag over, so this is much easier to trigger.

One set of STR is:

1. Run Firefox with NSPR_LOG_MODULES=nsDragService:5,WidgetDrag:5 in the
   environment.

2. Drag something from another app quickly across the corner of about:blank.

3. Repeat 2 until the log spew continues without drag motion.

The impact on the user is that, after this starts, no drags can be initiated, and the app fires a timer every 100ms, increasing CPU use.

Comment 4

[Karl Tomlinson (:karlt)]

I'll attach a series of patches.  Most are moving things around or simplifying things to minimize the size of the final patch in the series.  Some are corrections to behaviour that seemed appropriate to do here in this bug because otherwise I would have been designing new code to implement the old wrong behaviour.  For parts that alter interaction with Gecko drag code or the DOM, I'll request review from enndeakin.  For parts that are specific to nsWindow and GTK interaction, I'll request review from roc.

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: part 1: Only dispatch drop events when canDrop has been set via dragover and only indicate successful drop when the drop is sent

Only dispatch "drop" events when canDrop has been set via "dragover" and only indicate successful drop when the drop is sent.

Dispatching "dragleave" instead of "drop" when canDrop is false is consistent
with http://hg.mozilla.org/mozilla-central/annotate/a05e91710adb/widget/src/cocoa/nsChildView.mm#l5795
and, while canDrop is consistent with dragAction, this is consistent with 
http://www.whatwg.org/specs/web-apps/current-work/multipage/dnd.html#drag-and-drop-processing-mode

(I can't see that we actually track whether the data was successfully fetched
during drop so the "success" value for gdk_drop_finish can still sometimes be
true when it should not be.)

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: part 2: no need to explicitly dispatch NS_DRAGDROP_ENTER as this happens during NS_DRAGDROP_OVER

Comment 7

[Karl Tomlinson (:karlt)]

In part 2, we can simplify things a bit by leaving NS_DRAGDROP_ENTER handling to the event state manager.

nsEventStateManager::GenerateDragDropEnterExit (called from
nsEventStateManager::PreHandleEvent for NS_DRAGDROP_OVER events) will dispatch
an NS_DRAGDROP_ENTER event.

This dispatch differs from the NS_DRAGDROP_ENTER processing in that
nsEventStateManager::PostHandleEvent will not be invoked to SetDragAction,
SetCanDrop or SetOnlyChromeDrop on the nsIDragSession.  dragAction and canDrop
get reset before the DRAGDROP_OVER event anyway and I assume onlyChromeDrop is
only needed for the drop (which will not happen until after the DRAGDROP_OVER.

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: part 3: don't set a drag context with the new widget for a leave event on the old widget

This code is going to be rearranged in the final patch in the series, but I'm
requesting review on this logic change separately. 

The comment "set this now before any of the drag enter or leave events happen"
was in the initial port from GTK1 in bug 121251.  With this change we still
set the context before enter events (during _OVER) but after leave events.

With this patch, during the leave events there will be no context from which
nsIDragSession::GetData or GetNumDropItems or isDataFlavorSupported can fetch
information during the leave event (for an external drag).  That seems
logically appropriate to me, because once the drag has passed over an element
it is clearly not intended for that element.  This was already the case for
_LEAVE events sent after a native drag-leave signal; this patch simply makes
the behavior consistent for synthetic _LEAVE events generated during
CheckNeedDragLeave().

Comment 9

[Karl Tomlinson (:karlt)]

Attachment: part 4: fire NS_DRAGDROP_DRAG at source even before synthetic NS_DRAGDROP_OVER events

Later in the series, this code will be shared with the drag-motion code. 

For consistency with regular dragover events, a drag event should be fired at
the source.

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: part 5: move gdk_drag_status code from nsDragService to drag-motion signal handler

gdk_drag_status() is associated with the drag-motion signal, not nsDragService.
I think this makes what is happening more explicit.
I've left the nsIDragSessionGTK interface intact.  Now, we just don't do anything in response to the notifications.

Comment 11

[Karl Tomlinson (:karlt)]

Attachment: part 6: use event and widget coordinates for drop point instead of new cursor position

gdk_display_get_pointer synchronously gets the current pointer position, but
we want the position where the drop happened.  With all the inter-process
communication during drags, the pointer may well have moved well away from the
drop location.  In the final patch in this series, the _DROP event is
scheduled off the event loop adding to the issues.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 512363 [details] [diff] [review]
part 5: move gdk_drag_status code from nsDragService to drag-motion signal handler

I presume we need a followup patch to remove the unused methods from nsIGTKDragSession?

Comment 13

[Karl Tomlinson (:karlt)]

Attachment: part 6b: use event coordinates for source drag end point instead of new cursor position

This patch doesn't /need/ to be in this series, but part 6 sets the drag end point for drags ending in the Gecko app.  This is the corresponding fix for drags starting in Gecko but ending outside the Gecko app.

Comment 14

[Karl Tomlinson (:karlt)]

Attachment: part 7: use a helper function for dispatching destination drag events

Comment 15

[Karl Tomlinson (:karlt)]

Attachment: part 8: split drag event dispatch code from GTK signal handling methods

Comment 16

[Karl Tomlinson (:karlt)]

Attachment: part 9: rename UpdateDragStatus to UpdateDragAction

Comment 17

[Karl Tomlinson (:karlt)]

Attachment: part 10: use nsIDragSession::getCurrentSession to remove sIsDraggingOutOf

This implementation of DragInProgress, added for bug 179078, is more convenient for this patch series.

Comment 18

[Karl Tomlinson (:karlt)]

Attachment: part 11: move early drag end code to drag_motion_event_cb and correct return value

OnDragMotionEvent, etc. won't exist after the next patch.

Comment 19

[Karl Tomlinson (:karlt)]

Attachment: part 12: schedule event dispatch in response to GTK drag signals to avoid running the event loop at unexpected times

This is the key fix for this bug.

The DragEventDispatcher comment gives an overview and most of the reasoning.

Two changes included here that didn't make sense to separate out into earlier patches are:

gtk_drag_finish() is what is required after a GTK drag-drop signal.
gdk_drop_finish() was OK for Xdnd but does not send the right events for Motif
drops.

We don't need to send another _OVER event before the _DROP if we have already
sent one exactly the same.

Comment 20

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/try/annotate/8a1e416fa877/widget/src/gtk2/nsWindow.cpp
can be used to see the final state of nsWindow.cpp.
(That version is missing some comment changes though.)

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+    nsRefPtr<nsWindow> mCurrentWindow;
+    nsRefPtr<nsWindow> mPendingWindow;
+    nsCountedRef<GdkDragContext> mDragContext;
+    nsIntPoint mCurrentWindowPoint;
+    nsIntPoint mPendingWindowPoint;
+    nsIntPoint mRootDropPoint;
+    guint mTime;
+    DragStatus mScheduledStatus;

Can we have some comments about what these mean?

I wonder whether we should take this for FF4. It looks very risky.

Comment 22

[Karl Tomlinson (:karlt)]

(In reply to comment #3)
> 2. Drag something from another app quickly across the corner of about:blank.

> The impact on the user is that, after this starts, no drags can be initiated,
> and the app fires a timer every 100ms, increasing CPU use.

The fix for bug 495343 means that the timer no longer fires every 100ms.

Without the patches here though, I still sometimes find myself unable to drag anything in Firefox because I've dragged something completely unrelated across the desktop which happened to pass over Firefox.  I can resolve this by dragging something across Firefox again but slowly.

Comment 23

[Karl Tomlinson (:karlt)]

Attachment: part 12: schedule event dispatch in response to GTK drag signals to avoid running the event loop at unexpected times v1.1

Added documentation for member variables.

Comment 24

[Neil Deakin]

With these patches, and the drop doesn't occur, I get a dragleave event occurring after the dragend event.

Comment 25

[Karl Tomlinson (:karlt)]

Thanks Neil.  I assume that's because we get the drag-end signal when we reply to drag-drop before sending the NS_DRAGDROP_EXIT.  I'll need to think about that.

Comment 26

[Karl Tomlinson (:karlt)]

Attachment: dropless dragleave/dragend testcase

Drag div A and drop onto div B, which doesn't accept the drop.
Expected order of events is dragstart, dragenter, dragleave, dragend.

Without the patches here, the dragleave does not arrive until after a second
drag starts.

43:373 DIV: dragstart client:430,32 screen:983,309
43:699 DIV: dragenter client:436,68 screen:989,345
45:364 DIV: dragend client:0,0 screen:983,388
46:538 DIV: dragstart client:385,43 screen:938,320
46:573 DIV: dragleave client:386,54 screen:939,331
46:635 DIV: dragenter client:388,68 screen:941,345
47:356 DIV: dragend client:0,0 screen:945,391

Which the patches here, the dragleave arrives soon after the dragend.

51:897 DIV: dragstart client:408,51 screen:554,292
51:948 DIV: dragenter client:410,81 screen:556,322
52:547 DIV: dragend client:0,0 screen:558,361
52:552 DIV: dragleave client:412,120 screen:558,361
53:747 DIV: dragstart client:383,45 screen:529,286
53:893 DIV: dragenter client:384,76 screen:530,317
54:493 DIV: dragend client:0,0 screen:533,362
54:499 DIV: dragleave client:387,121 screen:533,362

This is an improvement on the current situation, but still not quite right.
I'll look into what is happening.

Comment 27

[Karl Tomlinson (:karlt)]

I filed Bug 634719 on the order of the dragleave event as the cause is not related to patches here.

Comment 28

[Karl Tomlinson (:karlt)]

Attachment: part 12: schedule event dispatch in response to GTK drag signals to avoid running the event loop at unexpected times v1.2

Changed some word usage to distinguish "tasks" that are run on the main loop from "events" that are dispatched from widget code.  The signal/task/event distinction is helpful.

Comment 29

[Karl Tomlinson (:karlt)]

However, fixing Bug 634719 involves co-ordinating source and destination events, and moving DragEventDispatcher onto nsDragService could make that easier.

Comment 30

[Neil Deakin]

The patches here look ok, but the dragleave issue means that my manual dragdrop tests fail, so I'm not absolutely certain there isn't some other issue.

Comment 31

[Karl Tomlinson (:karlt)]

Thanks for looking, Neil.
I'm going to look at moving the new code to nsDragService (though I'm not sure I'll being doing that soon).  At that point it should be relatively easy to add code to coordinate dragleave and dragend.  It sounds like that will help with your tests, which would provide reassurance, so that would be the way to go here.

Comment 32

[Neil Deakin]

Karl, are you still plans here or can I clear the review requests for now?

Comment 33

[Karl Tomlinson (:karlt)]

The review requests that I've left here are for patches that are still going to be stages in the new plan as of comment 31.

I think they are mostly correctness or simplification patches, and they can land before the new work is done, but if you'd like to wait to test the whole series when I've written the patches for the new plan, then that is fine too.

Comment 34

[Karl Tomlinson (:karlt)]

Most of the patches here are still relevant apart from minor merge collisions with the PRBool-to-bool and GTK3 changes.  I'll upload patches that have more changes than that.  The easiest way to get the full updated and merged series for this bug and bug 634719 is from the branch with head https://hg.mozilla.org/try/rev/f65485cf4e20 (i.e. hg pull -r f65485cf4e20 https://hg.mozilla.org/try).

Comment 35

[Karl Tomlinson (:karlt)]

Attachment: part 5: use nsDragService::SetCanDrop instead of nsIDragSessionGTK::StartDragMotion to reset before NS_DRAGDROP_OVER

With event dispatch moving to nsDragService, it no longer makes sense to move the gdk_drag_status code to nsWindow so I've dropped that code from the patch.
I've kept the changes beginning migration away from nsIDragSessionGTK as subsequent reviewed patches depend on this.  Carrying forward review.

Comment 36

[Karl Tomlinson (:karlt)]

Attachment: part 12a: prevent GetTargetDragData (and IsTargetContextList) from being called without a target GtkWidget

Part 9 will change and move to after the bulk of the changes.

Part 12 will change to an implementation on nsDragService.

This patch is new.  There is an existing bug in that mTargetWidget and mTargetDragContext can be inappropriately used when null, if some of these public methods get called at the wrong times.  The new code in the next patch will also add the possibility that mTargetWidget is null if a target window is destroyed, so including the null check here.

Comment 37

[Karl Tomlinson (:karlt)]

Additional comment on part 12a:
mTargetDragContext will still always be non-null when mTargetWidget is non-null.

Comment 38

[Karl Tomlinson (:karlt)]

Attachment: part 12b: schedule event dispatch in response to GTK drag target signals to avoid running the event loop at unexpected times

This is the bulk of the old part 12 with DragEventDispatcher replaced with an implementation on nsDragService.

See comment 19 for a description of the more minor behaviour changes introduced here.

There are a couple of things for which I felt I didn't find an elegant solution:

1. The StartDragSession and EndDragSession both happen to work on the same
nsDragService, I assume because the first do_GetService call must hold onto an
extra reference to the nsDragService until shutdown.  Using that knowledge we
could provide a static method on nsDragService to return a pointer to the
singleton, but it would look precarious copying the raw pointer from the
nsCOMPtr and then releasing when the nsCOMPtr goes out of scope.  As this patch is we end up with the do_GetService/static_cast code splattered around.  It's a step in the direction on deCOMtamination, but barely.  I don't know if there's a more elegant way to have a singleton accessible both through XPCOM and directly.

2. We can't have multiple definitions of nsAutoRefTraits template specializations, which makes using nsAutoRef tricky in headers.  While this patch doesn't introduce any conflicts, the potential arises once these specializations are in headers, so the HAVE_NSAUTOREFTRAITS_* preprocessor symbols are trying to establish a convention to resolve these problems.  I guess we could try to put all of these in a global header file.  There aren't many to put in there right now, but let me know if you'd prefer that.

Comment 39

[Karl Tomlinson (:karlt)]

Attachment: part 13: move drag event dispatch logic from nsWindow to nsDragService

This logic is now simpler on nsDragService and with that the inter-file interface is a little smaller.

Comment 40

[Karl Tomlinson (:karlt)]

Attachment: part 14: move nsWindow::UpdateDragStatus to nsDragService::UpdateDragAction

This replaces the old part 9 and now occurs later in the series.

Again things are simpler with this on nsDragService.

Comment 41

[Karl Tomlinson (:karlt)]

Attachment: part 14: move nsWindow::UpdateDragStatus to nsDragService::UpdateDragAction

(right patch this time)

Comment 42

[Karl Tomlinson (:karlt)]

Attachment: part 15: remove nsIDragSessionGTK

This interface is Copyright (C) 1998.  I don't think we need XPCOM for this, but let me know if you prefer the type-safety it offers over static_cast.

Comment 43

[Neil Deakin]

Comment on attachment 607425 [details] [diff] [review]
part 14: move nsWindow::UpdateDragStatus to nsDragService::UpdateDragAction

>+nsDragService::UpdateDragAction()
>+{
>+    // This doesn't look right.  dragSession.dragAction is used by
>+    // nsContentUtils::SetDataTransferInEvent() to set the initial
>+    // dataTransfer.dropEffect, so GdkDragContext::suggested_action would be
>+    // more appropriate.  GdkDragContext::actions should be used to set
>+    // dataTransfer.effectAllowed, which doesn't currently happen with
>+    // external sources.
>+

Yes, it should be the setting the drag action to the one the user is requesting.

Comment 44

[Neil Deakin]

Comment on attachment 512360 [details] [diff] [review]
part 3: don't set a drag context with the new widget for a leave event on the old widget

Not sure if this will regress bug 539862, but later patches seem to remove this code anyway.

Comment 45

[Neil Deakin]

(In reply to Karl Tomlinson (:karlt) from comment #7)
> This dispatch differs from the NS_DRAGDROP_ENTER processing in that
> nsEventStateManager::PostHandleEvent will not be invoked to SetDragAction,
> SetCanDrop or SetOnlyChromeDrop on the nsIDragSession.  dragAction and
> canDrop
> get reset before the DRAGDROP_OVER event anyway and I assume onlyChromeDrop
> is
> only needed for the drop (which will not happen until after the
> DRAGDROP_OVER.

Is dragover guaranteed to be fired after dragenter?

The actions can be changed during dragenter as well.

Comment 46

[Karl Tomlinson (:karlt)]

(In reply to Neil Deakin from comment #44)
> Comment on attachment 512360 [details] [diff] [review]
> part 3: don't set a drag context with the new widget for a leave event on
> the old widget
> 
> Not sure if this will regress bug 539862, but later patches seem to remove
> this code anyway.

I'm assuming bug 539862 was about the enter event.
(Later patches use different code, but implement similar logic.)
I tested with a theme, and got a door hanger saying it couldn't be installed because the theme wasn't compatible with nightly.  That seems to indicate that the drop was successful.

Comment 47

[Karl Tomlinson (:karlt)]

(In reply to Neil Deakin from comment #45)
> (In reply to Karl Tomlinson (:karlt) from comment #7)
> > This dispatch differs from the NS_DRAGDROP_ENTER processing in that
> > nsEventStateManager::PostHandleEvent will not be invoked to SetDragAction,
> > SetCanDrop or SetOnlyChromeDrop on the nsIDragSession.  dragAction and
> > canDrop get reset before the DRAGDROP_OVER event anyway and I assume
> > onlyChromeDrop is only needed for the drop (which will not happen until
> > after the DRAGDROP_OVER.
> 
> Is dragover guaranteed to be fired after dragenter?

GTK has only a single "drag-motion" signal with no separate "drag-enter" or
similar.  The existing code synthesizes a NS_DRAGDROP_ENTER during drag-motion
when appropriate before the NS_DRAGDROP_OVER, which always follows.

This patch removes the synthetic NS_DRAGDROP_ENTER, dispatching only the
NS_DRAGDROP_OVER and depending on the ESM to dispatch appropriate enter events
(as it needs to do this anyway to handle crossing into a new element).

As the ESM dispatches the NS_DRAGDROP_ENTER during PreHandleEvent for
NS_DRAGDROP_OVER, I assume this is before the dispatch of the over event.
I'm not expecting the ESM to cancel the dispatch of the over event.

Please let me know if I'm missing the point of your question.

> The actions can be changed during dragenter as well.

I see my comment was wrong about the dragAction getting reset before
DRAGDROP_OVER.  That actually happened before the DRAGDROP_ENTER.

If a NS_DRAGDROP_ENTER were dispatched from widget PostHandleEvent would
SetDragAction if the dragenter is cancelled.  If a subsequent NS_DRAGDROP_OVER
is dispatched but not cancelled, it looks like PostHandleEvent resets
dropEffect to none, but SetDragAction is not called again.  If widget
dispatches only the NS_DRAGDROP_OVER, it looks like we are better off if anything
because we don't end up with a stale dragAction.

I'm really assuming the ESM gets synthetic enter events right, as otherwise there would be existing problems.

Again, please let me know if I'm missing the point of your comment.

Comment 48

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 607420 [details] [diff] [review]
part 12b: schedule event dispatch in response to GTK drag target signals to avoid running the event loop at unexpected times

Review of attachment 607420 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/gtk2/nsDragService.cpp
@@ +1802,5 @@
> +        // that a leave or drop is waiting, but managing different priorities
> +        // may not be worth the effort.  Motion tasks shouldn't queue up as
> +        // they should be throttled based on replies.
> +        mTaskSource =
> +            g_idle_add_full(G_PRIORITY_HIGH, TaskDispatchCallback, this, NULL);

What keeps 'this' alive during the TaskDispatchCallback?

Should we addref "this" here and unref it during the callback?

Should we perhaps not even pass the nsDragService here, and just get the singleton during the callback? r+ if you do that.

::: widget/gtk2/nsWindow.cpp
@@ +5820,5 @@
>          return;
>  
> +    nsCOMPtr<nsIDragService> dragService = do_GetService(kCDragServiceCID);
> +    nsDragService *dragServiceGTK =
> +        static_cast<nsDragService*>(dragService.get());

Put this in a helper function in nsDragService --- nsDragService::GetInstance()?

Comment 49

[Karl Tomlinson (:karlt)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #48)
> > +        mTaskSource =
> > +            g_idle_add_full(G_PRIORITY_HIGH, TaskDispatchCallback, this, NULL);
> 
> What keeps 'this' alive during the TaskDispatchCallback?

It's relying on XPCOM not shutting down before returning to the main event loop.
This is a pre-existing assumption made by the drag-end and drag-failed signal handlers when they call nsBaseDragService::EndDragSession to dispatch dom events and then modify the DragService.

Comment 50

[Karl Tomlinson (:karlt)]

Attachment: part 12a2: Add a helper to get the nsDragService

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #48)
> > +    nsCOMPtr<nsIDragService> dragService = do_GetService(kCDragServiceCID);
> > +    nsDragService *dragServiceGTK =
> > +        static_cast<nsDragService*>(dragService.get());
> 
> Put this in a helper function in nsDragService ---
> nsDragService::GetInstance()?

Is this what you mean, or do you want to return already_AddRefed for callers to use nsRefPtrs?

Comment 51

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Karl Tomlinson (:karlt) from comment #49)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #48)
> > > +        mTaskSource =
> > > +            g_idle_add_full(G_PRIORITY_HIGH, TaskDispatchCallback, this, NULL);
> > 
> > What keeps 'this' alive during the TaskDispatchCallback?
> 
> It's relying on XPCOM not shutting down before returning to the main event
> loop.
> This is a pre-existing assumption made by the drag-end and drag-failed
> signal handlers when they call nsBaseDragService::EndDragSession to dispatch
> dom events and then modify the DragService.

It wasn't obvious to me that XPCOM can't shut down before that glib idle handler runs. But OK.

Comment 52

[Karl Tomlinson (:karlt)]

XPCOM can shut down before the idle handler runs, but ~nsDragService takes care of that.  It's not expected to shut down while the handler is running.

Comment 53

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 607420 [details] [diff] [review]
part 12b: schedule event dispatch in response to GTK drag target signals to avoid running the event loop at unexpected times

Review of attachment 607420 [details] [diff] [review]:
-----------------------------------------------------------------

OK, r+ assuming you revise this to call the new GetInstance method.

Comment 54

[Neil Deakin]

Comment on attachment 512354 [details] [diff] [review]
part 2: no need to explicitly dispatch NS_DRAGDROP_ENTER as this happens during NS_DRAGDROP_OVER

Sorry, didn't realize I didn't mark this as reviewed.

Comment 55

[Karl Tomlinson (:karlt)]

I pushed the small parts 1 to 12a (with no part 9).
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e9d4bca6460
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bd66f15b1f2
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d31d468778b
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a9491869898
https://hg.mozilla.org/integration/mozilla-inbound/rev/46b2bf54dd1e
https://hg.mozilla.org/integration/mozilla-inbound/rev/fadcc45f8fe8
https://hg.mozilla.org/integration/mozilla-inbound/rev/8bea9978c445
https://hg.mozilla.org/integration/mozilla-inbound/rev/93a7a0655014
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f5e0ef6ad58
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf998e485300
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e72b79e7cee
https://hg.mozilla.org/integration/mozilla-inbound/rev/b73de501ea5e

I'll leave part 12a2 and subsequent (including the major part 12b) til after mozilla14 branches.

Comment 56

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/9e9d4bca6460
https://hg.mozilla.org/mozilla-central/rev/5bd66f15b1f2
https://hg.mozilla.org/mozilla-central/rev/0d31d468778b
https://hg.mozilla.org/mozilla-central/rev/3a9491869898
https://hg.mozilla.org/mozilla-central/rev/46b2bf54dd1e
https://hg.mozilla.org/mozilla-central/rev/fadcc45f8fe8
https://hg.mozilla.org/mozilla-central/rev/8bea9978c445
https://hg.mozilla.org/mozilla-central/rev/93a7a0655014
https://hg.mozilla.org/mozilla-central/rev/7f5e0ef6ad58
https://hg.mozilla.org/mozilla-central/rev/cf998e485300
https://hg.mozilla.org/mozilla-central/rev/4e72b79e7cee
https://hg.mozilla.org/mozilla-central/rev/b73de501ea5e

Comment 57

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 512352 [details] [diff] [review]
part 1: Only dispatch drop events when canDrop has been set via dragover and only indicate successful drop when the drop is sent

Review of attachment 512352 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/src/gtk2/nsWindow.cpp
@@ +3520,5 @@
>      LOGDRAG(("nsWindow::OnDragDropSignal\n"));
>  
>      // get our drag context
>      nsCOMPtr<nsIDragService> dragService = do_GetService(kCDragServiceCID);
> +    nsDragService *dragServiceGTK = static_cast<nsDragService*>(dragService.get());

Can't extensions override this service?

Comment 58

[Neil Deakin]

> >      nsCOMPtr<nsIDragService> dragService = do_GetService(kCDragServiceCID);
> > +    nsDragService *dragServiceGTK = static_cast<nsDragService*>(dragService.get());
> 
> Can't extensions override this service?

We already static_cast the drag service on Windows and Mac.

Comment 59

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc7ea0e82d83
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c6358d51fa5
https://hg.mozilla.org/integration/mozilla-inbound/rev/509d2f42fc7f
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce526785dc49
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac5c8077e353

Comment 60

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/3c6358d51fa5
https://hg.mozilla.org/mozilla-central/rev/509d2f42fc7f
https://hg.mozilla.org/mozilla-central/rev/ce526785dc49
https://hg.mozilla.org/mozilla-central/rev/ac5c8077e353