RESOLVED FIXED 16462
REGRESSION: access keys broken on Windows
https://bugs.webkit.org/show_bug.cgi?id=16462
Summary REGRESSION: access keys broken on Windows
Alexey Proskuryakov
Reported 2007-12-16 03:43:17 PST
I realized that I broke access keys with my fix to bug 16436. Layout tests didn't catch that, because we send WM_KEYDOWN/WM_CHAR events with Alt key pressed, and not WM_SYSKEYDOWN. Patch forthcoming.
Attachments
proposed fix (6.58 KB, patch)
2007-12-16 05:21 PST, Alexey Proskuryakov
no flags
proposed fix (8.25 KB, patch)
2007-12-16 05:35 PST, Alexey Proskuryakov
darin: review+
Alexey Proskuryakov
Comment 1 2007-12-16 05:21:10 PST
Created attachment 17930 [details] proposed fix I hate to submit this without an automated test - but it looks like while our behavior matches IE well, the underlying mechanics are quite different. So, while it's possible to catch the changes with a test, such a test would fail for certain legitimate changes, too.
Alexey Proskuryakov
Comment 2 2007-12-16 05:24:45 PST
Comment on attachment 17930 [details] proposed fix This needs some more work...
Alexey Proskuryakov
Comment 3 2007-12-16 05:35:14 PST
Created attachment 17931 [details] proposed fix Fixed DRT to send system key events as appropriate.
Darin Adler
Comment 4 2007-12-16 07:21:20 PST
(In reply to comment #1) > I hate to submit this without an automated test - but it looks like while our > behavior matches IE well, the underlying mechanics are quite different. So, > while it's possible to catch the changes with a test, such a test would fail > for certain legitimate changes, too. It's OK to have an automated test that captures our current behavior, even if it's too specific, and requires behavior that is not necessarily "correct". Our automated tests help us notice behavior changes. Failures in them don't have to reflect bugs.
Darin Adler
Comment 5 2007-12-16 07:23:55 PST
Comment on attachment 17931 [details] proposed fix I don't entirely understand why the code to call handleAccessKey couldn't just go into keyEvent. r=me, though
Alexey Proskuryakov
Comment 6 2007-12-16 08:18:55 PST
Committed revision 28770. (In reply to comment #4) > It's OK to have an automated test that captures our current behavior, even if > it's too specific, and requires behavior that is not necessarily "correct". Our > automated tests help us notice behavior changes. Failures in them don't have to > reflect bugs. I looked into it a bit more, and it seems that a test cannot be written without changing code - there is no way to dispatch a WM_SYSCHAR with alt key pressed. I agree that it is possible to write some kind of test for this, possibly by adding functionality to DRT, but somehow, it feels wrong to me. > I don't entirely understand why the code to call handleAccessKey couldn't just > go into keyEvent. Checking for system key is entirely platform-specific, not even wxWindows does that. I still have a feeling that a perfect implementation of this feature on Windows should use some mechanism that's separate form event dispatch.
Note You need to log in before you can comment on or make changes to this bug.