216800 - drag-n-drop to a GTK2 mozilla build often fails

Status: RESOLVED FIXED

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

Description

[Darin Fisher]

drag-n-drop to a GTK2 mozilla build often fails.  i see this JS exception:

JavaScript error:
 line 0: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsITransferable.getAnyTransferData]"  nsresult:
"0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame ::
chrome://global/content/nsDragAndDrop.js :: anonymous :: line 89"  data: no]

it seems like there are no problems if the GTK2 build is initiating a
drag-n-drop, but if the GTK2 build is on the receiving end, it will often fail.
 i'll attach a Widget:5,nsDragService:5 NSPR log...

Comment 1

[Darin Fisher]

Attachment: Widget:5,nsDragService:5 log file

Comment 2

[Darin Fisher]

i meant to say that the log file captures two drag-n-drops.  the first is
successful, but in the second case the JS exception is hit.  i cut-n-paste the
JS exception into the log file at the right place in case that helps track down
the cause of this bug.

right now, i'm trying to figure out why the JS code (nsDragAndDrop.js's
nsTransferable) is being invoked at all prior to
nsDragService::TargetDataReceived getting called.

Comment 3

[Darin Fisher]

ok, this turns out to be a consequence of the 20ms timeout in
nsDragService::GetTargetDragData in conjunction with the 0ms timeout in
nsWindow::OnDragLeaveEvent.  raising the latter timeout value to 30ms seems to
fix drag-n-drop.  this explains why the problem seemed to be intermittent for
me.  sometimes the call to gtk_main_iteration() in
nsDragService::GetTargetDragData would dispatch the timer callback event and
sometimes it would not.  maybe we just need to flush any pending events before
calling EndDragSession.  i'm not really sure... blizzard, what do you think? 
it'd be nice to get some kind of fix for this into 1.5.

Comment 4

[Darin Fisher]

i just thought about my last comment some more... the 20ms timeout value in
GetTargetDragData should not matter at all.  i think what is important is that
we give some time (more than 0ms anyways) to process any events that are already
in our event queue when we get the drag_leave GTK signal.

Comment 5

[Darin Fisher]

Attachment: v1 patch

hackish "delay the drag_leave event some" patch.  is there a better solution??

Comment 6

[Christopher Blizzard (:blizzard)]

Comment on attachment 130217 [details] [diff] [review]
v1 patch

r+sr=blizzard

Just a caveat, though.	I don't expect this to work in all cases since the
underlying problem, that drops on unix are async, is the real problem here and
we're only just causing reentrancy that lasts a little longer this way.  We're
just making an exisiting hack worse, that's all.

Comment 7

[Darin Fisher]

Comment on attachment 130217 [details] [diff] [review]
v1 patch

this is a hack, but i think it'd be a good hack to get into the 1.5 release. 
without this patch, drag-n-drop between tbird and fbird is hosed.

Comment 8

[Asa Dotzler [:asa]]

Comment on attachment 130217 [details] [diff] [review]
v1 patch

a=asa (on behalf of drivers) for checkin to 1.5beta

Comment 9

[Darin Fisher]

fixed-on-trunk

Comment 10

[Darin Fisher]

hmm.. the behavior in today's firebird build (nightly GTK2 linux) is better, but
it seems like the problem still exists 30-40% of the time :-(

perhaps the timeout needs to be increased... ugh... what a hack...

at any rate, reopening this bug.  maybe there is a real solution...

Comment 11

[Christopher Blizzard (:blizzard)]

More async apis required (bryner).

Comment 12

[Glen Nakamura]

Attachment: patch ported from GTK1 version

I recently migrated my mozilla builds to GTK2 and hit this bug, which
is really the GTK2 version of Bug 161824.  I've attached a GTK2 version
of the patch I previously submitted for GTK1.  See Bug 161824 for an
explanation of why it's necessary to explicitly cancel the drag leave
timer.	The patch is relative to mozilla 1.4 and fixes the problem on
my system.  It's probably too late for 1.4.1 and 1.5, but it would be
nice to get into 1.4.2 and trunk builds at least.

Comment 13

[Darin Fisher]

thanks for the patch glen!

Comment 14

[Christopher Blizzard (:blizzard)]

Comment on attachment 132442 [details] [diff] [review]
patch ported from GTK1 version

r+sr=blizzard

Comment 15

[Darin Fisher]

fixed-on-trunk .. thx guys!

Comment 16

[Darin Fisher]

.

Comment 17

[Mike Kaply [:mkaply]]

Let's put the right patch in 1.4.2