fix(ngAria): change accessibility keypress event to use event.which if it is provided #11340
Conversation
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
|
Oops, this computer had my git email set to another one of my emails. I've signed a CLA using that other email as well. |
|
We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm. |
|
I authored the commit, and both emails are mine. |
…f it is provided In Firefox, keyboard events for printable characters (e.g. space) do not use event.keyCode. Use event.which if it is provided before falling back to event.keyCode.
andrewaustin
force-pushed
the
11287-firefox
branch
from
93e52a4 to
6b835a6
Compare
|
CLAs look good, thanks! |
|
👍 nice work! Addresses #11287. |
|
MDN says:
Why do we want to use this? Then again, it says the same about keyCode |
|
Okay, so DOM 3 keyboard events say you want to use I think it might be wiser to use |
|
@caitp According to MDN, the See: https://developer.mozilla.org/en-US/docs/Web/Events/keypress and also the related bugs: https://bugzilla.mozilla.org/show_bug.cgi?id=680830 The API is also slightly different, which uses key values (string names) instead of keyCodes, but that is easy enough to fix. I'm more than happy to update the PR as needed. (It looks like DOM Level 3 events have been renamed to UI Events and are still a working draft?). |
I'm aware of that --- but I think we should go with the most consistent implementations of the most recent DOM events spec. Anyway I stopped caring, lgtm |
|
FWIW, D3E KeyboardEvent is a work in progress in Blink (https://crbug.com/227231), there seem to be some issues with different vendors shuffling around different versions of the draft, which is making things difficult and slow in the classic W3 style. Blink and WebKit currently both implement a It's kind of amazing that WK/Blink are so far behind in this area |
|
@caitp What do you think is the best way forward is for this PR? Would you like to see me try and incorporate DOM Level 3? Or do you think this PR still looks okay? My current understanding, based on what you've shared, is as follows:
Source for FF: https://bugzilla.mozilla.org/show_bug.cgi?id=900372 |
|
What you have is fine for now, I had said LGTM above |
…f it is provided In Firefox, keyboard events for printable characters (e.g. space) do not use event.keyCode. Use event.which if it is provided before falling back to event.keyCode. Closes angular#11340
fix(ngAria): change accessibility keypress event to use event.which if it is provided
In Firefox, keyboard events for printable characters (e.g. space) do not use event.keyCode. Use event.which if it is provided before falling back to event.keyCode.