Closed
Bug 345425
Opened 18 years ago
Closed 18 years ago
[mac] onDragExit in tabbrowser.xml gets called aDragSession.sourceNode == null (on windows, it is non-null) when I drop a tab on itself
Categories
(Firefox :: Tabbed Browser, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: moco, Assigned: mark)
References
Details
(Keywords: fixed1.8.1)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
jaas
:
review+
mikepinkerton
:
superreview+
beltzner
:
approval1.8.1+
|
Details | Diff | Splinter Review |
[mac] onDragExit in tabbrowser.xml gets called aDragSession.sourceNode == null (on windows, it is non-null)
to reproduce, drag and drop a tab onto itsself, and add a check to onDragExit to see what aDragSession.sourceNode is.
this comes from bug #333791.
I'm working around this (by checking aDragSession.sourceNode, but it appears to be a dnd platform parity bug.
Assignee | ||
Comment 1•18 years ago
|
||
> onDragExit in tabbrowser.xml gets called aDragSession.sourceNode == null
Hi, Seth, how're the kids? I don't see this happening. I placed a couple of dumps:
if (aDragSession.sourceNode) dump("i HAVE a sourceNode\n");
else dump("you HAVE no sourceNode!\n");
in tabbrowser.xml here:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/content/widgets/tabbrowser.xml&rev=1.103.2.54&mark=1803#1798
and always HAD a sourceNode.
I did find another bug in the Mac drag'n'drop code that caused NS_DRAGDROP_EXIT to be sent instead of NS_DRAGDROP_ENTER, due to a stupid mechanical error I made in bug 342361. Could the extraneous ondragexit events for widgets that we're actually entering be causing your problems?
Attachment #230151 -
Flags: review?(joshmoz)
Reporter | ||
Comment 2•18 years ago
|
||
> Hi, Seth, how're the kids?
don't make me come over there.
> I don't see this happening. I placed a couple of dumps:
> if (aDragSession.sourceNode) dump("i HAVE a sourceNode\n");
> else dump("you HAVE no sourceNode!\n");
>
> and always HAD a sourceNode.
Yesterday, on the trunk, on mac os x, debug, I definitely did not have a aDragSession.sourceNode when I attempted to drag-n-drop a tag onto itself.
I'll try your patch again and double check.
> I did find another bug in the Mac drag'n'drop code that caused
> NS_DRAGDROP_EXIT to be sent instead of NS_DRAGDROP_ENTER, due to a stupid
> mechanical error I made in bug 342361. Could the extraneous ondragexit events > for widgets that we're actually entering be causing your problems?
I don't think that would cause this, unless in that scenario, aDragSession.sourceNode was null.
Reporter | ||
Comment 3•18 years ago
|
||
> when I attempted to drag-n-drop a tag onto itself.
oops, sorry mark. see what happens when you actually drop the tab on itself.
that's when I get the onDragExit with aDragSession.sourceNode == null.
Summary: [mac] onDragExit in tabbrowser.xml gets called aDragSession.sourceNode == null (on windows, it is non-null) → [mac] onDragExit in tabbrowser.xml gets called aDragSession.sourceNode == null (on windows, it is non-null) when I drop a tab on itself
Assignee | ||
Comment 4•18 years ago
|
||
OK, I see that. The patch on this bug won't fix this particular problem, but it will fix other problems.
Assignee | ||
Comment 5•18 years ago
|
||
We're calling EndDragSession too soon, and that clears out sourceNode. We don't need to call EndDragSession in the Drop proc because we already call it from InvokeDragSession immediately after calling TrackDrop, and the Drop proc can only be called from inside of TrackDrop.
I've also included the enter/exit confusion patch and removed a couple of overridden methods that did nothing but call their superclasses. I love it when you can fix bugs purely by removing code.
Attachment #230151 -
Attachment is obsolete: true
Attachment #230160 -
Flags: review?(joshmoz)
Attachment #230151 -
Flags: review?(joshmoz)
Attachment #230160 -
Flags: review?(joshmoz) → review+
Assignee | ||
Updated•18 years ago
|
Attachment #230160 -
Flags: superreview?(sspitzer)
Reporter | ||
Comment 6•18 years ago
|
||
Comment on attachment 230160 [details] [diff] [review]
Fix (for this AND something else)
I'm not comfortable sr'ing anything in mozilla/widget.
who else could do this? (stuart?)
Attachment #230160 -
Flags: superreview?(sspitzer) → superreview?(pavlov)
Reporter | ||
Comment 7•18 years ago
|
||
Comment on attachment 230160 [details] [diff] [review]
Fix (for this AND something else)
better idea: pink!
Attachment #230160 -
Flags: superreview?(pavlov) → superreview?(mikepinkerton)
Comment 8•18 years ago
|
||
Comment on attachment 230160 [details] [diff] [review]
Fix (for this AND something else)
er, ok, if you say so.
Attachment #230160 -
Flags: superreview?(mikepinkerton) → superreview+
Assignee | ||
Comment 9•18 years ago
|
||
Checked in on the trunk.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 10•18 years ago
|
||
Comment on attachment 230160 [details] [diff] [review]
Fix (for this AND something else)
Needed for drag and drop parity, supports bug 333791.
Attachment #230160 -
Flags: approval1.8.1?
Comment 11•18 years ago
|
||
Is this going on the 1.8 branch as well? If so, we can remove the extra check from the patch on bug 333791 before it's applied to Fx 2.0
Assignee | ||
Comment 12•18 years ago
|
||
This should go on to the 1.8 branch if bug 333791 does.
Blocks: 333791
Comment 13•18 years ago
|
||
Comment on attachment 230160 [details] [diff] [review]
Fix (for this AND something else)
a=drivers. Please land on MOZILLA_1_8_BRANCH.
Attachment #230160 -
Flags: approval1.8.1? → approval1.8.1+
Assignee | ||
Comment 14•18 years ago
|
||
Checked in on MOZILLA_1_8_BRANCH before 1.8.1b2.
Keywords: fixed1.8.1
Blocks: 370721
You need to log in
before you can comment on or make changes to this bug.
Description
•