Opened 11 years ago

Closed 11 years ago

#8951 closed defect (fixed)

Button: clicking dijit.form.Button with ENTER key fires handler twice (IE8)

Reported by: bill Owned by: Becky Gibson
Priority: high Milestone: 1.4
Component: Accessibility Version: 1.3.0b3
Keywords: Cc: Joseph Scheuhammer
Blocked By: Blocking:

Description

Load test_Button.html on IE8 and tab to the "Create" button.

Hit ENTER key.

Check console: "clicked simple" is fired twice.

This is what's causing at least some of the errors in the Button_a11y.html test.

Problem doesn't happen w/native buttons.

Attachments (1)

keyEvents.html (5.5 KB) - added by Becky Gibson 11 years ago.
This test file may help to shed some light on this

Download all attachments as: .zip

Change History (8)

comment:1 Changed 11 years ago by bill

See also #8879, another IE8 ondijitclick bug.

comment:2 Changed 11 years ago by Joseph Scheuhammer

Cc: Joseph Scheuhammer added

For diagnosing, I added keydown, keyup, keypress event handlers to the "Create" button, printed out the event type in the 'onclick' handler (the one that prints out "clicked simple"), and added other messages where 'ondijitclick' is remapped to a key event in _Widget.connect().

In IE8, all three key events occur with an ENTER keystroke. In contrast, in IE7, there are only keydown and keyup events.

In IE7, the event type associated with the "clicked simple" message is a keydown event. This is due to _Widget.connect() mapping "ondijitclick" to keydown/ENTER for non-FF browsers.

In IE8, the first event associated with "clicked simple" is also a keydown event, and occurs for the same reason as in IE7.

But, the second "clicked simple" is assoicated with a click event. This click event is independent of _Widget.connect(). It looks like IE8 is turning the keypress into a click event, and because the button has an "onclick" handler, you get "clicked simple" for both the keydown and that click event.

A fix could be added in _Widget.connect(), specifically for IE8, to remove the keydown/ENTER handler. In the case of keydown, extend the:

if(!d.isFF && e.keyCode == d.keys.ENTER &&
  !e.ctrlKey && !e.shiftKey && !e.altKey && !e.metaKey{
...
}

to include a check for IE8:

if(!d.isFF && e.keyCode == d.keys.ENTER &&
    !e.ctrlKey && !e.shiftKey && !e.altKey && !e.metaKey &&
    (!d.isIE || d.isIE < 8){
...
}

It solves the problem, but that if() clause is getting pretty harry.

comment:3 Changed 11 years ago by bill

A few thoughts here:

  1. I bet you need to test for quirks mode too... if d.isQuirks is true then probably IE8 operates like IE7
Considering that ondijitclick's extra handlers are a workaround to browser bugs, maybe the check should be for browsers that need the fix instead of browsers that don't.. if(d.isIE <=7
d.isQuirks) perhaps.
  1. You might want to try safari4 or chrome 2 beta to see if they need the extra handler (although I didn't get diffs on the regression tests on those browsers). And also firefox 4.1 beta.

comment:4 Changed 11 years ago by Becky Gibson

Milestone: tbd1.3.1

comment:5 Changed 11 years ago by bill

Milestone: 1.3.11.4

Talked with Becky about the ondijitclick bugs; agreed to move them to 1.4 since they aren't regressions

comment:6 Changed 11 years ago by Becky Gibson

So, the simple fix for this is to not use ondijitclick for the button since onclick is keyboard accessible for native buttons and the dijit button uses a native button element in the template. But, I am looking at a bunch of other ondijitclick issues so will wait to make this change.

Changed 11 years ago by Becky Gibson

Attachment: keyEvents.html added

This test file may help to shed some light on this

comment:7 Changed 11 years ago by Becky Gibson

Resolution: fixed
Status: newclosed

(In [17832]) fixes #8879, #8946, #8951, #8978, #8979, #9304, #9156 rework of ondijitclick event handler. Perform action only on keyup of enter or space. Track object that receives keydown and only invoke action on keyup when target matches the keydown object. Do not use ondijitclick for elements that already have onclick support for enter and space key press (button, links) or when that onclick event will bubble up to a parent element. Thus, changed button and combobutton templates to use onclick rather than ondijitclick. No longer need special case for submit and reset buttons in button.js _onButtonClick(). Updated button_a11y.html test file to include test of submit and reset buttons. Updated widget-ondijitclick.html to again include space and enter key testing. !strict

Note: See TracTickets for help on using tickets.