Opened 11 years ago

Closed 11 years ago

Last modified 6 years ago

#6667 closed defect (fixed)

IE key events don't match Firefox

Reported by: Douglas Hays Owned by: Douglas Hays
Priority: high Milestone: 1.2
Component: General Version: 1.1.0
Keywords: Cc:
Blocked By: Blocking:

Description

Pressng the left paren key "(" generates keyCode=40, charCode=40 on IE6, but on FF2 and Safari, it generates, keyCode=0, charCode=40. Since Firefox is the standard, IE is broken.
This is causing dijit/tests/form/test_ComboBox.html to fail. Run the test in IE6, focus the first box with Iowa and try to type (. The dropdown appears since the code thinks the down arrow was typed since it also has keyCode=40.
This works fine in FF and Safari.

Change History (15)

comment:1 Changed 11 years ago by Douglas Hays

IBM #84322

comment:2 Changed 11 years ago by sjmiles

It's not possible to patch all the codes under IE.

IIRC, the rule for Dojo key events is that you have to check charCode first to determine if it's a printable character; only if the charCode is 0 is it considered safe to check keyCode.

If my recollection is faulty, please heap abuse as needed. :)

comment:3 Changed 11 years ago by sjmiles

Resolution: invalid
Status: newclosed

comment:4 Changed 11 years ago by bill

I don't remember the rule myself, I thought there was something about using keydown vs. keypress events for printable vs. non-printable characters, can you refresh my memory on that? (see also #5869)

comment:5 Changed 11 years ago by dante

I'm assuming this is what this snippet is for:

	var dk = dojo.keys;
		var key = (e.charCode == dk.SPACE ? dk.SPACE : e.keyCode);

comment:6 Changed 11 years ago by Douglas Hays

Resolution: invalid
Status: closedreopened

comment:7 Changed 11 years ago by Douglas Hays

Owner: changed from sjmiles to Douglas Hays
Status: reopenednew

I'll have to add workarounds in dijit.

comment:8 Changed 11 years ago by Douglas Hays

The current key event implementation seems wasteful and will cause code bloat:
if(evt.keyCode == dojo.keys.DOWN_ARROW)
becomes
if(evt.charCode == 0 && evt.keyCode == dojo.keys.DOWN_ARROW)
In my opinion, the IE keypress event should be normalized to match Firefox. The only dojo core change would be to zero out the keyCode if charCode != 0.
I'll use this defect to add the extra code throughout dijit.

comment:9 Changed 11 years ago by sjmiles

Unfortunately we cannot "normalize IE event to Firefox", this was already tried. From line 322 in event.js:

// Mozilla sets keyCode to 0 when there is a charCode
// but that stops the event on IE.

I should have been clearer that it's not some arbitrary decision I was making. :)

Alternatives I have considered are:

  • Synthesize a phony event object for keypress. This could work, but there also could be consequences.
  • Come up with a whole new model for handling keyboard. This is an area where browser compatibility is even more broken that usual due to an utterly vague spec. This could be some kind of YUI-like connect-to-this-key, or some kind of overlaid implementation of the latest w3c draft.

Neither of these is something that can be done haphazardly.

Sorry for confusion on valid/invalid. IMO, this is previously trodden ground, and the defect is not with the event system, but how Dijit has implemented key handling. I realize it's not at all cut-n-dry, so if you want to attach fixes to this ticket, I won't squawk.

Currently keyboard code should look something like this:

if (evt.charCode) {
  // handle printable keys (using charCode and keyChar)
} else {
  // handle non-printable keys (using keyCode)
}

comment:10 Changed 11 years ago by Douglas Hays

Thanks for the explanation.
evt.keyChar is almost perfect, except it sets the invalid value to an empty string instead of evt.keyCode.
If it were changed, then we could do:

switch(evt.keyChar){
  case dojo.keys.DOWN_ARROW: blah;
  case dojo.keys.UP_ARROW: blah;
  case 'w': blah;
  case dojo.keys.ENTER:
  case ' ': buttonpressed();

I wanted to point out that often a switch statement is used for key handling and that both kinds of keys (named numeric and literal ascii chars) are used, and that often they need to be logically together (ie. ENTER and space for button presses).
I've been changing the code throughout digit to be:

switch(evt.keyChar || evt.keyCode)

I doubt we can change keyChar at its creation time to incorporate evt.keyCode for backward-compatibility reasons.

comment:11 Changed 11 years ago by sjmiles

change keyChar at its creation time to incorporate evt.keyCode

That's a really good idea.

As you say, if anybody is relying on evt.keyChar being "", then we would break them. But offhand it seems like a worthy risk.

Alternatively, we could introduce a new field that is simply set to (keyChar
keyCode).

comment:12 in reply to:  11 Changed 11 years ago by sjmiles

Supposed to be vertical bars there...

keyChar || keyCode

comment:13 Changed 11 years ago by Douglas Hays

(In [13544]) References #6667 !strict. Add new charOrCode member to key event object to simplify code, removing the need to check both charCode/keyChar AND keyCode when looking for specific keys. Reviewed by sjmiles.

comment:14 Changed 11 years ago by Douglas Hays

Resolution: fixed
Status: newclosed

(In [13547]) Fixes #6667 !strict. Change keypress event handlers in dijit to use charOrCode instead of keyCode to prevent printable characters from being mistaken for functional keys. For example, the left parenthesis has the same keyCode as down arrow in IE.

comment:15 Changed 6 years ago by bill

In [30480]:

Deprecate typematic's charOrCode option, and un-deprecate keyCode and charCode, making keyCode use on(node, "keydown", ...), and charCode use on(node, "keypress", ...). Dijit only uses keyCode.

This reversal of the original plan is because dojo/_base/connect itself is deprecated, so if typematic is carried over to version 2, it cannot support charOrCode. Using on(node, "keydown", ...) solves the problem mentioned in #6667 about the left parenthesis being treated as the down arrow.

Refs #6667, #16585 !strict.

Note: See TracTickets for help on using tickets.