Last Comment Bug 319072 - Tab drag and drop does not work correctly on OS/2
: Tab drag and drop does not work correctly on OS/2
Status: RESOLVED FIXED
[nvn-dl]
: fixed1.8.0.2, fixed1.8.1
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: 1.8 Branch
: x86 OS/2
: -- normal (vote)
: ---
Assigned To: Peter Weilbacher
: Hixie (not reading bugmail)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-12-04 16:37 PST by Peter Weilbacher
Modified: 2006-02-26 08:53 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Tab d&d fix, part of patch from bug 307086 (3.60 KB, patch)
2005-12-12 19:08 PST, Peter Weilbacher
mozilla: review+
mozilla: approval1.8.0.2+
mozilla: approval1.8.1+
Details | Diff | Splinter Review

Description Peter Weilbacher 2005-12-04 16:37:18 PST
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 2 Peter Weilbacher 2005-12-05 10:46:29 PST
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 2005-12-12 12:33:36 PST
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 2005-12-12 17:50:41 PST
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 2005-12-12 19:08:05 PST
Created attachment 205688 [details] [diff] [review]
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] 2005-12-12 19:53:57 PST
What exactly is different about this new path?
Comment 7 Peter Weilbacher 2005-12-13 02:13:04 PST
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 2006-01-15 10:15:31 PST
Review reminder for mkaply... :-)
Comment 9 Mike Kaply [:mkaply] 2006-01-15 11:12:10 PST
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] 2006-02-13 07:04:59 PST
Fix checked in to branches

Note You need to log in before you can comment on or make changes to this bug.