Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#2894 closed defect (fixed)

dojo.disconnect is not disconnecting "onkeypress" event listeners

Reported by: simonjb Owned by: sjmiles
Priority: high Milestone:
Component: Core Version: 0.9
Keywords: Cc: alex
Blocked By: Blocking:

Description (last modified by bill)

To reproduce:

  1. open test_Menu2.html
  2. right click on the page background to open up the menu
  3. repeatedly press the down arrow key: focus changes 1 item at a time
  4. click off the menu to close it
  5. reopen the menu
  6. repeatedly press the down arrow key: focus changes 2 items at a time
  7. if you close and open the menu again then focus will change 3 items, then 4 items, and so on

Attachments (1)

event.diff (604 bytes) - added by simonjb 12 years ago.

Download all attachments as: .zip

Change History (11)

Changed 12 years ago by simonjb

Attachment: event.diff added

comment:1 Changed 12 years ago by simonjb

Added a patch that fixes the issue.

comment:2 Changed 12 years ago by sjmiles

(In [8366]) dojo.event.addListener was returning the wrong function, refs #2894.

comment:3 Changed 12 years ago by sjmiles

If you don't mind, please test this in IE6 also, because the event mechanism is different in many ways (to avoid leaks).

comment:4 Changed 12 years ago by bill

Description: modified (diff)

BTW I checked in the Menu2 patch from #2788 so I'm updating the instructions above.

comment:5 Changed 12 years ago by sjmiles

Are you reporting that Simon's patch did not fix the issue?

comment:6 Changed 12 years ago by bill

No, I'm saying that you don't need to apply simon's patch to Menu in order to test this problem. Actually, you can just ignore my previous comment altogether. The instructions above should reproduce the problem, and the attached patch should fix it, I think/hope.

comment:7 Changed 12 years ago by sjmiles

Ok, I can see that it's not clear from the report, but I checked in Simon's patch in [8366], aka "dojo.event.addListener was returning the wrong function".

comment:8 Changed 12 years ago by simonjb

I've tested in IE 6 and the keyboard behavior for the menu is as expected. Do I need to do testing or measurement in terms of memory leaks?

comment:9 Changed 12 years ago by sjmiles

Resolution: fixed
Status: newclosed

Thanks Simon. Wrt to memory leaks, periodic testing to make sure leaks aren't creeping in is a very good idea. Currently, I don't know how to automate that (e.g., in DOH). As it happens, I believe the event.js has developed a leak, but I should be fixing it shortly. After that, I appreciate as many eyes on leak testing as possible.

comment:10 Changed 12 years ago by (none)

Milestone: 0.9M2

Milestone 0.9M2 deleted

Note: See TracTickets for help on using tickets.