PDA

View Full Version : [1.1.1] Ext.dd.Registry.register/unregister broken



malraux
16 Oct 2007, 5:55 PM
I've observed the following behavior in Firefox 2 on both Mac and Linux.

I use the following to register a number of drop points for use in a DropZone:


var drops = Ext.query('#myGrid .bldg-drop');
for (var i = 0; i < drops.length; i++)
{
Ext.dd.Registry.register(drops[i]);
}


After I use a particular drop point, I want it to be inelegible for further drops, so I unregister it:


myDropZone.onNodeDrop = function(targetData, source, e, data)
{
// Do the drop stuff here
// Then unregister
Ext.dd.Registry.unregister(targetData.ddel);
return true;
}


Now, unregister removes the element from the Registry's internal elements hash, but the drop point is still valid for dropping. The culprit? This line of code from the register method (line 56 of Registry.js):


if(data.isHandle !== false){


Since I'm not putting isHandle as true *or* false in my drop item registration data (I'm not passing in data at all, for that matter), the !== false is returning true because 'isHandle' is undefined in the 'data' variable at that point. As a result, the element gets added as a handle in the handles hash, and unregister() merrily unregisters the element from 'elements' only, not from 'handles', because it's checking for the existence of data.handles, not data.isHandle like register() does.

Then, Registry.getTargetFromEvent() does this when determining if something is a drop point:


return t ? elements[t.id] || handles[t.id] : null;

and bingo-bango, the unregistered element (faux handle) is considered still registered and things may still be dropped upon it.

Changing that line 56 to:


if(data.isHandle){

fixes the problem, but I presume from the documentation that the !== is used to default the "element drags itself" state to true. unregister(), however, doesn't clear the handles unless the 'handles' portion of the data is passed in; ie. the defaults isHandle = true assumption that is made by register() is not kept by unregister().

In my reading of the code, the default of isHandle to true seems unnecessary since everything seems to work as advertised when that assumption is removed. However, if that is necessary for some other reason, then the proper fix appears to be to add a simple:


delete handles[id];

after line 75 in Registry.js.

The workaround I'm using is to provide {isHandle: false} as the registration data when calling Ext.dd.Registry.register().

Regards,
-scott anderson

malraux
24 Oct 2007, 3:22 PM
I've attached code that can be dropped into the examples directory.

Regards,
-scott anderson

malraux
5 Nov 2007, 8:07 PM
No response? Is a bug? Is not a bug? So long and thanks for all the fish?

:-)

Just bumping this because the forum guidelines say I can, and I was interested in what y'all thought of this one. Thanks.