Closed
Bug 773651
Opened 13 years ago
Closed 13 years ago
on windows XP KeyboardEvent.location for CTRL keys is always set to DOM_KEY_LOCATION_LEFT
Categories
(Core :: Widget: Win32, defect)
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: louis.jolibois, Assigned: masayuki)
References
Details
Attachments
(1 file, 2 obsolete files)
225.78 KB,
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.1 (KHTML, like Gecko) Chrome/22.0.1201.0 Safari/537.1
Steps to reproduce:
On a windows XP platform, go to https://bugzilla.mozilla.org/attachment.cgi?id=618602 and hit both CTRL keys one after the other
Actual results:
Both keys returns the same location as being DOM_KEY_LOCATION_LEFT:
keyup: target=<INPUT>, location=DOM_KEY_LOCATION_LEFT
keydown: target=<INPUT>, location=DOM_KEY_LOCATION_LEFT
keyup: target=<INPUT>, location=DOM_KEY_LOCATION_LEFT
keydown: target=<INPUT>, location=DOM_KEY_LOCATION_LEFT
Expected results:
The right CTRL key location should have been DOM_KEY_LOCATION_RIGHT
Reporter | ||
Updated•13 years ago
|
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → masayuki
Status: UNCONFIRMED → ASSIGNED
Component: General → Widget: Win32
Ever confirmed: true
Product: Firefox → Core
Version: 16 Branch → Trunk
Assignee | ||
Comment 1•13 years ago
|
||
I'll test this patch this weekend if I have much time.
But unfortunately, currently, tryserver isn't available, sigh...
Reporter | ||
Comment 2•13 years ago
|
||
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #1)
> Created attachment 641935 [details] [diff] [review]
> Patch
>
> I'll test this patch this weekend if I have much time.
>
> But unfortunately, currently, tryserver isn't available, sigh...
For what it's worth, I just tested it and everything works fine so far on my old xp.
Thanks for the quick and efficient answer.
Assignee | ||
Comment 3•13 years ago
|
||
We shouldn't overwrite the mVirtualKeyCode with wrong scancode.
Attachment #641935 -
Attachment is obsolete: true
Assignee | ||
Comment 4•13 years ago
|
||
Jimm, could you review this quickly if you realized this request on the 15th?
This patch fix the keycode values for shift/ctrl/alt of NativeKey::mOriginalVirtualKeyCode and NativKey::mVirtualKeyCode.
mOriginalVirtualKeyCode must have VK_SHIFT/VK_CONTROL/VK_MENU.
mVirtualKeyCode must have VK_LSHIFT/VK_RSHIFT/VK_LCONTROL/VK_RCONTROL/VK_LMENU/VK_RMENU.
If the left-right distinguished key code comes, this patch sets VK_SHIFT/VK_CONTROL/VK_MENU manually and does NOT compute the mVirtualKeyCode. Instead of that, it just sets the received keycode to mVirtualKeyCode.
If the received keycode isn't one of VK_SHIFT/VK_CONTROL/VK_MENU, this patch does NOT compute the mVirtualKeyCode because the lParam might be broken value if the message is sent/posted by other applications. This patch just set the received keycode to the mVirtualKey in this case too.
If the received keycode is one of VK_SHIFT/VK_CONTROL/VK_MENU, this patch computes the left-right distinguished keycode with MapVirtualKeyEx() and scan code in lParam.
But if it's impossible on XP or WinServer2003, this patch *guess* the LR difference from the extended key flag if the received keycode is VK_CONTROL or VK_MENU. On the other hand, if the received keycode is VK_SHIFT, always use VK_LSHIFT because both shift keys are not extended keys. So, we have no hint in this case.
If the result of MapVirtualKeyEx() isn't indicate the left-right distinguished keycode of the received keycode, this patch *guess* the mVirtualKeyCode with extended key flag.
So, this patch supports the generated key messages without scancode this might help compatibility with other utils.
And for the conformation of the change, this patch adds the KeyboardEvent.location tests into test_keycodes.xul.
Attachment #642338 -
Attachment is obsolete: true
Attachment #642352 -
Flags: review?(jmathies)
Assignee | ||
Updated•13 years ago
|
![]() |
||
Comment 5•13 years ago
|
||
Comment on attachment 642352 [details] [diff] [review]
Patch
Sorry for the delay. This looks ok overall.
Attachment #642352 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 6•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/205aaea8796d
No problem, this is really my fault and I requested the review in the last weekend.
Thanks.
Target Milestone: --- → mozilla17
Comment 7•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•