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)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: louis.jolibois, Assigned: masayuki)

References

Details

Attachments

(1 file, 2 obsolete files)

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
Component: Untriaged → General
OS: Windows 7 → Windows XP
Hardware: x86_64 → x86
See Also: → 166240
Assignee: nobody → masayuki
Status: UNCONFIRMED → ASSIGNED
Component: General → Widget: Win32
Ever confirmed: true
Product: Firefox → Core
Version: 16 Branch → Trunk
Attached patch Patch (obsolete) — Splinter Review
I'll test this patch this weekend if I have much time. But unfortunately, currently, tryserver isn't available, sigh...
(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.
Attached patch Patch (obsolete) — Splinter Review
We shouldn't overwrite the mVirtualKeyCode with wrong scancode.
Attachment #641935 - Attachment is obsolete: true
Attached patch PatchSplinter Review
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)
Comment on attachment 642352 [details] [diff] [review] Patch Sorry for the delay. This looks ok overall.
Attachment #642352 - Flags: review?(jmathies) → review+
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
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.

Attachment

General

Created:
Updated:
Size: