Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#16371 closed defect (fixed)

dojo/on in Opera 12 doesn't generate keydown event

Reported by: Paul Christopher Owned by: bill
Priority: undecided Milestone: 1.8.2
Component: Events Version: 1.8.1
Keywords: Cc: Kris Zyp
Blocked By: Blocking:

Description

Visit http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/themes/themeTester.html with Opera. Select tab "Select Widgets". Click in the input field dijit.form.FilteringSelect and try to open the drop down with cursor down. This does not work for some reason. The drop down remains closed. A browser default/ limitation since Opera never sends a keydown event of "cursor down" to a textbox because of its strange keyboard navigation concept?

Attachments (1)

testInput.html (1.7 KB) - added by Paul Christopher 7 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 7 years ago by Paul Christopher

I had now some time to look into this and to create a reduced test case, see attachment above.

  • If you use native javascript keydown, keyup, keypress handlers, cursor down is recognized
  • Using dojo/on and binding to keydown, keyup, keypress, dojo/on gives you only one single keyup event regardeless how long the key is hold down.
  • The same holds true for dijit.form.ValidationTextBox and binding to the dijit events with built in on-method: Dojo only gives you one single keyup event.

Moreover: There seems to be no event normalization in dojo/on for key events, i.e. the passed event object is not the same in all browsers. This is quite a pain - at least for me as unexperienced programmer. I would expect when using dojo/on to get a normalized event object like you get when connecting to a dijit key event. I realized the difficulties especially when trying to write cross-browser compatible code for http://bugs.dojotoolkit.org/ticket/16102.

Changed 7 years ago by Paul Christopher

Attachment: testInput.html added

comment:2 Changed 7 years ago by bill

Cc: Kris Zyp added

Right, dojo/on has no normalization of keydown and keypress events, as per Kris' design. A reaction to our previous normalization that turned out not to match what the official standard became. I agree there's an argument for event normalization.

Note that Opera is not supported by dijit. Still, did you find the actual cause of this problem with FilteringSelect, given that dojo/on is working according to spec?

BTW, I'm guessing it happens with anything extending _HasDropDown.

comment:3 Changed 7 years ago by Paul Christopher

My impression is, that dojo/on is not working correctly (compared to natively binded event handlers, see test case): dojo/on simply does not recognize keydown and keypress for cursor down (and other special keys?). Since _hasDropDown's _onKey does connect to keydown only, FilteringSelect does not get the cursor down event, I guess.

All in all, when using a javascript framework and binding to key events, I would have expected not to be confronted with the usual key event madness as described here: http://unixpapa.com/js/key.html. But my impression was: I am.

comment:4 Changed 7 years ago by Paul Christopher

I think line 300-302 in dojo/on (1.8.0) needs to be changed to

if(has("opera")){
   captures.keypress = "keydown";
}

instead of

if(has("opera")){
	captures.keydown = "keypress"; 
}

Explanation: Opera doesn't give you a keypress event for non-printable characters. And since keypress is transformed to keydown, we get no event at all for non-printable characters.

However changing the code as mentioned above gives you even a keypress event for non-printable characters on Opera. And I think that's, what we want.

Yet unsolved, see test case: If you enter a character in the dijit/form/ValidationTextBox, Opera gives you a "Keypress, Keydown, Keyup". FF, IE gives you "Keydown, Keypress, Keyup". No idea why.. Is the order the events are fired important? If so: Something more seems to be wrong with Opera. Please note: With the above fix, dojo/on works perfectly with native <inputs>. There, the event order is correct "Keydown, Keypress, Keyup". Only the dijit controls are wrong. Any idea why?

comment:5 in reply to:  4 Changed 7 years ago by bill

The idea for app code is to use keypress for printables and keydown for non-printables, and to ignore keypress events for non-printables if they occur.

At a glance it sounds like all the opera code should just be removed from dojo/on? My guess is that it used to be needed, but now it's harmful instead.

Last edited 7 years ago by bill (previous) (diff)

comment:6 Changed 7 years ago by Paul Christopher

_KeyNavContainer in version 1.8.0 connects for the detection of cursor up/ cursor down events to keypress only. With the above patch, Opera would get this event, too. All in all, it would make Opera behave simply the same as FF/IE. That's no harm I think.

However the missing event normalization is still a problem and thus navigating with cursor up/ cursor down does not work in _KeyNavContainer: Opera gives you for cursor down a charOrCode of "(", Firefox a charOrCode of 40. focusPrev/ focusNext are therefore never called.

With the above patch, keyboard navigation for DateTextBox isn't working either. No idea why..

comment:7 Changed 7 years ago by bill

I tried your test file and I agree, things are really messed up now because (due to the code you mentioned) dojo/on generates no keydown event for the arrow keys. And it seems like the fix you suggested fixes the problem. It looks like DateTextBox is working fine too, except that the focus outline isn't appearing on the Calendar so you can't tell where you are. But that's an unrelated problem.

Regarding _KeyNavContainer... I should have been more specific. We need to differentiate between code using the old dojo.connect() API, and code using the new dojo/on API. What I meant to say above is that code using dojo/on should use keypress for printables and keydown for non-printables.

The old dojo.connect() API is a different story, because IIRC keypress can be used for everything, and the events have that extra charOrCode attribute. _KeyNavContainer in version 1.8 is using the old dojo.connect() API, so we probably should ignore it for now. Actually not sure why you are talking about it.

Finally, about the order of events in dijit controls... again those events are based on dojo.connect(), the old API, so we probably shouldn't worry about it.

So in conclusion, I think your fix is good.

Last edited 7 years ago by bill (previous) (diff)

comment:8 Changed 7 years ago by Paul Christopher

Bill, I have just tested again and now I think your first intuition of removing the code was right. Why? My fix generates a keypress event for non-printable characters and makes Opera behave like other browsers, but at the same time it overwrites the native keypress event for printable characters. Therefore for printable characters, the passed event object is wrong: it's a keydown and not a keypress and you can barely work with it. Just log the event object to the console and you'll see what I mean. Thus removing the has('opera') should be the correct way to fix the issue, I think.

Moreover I have seen that _KeyNavContainer has been refactor in the meanwhile to use keydown events for cursor key detection, too - and that's fine.

I haven't noticed in DateTextBox, that just the focus outline is missing. So it basically works and that's fine, and if I have time I'll have a look at this.

All in all, maybe we don't need event normalization for key events at all if you know that you need to use keydown/keyup for non-printable characters and keypress for printable characters? Is that the way the specs suggest?

comment:9 Changed 7 years ago by bill

Component: Dijit - FormEvents
Owner: changed from Douglas Hays to Kris Zyp
Summary: Opera and FilteringSelect: Cannot open drop down with cursor downdojo/on in Opera doesn't generate keydown event

Hmm, good point, it's no good if on(node, "keypress", func) calls func(evt) without evt.charCode set.

The comment in dojo/on says the current opera specific code is necessary because:

// this one needs to be transformed because Opera doesn't support repeating keys on keydown

Seems like a poor reason though. Key-repeat is a minor issue.

The only thing unclear to is what removing that opera-specific code will do to dojo.connect(). Because I notice dojo/_base/connect.js also has opera specific code, and that needs to be changed.

All in all, maybe we don't need event normalization for key events at all if you know that you need to use keydown/keyup for non-printable characters and keypress for printable characters? Is that the way the specs suggest?

Not sure, Kris told me that's the correct behavior but I haven't actually seen the spec. IIRC he said that's how all browsers except firefox work, but recently I saw chrome generating keypress and keydown events for everything. It does seem to be a portable way to write event code though.

comment:10 Changed 7 years ago by bill

Milestone: tbd1.8.2
Owner: changed from Kris Zyp to bill
Status: newassigned
Summary: dojo/on in Opera doesn't generate keydown eventdojo/on in Opera 12 doesn't generate keydown event

Was hoping for a response from Kris for this, but since he hasn't answered I'm going to just check in the change. It turns out that Opera's behavior changed between Opera 11 and Opera 12. With Opera 11 the current dojo/on code at least works. Not sure if the workaround code in dojo/on is helpful, but at least it doesn't break things. So apparently Opera 12 started handling keydown and keypress like other browsers, and therefore the workaround code needs to be removed.

I tried making the same change to Dojo 1.7, but that doesn't fix the behavior of FilteringSelect, so I'll just check in the change for 1.8+, and leave Dojo 1.7 broken.

comment:11 Changed 7 years ago by bill

Resolution: fixed
Status: assignedclosed

In [30100]:

remove code no longer needed for opera 12, fixes #16371 on 1.8 branch !strict

comment:12 Changed 7 years ago by bill

In [30101]:

Remove keydown --> keypress code no longer needed for opera 12, fixes #16371 on trunk !strict. Also, dojo/on was depending on things like has("android") without listing dojo/sniff as a dependency, so fixing that too. Plus, some spelling errors.

Note: See TracTickets for help on using tickets.