Opened 13 years ago

Closed 12 years ago

#2641 closed defect (fixed)

port connect() + tests into core

Reported by: alex Owned by: alex
Priority: high Milestone: 0.9beta
Component: Events Version: 0.4.2
Keywords: Cc:
Blocked By: Blocking:

Description

as per the roadmap document and refactor.txt, port dojo.event() and tests into Core. Move dojo.event.* to dojox.advices.

Attachments (2)

event.js_8264.patch (476 bytes) - added by guest 13 years ago.
patch for issue introduced in rev.8264: evt.keyCode is read-only in IE if key is ctrl or shift, thus try/catch is required, as already implemented elsewhere in the same file
IE Keypress Strategy.png (32.9 KB) - added by sjmiles 13 years ago.
Diagram I dashed off to explain the onkeypress/onkeydown trick on IE. For posterity.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 13 years ago by sjmiles

(In [7988]) First draft of new connect/event stuff. Refs #2641

comment:2 Changed 13 years ago by alex

(In [7994]) API location refacotoring, style updates, and reformatting. Refs #2641

comment:3 Changed 13 years ago by alex

(In [8235]) committing Scott's new keyboard handling work + tests. Refs #2641. Still needs more unit tests.

comment:4 Changed 13 years ago by alex

(In [8249]) I'm a freaking idiot. My apologies to scott and adam for screwing things up for both of them. Refs #2641

comment:5 Changed 13 years ago by sjmiles

(In [8264]) Event.js: all browsers: set keyCode to 0 when charCode has a value, norm CTRL-BREAK to CTRL-c where possible, IE: use native keypress for ESC and ENTER refs #2641

Changed 13 years ago by guest

Attachment: event.js_8264.patch added

patch for issue introduced in rev.8264: evt.keyCode is read-only in IE if key is ctrl or shift, thus try/catch is required, as already implemented elsewhere in the same file

comment:6 Changed 13 years ago by sjmiles

(In [8306]) Remove vestigal argument to "dispatcher". Refs #2641.

comment:7 Changed 13 years ago by sjmiles

(In [8308]) Make sure stealth onkeydown listeners are disconnected with their progenitors. Fixes last known missing requirement for event.js. Refs #2641

comment:8 Changed 13 years ago by sjmiles

(In [8402]) Another round of revisions to connect and event. Refs #2641. *connect*: had to separate argument normalization from the main functions (hopefully this will aid the doc parser since there is only one "connect/disconnect" pair now, and overrides are done to private "_connect/_disconnect" functions. *event*: eliminated confusing 'clean' methods and moved a key function out of the closure for leak prevention. *disconnect*: changed disconnect so that it only takes one argument: the handle returned from connect, simplifying client code that needs to manage connections. Updated code in various modules to reflect this change.

comment:9 Changed 13 years ago by sjmiles

(In [8403]) Updated code in various modules to reflect changes in Dojo r8402. Refs #2641. *disconnect*: changed disconnect so that it only takes one argument: the handle returned from connect, simplifying client code that needs to manage connections.

comment:10 Changed 13 years ago by sjmiles

(In [8404]) Updated code in various modules to reflect changes in Dojo r8402. Refs #2641. *disconnect*: changed disconnect so that it only takes one argument: the handle returned from connect, simplifying client code that needs to manage connections.

comment:11 Changed 13 years ago by sjmiles

(In [8424]) Fine tuning in connect. Redo IE keyboard magic in event.js so it actually works. Update docs. Refs #2641.

Changed 13 years ago by sjmiles

Attachment: IE Keypress Strategy.png added

Diagram I dashed off to explain the onkeypress/onkeydown trick on IE. For posterity.

comment:12 Changed 13 years ago by alex

(In [8431]) style guide conformance for changeset 8402. Refs #2641

comment:13 Changed 13 years ago by sjmiles

(In [8457]) Fix bug (x2) setting stealthKeyDown flag. Thanks Doug. Refs #2641.

comment:14 Changed 13 years ago by guest

Concerning changeset 8457 done by sjmiles:

I do not understand why "!kd.listeners" is present in the condition on line 231. May be it should be simply

if(!kd||!kd._stealthKeydown)

Best regards,
Konstantin Kolinko

comment:15 Changed 13 years ago by sjmiles

Someone could have replaced the Dojo dispatcher at onkeydown (kd) with some other function. In this case, _stealthKeyDown is true, but kd.listeners is false.

Another scenario is that someone clobbers the Dojo dispatcher, then connects to onkeydown, then connects to onkeypress. We don't actually trap this case. I suppose we could write a more robust test to see if a stealth listener is actually there, but it seems like an awfully unlikely circumstance.

Thanks for studying this code.

comment:16 Changed 13 years ago by sjmiles

(In [8474]) Fix test that was using the console instead of the test object. Refs #2641.

comment:17 Changed 13 years ago by guest

It looks like the new connect() is changing "onevent" to just "event" in Mozilla. This works for DOM events, but it doesn't work for connecting normal object methods.

say I have a object { onrender: function(){} } dojo.connect(obj, "onrender",...) will try to connect to "render" in Mozilla and "onrender" in IE...

comment:18 Changed 12 years ago by sjmiles

It should only munge the names if it detects that 'obj' is a node, in which case it assumes you are trying to connect to a DOM event.

If you have a test case where it is doing this and 'obj' is not a node, please post the particulars.

If 'obj' is in fact a node (or node-ish) but you want to connect to a custom event (not a DOM event) pass true for the fifth parameter (dontFix) to connect.

comment:19 Changed 12 years ago by guest

Thanks! I just realized that my object has a method named addEventListener...

The node-ish check (nodeType, attachEvent, addEventListener) probably should be documented near connect(), it wasn't obvious at first sight.

comment:20 Changed 12 years ago by alex

Status: newassigned

comment:21 Changed 12 years ago by alex

Resolution: fixed
Status: assignedclosed

I'm gonna mark this closed. We can file new issues under separate cover.

Note: See TracTickets for help on using tickets.