319072 - Tab drag and drop does not work correctly on OS/2

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Peter Weilbacher]

As discussed about two weeks ago in netscape.public.mozilla.os2 Firefoxes tab drag and drop feature is broken on the branch (OS/2 only, but working on the trunk) in such a way that one can always only drag any tab to the far right but nowhere else. The same is true for SeaMonkey after applying the respective patch from bug 105885, so it is an issue in the backend.

I tried a bit of debugging using JS dump() in tabbrowser.xml and it turns out that aEvent.clientX is undefined in <method name="getNewIndex">, so the comparison here
http://lxr.mozilla.org/mozilla1.8/source/toolkit/content/widgets/tabbrowser.xml#1598
is always wrong and always only the number of tabs is returned. It's late so I give up for now but it seems that during drag events this clientX is not computed (is this the one from nsDOMMouseEvent::GetClientX?).

Comment 1

[Mike Kaply [:mkaply]]

What about this fix that only went in the trunk?

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsWindow.cpp&branch=&root=/cvsroot&subdir=mozilla/widget/src/os2&command=DIFF_FRAMESET&rev1=1.194&rev2=1.195

Comment 2

[Peter Weilbacher]

That is the patch from bug 286555 (which is marked fixed1.8) and I can see it in my branch code. I will keep looking.

Comment 3

[Peter Weilbacher]

I have looked into this on and off over the last week but so far haven't been successful. What I did look at so far indicates that on the trunk a call of nsDOMMouseEvent::GetClientX that is done through the JavaScript handler (I see js_Invoke, XPTC_InvokeByIndex on the call stack) while on the branch this call does not happen. It's really difficult to debug this properly, whenever I initiate a drag the debugger crashes and takes the program with it. :-(

I also managed to get the patches from bug 296036, bug 306222, and bug 307086 into my branch tree but that didn't help. Bug 306202 talks about clientX/Y although in a very different context, so it is not really of any help.

roc, bz, you are not involved in the OS/2 port but perhaps you have some idea what could be behind this?

Comment 4

[Peter Weilbacher]

Now, after several hours of compiling trunk Firefoxes between the branching and 2005-09-14 when the problem was fixed I am pretty sure that the patch from bug 307086 is the key. I think when I applied it last week I tried to combine it with one of the other patches I listed earlier and then it failed. Branch patch coming soon.

Comment 5

[Peter Weilbacher]

Attachment: Tab d&d fix, part of patch from bug 307086

OK, it is again late at night but I think this time I got it right and this does indeed fix the problem...

Comment 6

[Mike Kaply [:mkaply]]

What exactly is different about this new path?

Comment 7

[Peter Weilbacher]

Sorry, should have said that right away. It leaves out Rich's whitespace cleanup (and hence does not require the change from event.point to event.refPoint that was done on the trunk).

Comment 8

[Peter Weilbacher]

Review reminder for mkaply... :-)

Comment 9

[Mike Kaply [:mkaply]]

Comment on attachment 205688 [details] [diff] [review]
Tab d&d fix, part of patch from bug 307086

r=mkaply

I'll try to remember to put this in the 1.8 builds

Comment 10

[Mike Kaply [:mkaply]]

Fix checked in to branches