Opened 8 years ago

Closed 8 years ago

#13135 closed defect (fixed)

[regression] dojo.dnd.Movable.onMouseUp needs to disconnect more events

Reported by: JIm Fulton Owned by: Eugene Lazutkin
Priority: high Milestone: 1.7
Component: DnD Version: 1.6.1
Keywords: Cc:
Blocked By: Blocking:

Description

dojo.dnd.Movable.onMouseUp almost certainly needs to disconnect 4, rather than 2 events. I assume that the touch event handler registrations were added to onMouseDown after onMouseUp was written.

The bug is in svn as well as 1.6.1.

Change History (5)

comment:1 Changed 8 years ago by bill

Summary: dojo.dnd.Movable.onMouseUp needs to disconnect more events[regression] dojo.dnd.Movable.onMouseUp needs to disconnect more events

I must have broke this w/my patch to add touch support to DnD.

Probably would be fixed in trunk by checking in the DnD patch from #12934. Although seems safer to also use Eugene's recently suggested

while(this.events.length){
	dojo.disconnect(this.events.pop());
}

syntax in all cases anyway. Especially the 1.6 branch, where dojo.touch is not available.

comment:2 Changed 8 years ago by JIm Fulton

That while loop doesn't look right to me. It would disconnect handlers set up in the constructor. Maybe the while loop makes more sense in light of other patches.

comment:3 Changed 8 years ago by bill

Ah you are right. Anyway should apply the patch in #12934, and then 2 will be the right number again. (And if we want to fix for the 1.6/ branch, change 2 to 4 like you said originally.)

comment:4 Changed 8 years ago by bill

Milestone: tbd1.7

comment:5 Changed 8 years ago by bill

Resolution: fixed
Status: newclosed

(In [25666]) Use dojo.touch code in dojo.dnd, fixes #12934, #13135. (I checked w/Eugene, he said Evan's patch looked good.)

Note: See TracTickets for help on using tickets.