79289 2012-02-22 15:45:58 -0800 nativeKeyCode for WebKeyboardEvent not set on Windows 2012-02-28 11:32:59 -0800 1 1 1 Unclassified WebKit UI Events 528+ (Nightly build) PC Windows 7 RESOLVED FIXED P2 Normal --- 1 garykac webkit-unassigned fishd garykac japhet pkasting rniwa webkit.review.bot oldest_to_newest 562476 0 garykac 2012-02-22 15:45:58 -0800 Background: For most platforms, the |nativeKeyCode| field in the WebKeyboardEvent is used to store the native OS key encoding and the |windowsKeyCode| is used to store the Windows virtual key code equivalent (required for the DOM key events). Windows, however, doesn't record the native key code but instead stores the Windows virtual key code in both the |nativeKeyCode| and |windowsKeyCode| fields. This results in loss of information since the Windows virtual key codes contains a subset of the information found in the native key code. For example, the Windows virtual key code cannot distinguish between a left and a right Shift key press - they are both encoded as VK_SHIFT. This hasn't caused a problem until now because: (1) DOM events want the Windows virtual key code (2) the |nativeKeyCode| is only used to calculate the |windowsKeyCode| (thus, it doesn't need to be set on Windows) However, if downstream components need more accurate information about the key press, then they need access to the native key information on all platforms. The fix for this is to simply record the key event's LPARAM in the |nativeKeyCode| field (in Source/WebKit/chromium/src/win/WebInputEventFactory.cpp). This is safe because the |nativeKeyCode| field isn't used anywhere except in platform-specific code to calculate the |windowsKeyCode|. 562482 1 128308 garykac 2012-02-22 15:53:35 -0800 Created attachment 128308 Patch 566017 2 128308 rniwa 2012-02-27 15:15:37 -0800 Comment on attachment 128308 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=128308&action=review > Source/WebKit/chromium/ChangeLog:7 > + Why are we making this change? r- due to the lack of explanation. 566833 3 129286 garykac 2012-02-28 10:17:59 -0800 Created attachment 129286 Patch 566837 4 129286 rniwa 2012-02-28 10:20:58 -0800 Comment on attachment 129286 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129286&action=review > Source/WebKit/chromium/ChangeLog:3 > + Set the key event's nativeKeyCode on Windows Chromium so that it Nit: Tab! You also need to put this description below "Reviewed by" line. > Source/WebKit/chromium/ChangeLog:15 > + this change will bring Windows in line withe other platforms. Nit: typo withe. 566838 5 129286 rniwa 2012-02-28 10:21:22 -0800 Comment on attachment 129286 Patch r- due to tab characters in the change log. 566844 6 129287 garykac 2012-02-28 10:29:33 -0800 Created attachment 129287 Patch 566849 7 129287 rniwa 2012-02-28 10:37:52 -0800 Comment on attachment 129287 Patch You're not a committer yet, right? If you want me to land it via commit-queue or are you going to ask someone to land your patch? 566850 8 garykac 2012-02-28 10:39:34 -0800 Sending it to the commit-queue would be great. Thanks! 566855 9 rniwa 2012-02-28 10:42:32 -0800 (In reply to comment #8) > Sending it to the commit-queue would be great. Thanks! Then please set cq? flag. 566860 10 129287 garykac 2012-02-28 10:48:03 -0800 Comment on attachment 129287 Patch cq? set 566915 11 129287 webkit.review.bot 2012-02-28 11:32:54 -0800 Comment on attachment 129287 Patch Clearing flags on attachment: 129287 Committed r109128: <http://trac.webkit.org/changeset/109128> 566916 12 webkit.review.bot 2012-02-28 11:32:59 -0800 All reviewed patches have been landed. Closing bug. 128308 2012-02-22 15:53:35 -0800 2012-02-28 10:17:53 -0800 Patch bug-79289-20120222155334.patch text/plain 1429 garykac SW5kZXg6IFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCShyZXZpc2lvbiAxMDg1NjkpCisrKyBTb3Vy Y2UvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEz IEBACisyMDEyLTAyLTIyICBHYXJ5IEthY21hcmNpayAgPGdhcnlrYWNAY2hyb21pdW0ub3JnPgor CisgICAgICAgIG5hdGl2ZUtleUNvZGUgZm9yIFdlYktleWJvYXJkRXZlbnQgbm90IHNldCBvbiBX aW5kb3dzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03 OTI4OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICog c3JjL3dpbi9XZWJJbnB1dEV2ZW50RmFjdG9yeS5jcHA6CisgICAgICAgIChXZWJLaXQ6OldlYklu cHV0RXZlbnRGYWN0b3J5OjprZXlib2FyZEV2ZW50KToKKwogMjAxMi0wMi0yMSAgUnlvc3VrZSBO aXdhICA8cm5pd2FAd2Via2l0Lm9yZz4KIAogICAgICAgICBSZW1vdmUgdGhlIHJlbWFpbmluZyB1 c2VzIG9mIENTU1N0eWxlRGVjbGFyYXRpb24gaW4gRWRpdG9yCkluZGV4OiBTb3VyY2UvV2ViS2l0 L2Nocm9taXVtL3NyYy93aW4vV2ViSW5wdXRFdmVudEZhY3RvcnkuY3BwCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vc3JjL3dpbi9XZWJJbnB1dEV2ZW50RmFjdG9yeS5jcHAJ KHJldmlzaW9uIDEwNjg2NCkKKysrIFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vc3JjL3dpbi9XZWJJ bnB1dEV2ZW50RmFjdG9yeS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTEwNyw3ICsxMDcsOSBAQCBX ZWJLZXlib2FyZEV2ZW50IFdlYklucHV0RXZlbnRGYWN0b3J5OjprCiAgICAgLy8gY2FsbGVyLCB3 aG8gd291bGQga25vdyBmb3Igc3VyZS4KICAgICByZXN1bHQudGltZVN0YW1wU2Vjb25kcyA9IEdl dE1lc3NhZ2VUaW1lKCkgLyAxMDAwLjA7CiAKLSAgICByZXN1bHQud2luZG93c0tleUNvZGUgPSBy ZXN1bHQubmF0aXZlS2V5Q29kZSA9IHN0YXRpY19jYXN0PGludD4od3BhcmFtKTsKKyAgICByZXN1 bHQud2luZG93c0tleUNvZGUgPSBzdGF0aWNfY2FzdDxpbnQ+KHdwYXJhbSk7CisgICAgLy8gUmVj b3JkIHRoZSBzY2FuIGNvZGUgKGFsb25nIHdpdGggb3RoZXIgY29udGV4dCBiaXRzKSBmb3IgdGhp cyBrZXkgZXZlbnQuCisgICAgcmVzdWx0Lm5hdGl2ZUtleUNvZGUgPSBzdGF0aWNfY2FzdDxpbnQ+ KGxwYXJhbSk7CiAKICAgICBzd2l0Y2ggKG1lc3NhZ2UpIHsKICAgICBjYXNlIFdNX1NZU0tFWURP V046Cg== 129286 2012-02-28 10:17:59 -0800 2012-02-28 10:29:28 -0800 Patch bug-79289-20120228101758.patch text/plain 2221 garykac SW5kZXg6IFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCShyZXZpc2lvbiAxMDg1NjkpCisrKyBTb3Vy Y2UvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI5 IEBACisyMDEyLTAyLTIyICBHYXJ5IEthY21hcmNpayAgPGdhcnlrYWNAY2hyb21pdW0ub3JnPgor CisJU2V0IHRoZSBrZXkgZXZlbnQncyBuYXRpdmVLZXlDb2RlIG9uIFdpbmRvd3MgQ2hyb21pdW0g c28gdGhhdCBpdAorCWFjdHVhbGx5IGluY2x1ZGVzIHRoZSBuYXRpdmUgc2NhbmNvZGUgYW5kIG90 aGVyIGNvbnRleHQgYml0cy4KKwlDdXJyZW50bHkgb24gV2luZG93cywgdGhlIG5hdGl2ZUtleUNv ZGUgaXMgc2V0IHRvIGJlIHRoZSBzYW1lIHZhbHVlIGFzCisJdGhlIHdpbmRvd3NLZXlDb2RlLCB3 aGljaCBpcyByZWR1bmRhbnQuCisKKwlUaGUgYWN0dWFsIG5hdGl2ZSBrZXkgaW5mb3JtYXRpb24g aXMgcmVxdWlyZWQgc28gdGhhdCBjb21wb25lbnRzIChsaWtlCisJcGx1Z2lucykgY2FuIHJlY29u c3RydWN0IHRoZSBvcmlnaW5hbCBrZXkgZXZlbnQuIFRoZSB2YWx1ZSBzdG9yZWQgaW4KKwl0aGUg d2luZG93c0tleUNvZGUgaXMgYSBXaW5kb3dzIHZpcnR1YWwga2V5IGNvZGUgd2hpY2ggZG9lcyBu b3QgcHJvdmlkZQorCXN1ZmZpY2llbnQgaW5mbyB0byBkaXNhbWJpZ3VhdGUgdGhlIGFjdHVhbCBr ZXkgYmVpbmcgcHJlc3NlZCAoZm9yCisJZXhhbXBsZSwgaXQgY2Fubm90IGRpc3Rpbmd1aXNoIGJl dHdlZW4gdGhlIGxlZnQgYW5kIHJpZ2h0IHNoaWZ0IGtleXMpLgorCisJQWxsIHBsYXRmb3JtcyBl eGNlcHQgV2luZG93cyBjdXJyZW50bHkgc2VuZCBuYXRpdmUga2V5IGV2ZW50IGluZm8sIHNvCisJ dGhpcyBjaGFuZ2Ugd2lsbCBicmluZyBXaW5kb3dzIGluIGxpbmUgd2l0aGUgb3RoZXIgcGxhdGZv cm1zLgorCisJVGhpcyBjaGFuZ2UgaXMgcGFydCBvZiBvdXIgZWZmb3J0IHRvIHNlbmQgZW5oYW5j ZWQga2V5Y29kZSBpbmZvcm1hdGlvbgorCXRvIFBlcHBlciBwbHVnaW5zIGluIENocm9taXVtLgor CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03OTI4OQor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogc3JjL3dp bi9XZWJJbnB1dEV2ZW50RmFjdG9yeS5jcHA6CisgICAgICAgIChXZWJLaXQ6OldlYklucHV0RXZl bnRGYWN0b3J5OjprZXlib2FyZEV2ZW50KToKKwogMjAxMi0wMi0yMSAgUnlvc3VrZSBOaXdhICA8 cm5pd2FAd2Via2l0Lm9yZz4KIAogICAgICAgICBSZW1vdmUgdGhlIHJlbWFpbmluZyB1c2VzIG9m IENTU1N0eWxlRGVjbGFyYXRpb24gaW4gRWRpdG9yCkluZGV4OiBTb3VyY2UvV2ViS2l0L2Nocm9t aXVtL3NyYy93aW4vV2ViSW5wdXRFdmVudEZhY3RvcnkuY3BwCj09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJj ZS9XZWJLaXQvY2hyb21pdW0vc3JjL3dpbi9XZWJJbnB1dEV2ZW50RmFjdG9yeS5jcHAJKHJldmlz aW9uIDEwNjg2NCkKKysrIFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vc3JjL3dpbi9XZWJJbnB1dEV2 ZW50RmFjdG9yeS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTEwNyw3ICsxMDcsOSBAQCBXZWJLZXli b2FyZEV2ZW50IFdlYklucHV0RXZlbnRGYWN0b3J5OjprCiAgICAgLy8gY2FsbGVyLCB3aG8gd291 bGQga25vdyBmb3Igc3VyZS4KICAgICByZXN1bHQudGltZVN0YW1wU2Vjb25kcyA9IEdldE1lc3Nh Z2VUaW1lKCkgLyAxMDAwLjA7CiAKLSAgICByZXN1bHQud2luZG93c0tleUNvZGUgPSByZXN1bHQu bmF0aXZlS2V5Q29kZSA9IHN0YXRpY19jYXN0PGludD4od3BhcmFtKTsKKyAgICByZXN1bHQud2lu ZG93c0tleUNvZGUgPSBzdGF0aWNfY2FzdDxpbnQ+KHdwYXJhbSk7CisgICAgLy8gUmVjb3JkIHRo ZSBzY2FuIGNvZGUgKGFsb25nIHdpdGggb3RoZXIgY29udGV4dCBiaXRzKSBmb3IgdGhpcyBrZXkg ZXZlbnQuCisgICAgcmVzdWx0Lm5hdGl2ZUtleUNvZGUgPSBzdGF0aWNfY2FzdDxpbnQ+KGxwYXJh bSk7CiAKICAgICBzd2l0Y2ggKG1lc3NhZ2UpIHsKICAgICBjYXNlIFdNX1NZU0tFWURPV046Cg== 129287 2012-02-28 10:29:33 -0800 2012-02-28 11:32:54 -0800 Patch bug-79289-20120228102932.patch text/plain 2378 garykac SW5kZXg6IFNvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCShyZXZpc2lvbiAxMDg1NjkpCisrKyBTb3Vy Y2UvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDMw IEBACisyMDEyLTAyLTIyICBHYXJ5IEthY21hcmNpayAgPGdhcnlrYWNAY2hyb21pdW0ub3JnPgor CisgICAgICAgIFtjaHJvbWl1bV0gU2V0IG5hdGl2ZUtleUNvZGUgdG8gaW5jbHVkZSBXaW5kb3dz IHNjYW4gY29kZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9NzkyODkKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBTZXQgdGhlIGtleSBldmVudCdzIG5hdGl2ZUtleUNvZGUgb24gV2luZG93cyBDaHJvbWl1bSBz byB0aGF0IGl0CisgICAgICAgIGFjdHVhbGx5IGluY2x1ZGVzIHRoZSBuYXRpdmUgc2NhbmNvZGUg YW5kIG90aGVyIGNvbnRleHQgYml0cy4KKyAgICAgICAgQ3VycmVudGx5IG9uIFdpbmRvd3MsIHRo ZSBuYXRpdmVLZXlDb2RlIGlzIHNldCB0byBiZSB0aGUgc2FtZSB2YWx1ZSBhcworICAgICAgICB0 aGUgd2luZG93c0tleUNvZGUsIHdoaWNoIGlzIHJlZHVuZGFudC4KKworICAgICAgICBUaGUgYWN0 dWFsIG5hdGl2ZSBrZXkgaW5mb3JtYXRpb24gaXMgcmVxdWlyZWQgc28gdGhhdCBjb21wb25lbnRz IChsaWtlCisgICAgICAgIHBsdWdpbnMpIGNhbiByZWNvbnN0cnVjdCB0aGUgb3JpZ2luYWwga2V5 IGV2ZW50LiBUaGUgdmFsdWUgc3RvcmVkIGluCisgICAgICAgIHRoZSB3aW5kb3dzS2V5Q29kZSBp cyBhIFdpbmRvd3MgdmlydHVhbCBrZXkgY29kZSB3aGljaCBkb2VzIG5vdCBwcm92aWRlCisgICAg ICAgIHN1ZmZpY2llbnQgaW5mbyB0byBkaXNhbWJpZ3VhdGUgdGhlIGFjdHVhbCBrZXkgYmVpbmcg cHJlc3NlZCAoZm9yCisgICAgICAgIGV4YW1wbGUsIGl0IGNhbm5vdCBkaXN0aW5ndWlzaCBiZXR3 ZWVuIHRoZSBsZWZ0IGFuZCByaWdodCBzaGlmdCBrZXlzKS4KKworICAgICAgICBBbGwgcGxhdGZv cm1zIGV4Y2VwdCBXaW5kb3dzIGN1cnJlbnRseSBzZW5kIG5hdGl2ZSBrZXkgZXZlbnQgaW5mbywg c28KKyAgICAgICAgdGhpcyBjaGFuZ2Ugd2lsbCBicmluZyBXaW5kb3dzIGluIGxpbmUgd2l0aCBv dGhlciBwbGF0Zm9ybXMuCisKKyAgICAgICAgVGhpcyBjaGFuZ2UgaXMgcGFydCBvZiBvdXIgZWZm b3J0IHRvIHNlbmQgZW5oYW5jZWQga2V5Y29kZSBpbmZvcm1hdGlvbgorICAgICAgICB0byBQZXBw ZXIgcGx1Z2lucyBpbiBDaHJvbWl1bS4KKworICAgICAgICAqIHNyYy93aW4vV2ViSW5wdXRFdmVu dEZhY3RvcnkuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJJbnB1dEV2ZW50RmFjdG9yeTo6a2V5 Ym9hcmRFdmVudCk6CisKIDIwMTItMDItMjEgIFJ5b3N1a2UgTml3YSAgPHJuaXdhQHdlYmtpdC5v cmc+CiAKICAgICAgICAgUmVtb3ZlIHRoZSByZW1haW5pbmcgdXNlcyBvZiBDU1NTdHlsZURlY2xh cmF0aW9uIGluIEVkaXRvcgpJbmRleDogU291cmNlL1dlYktpdC9jaHJvbWl1bS9zcmMvd2luL1dl YklucHV0RXZlbnRGYWN0b3J5LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0L2Nocm9t aXVtL3NyYy93aW4vV2ViSW5wdXRFdmVudEZhY3RvcnkuY3BwCShyZXZpc2lvbiAxMDY4NjQpCisr KyBTb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy93aW4vV2ViSW5wdXRFdmVudEZhY3RvcnkuY3Bw CSh3b3JraW5nIGNvcHkpCkBAIC0xMDcsNyArMTA3LDkgQEAgV2ViS2V5Ym9hcmRFdmVudCBXZWJJ bnB1dEV2ZW50RmFjdG9yeTo6awogICAgIC8vIGNhbGxlciwgd2hvIHdvdWxkIGtub3cgZm9yIHN1 cmUuCiAgICAgcmVzdWx0LnRpbWVTdGFtcFNlY29uZHMgPSBHZXRNZXNzYWdlVGltZSgpIC8gMTAw MC4wOwogCi0gICAgcmVzdWx0LndpbmRvd3NLZXlDb2RlID0gcmVzdWx0Lm5hdGl2ZUtleUNvZGUg PSBzdGF0aWNfY2FzdDxpbnQ+KHdwYXJhbSk7CisgICAgcmVzdWx0LndpbmRvd3NLZXlDb2RlID0g c3RhdGljX2Nhc3Q8aW50Pih3cGFyYW0pOworICAgIC8vIFJlY29yZCB0aGUgc2NhbiBjb2RlIChh bG9uZyB3aXRoIG90aGVyIGNvbnRleHQgYml0cykgZm9yIHRoaXMga2V5IGV2ZW50LgorICAgIHJl c3VsdC5uYXRpdmVLZXlDb2RlID0gc3RhdGljX2Nhc3Q8aW50PihscGFyYW0pOwogCiAgICAgc3dp dGNoIChtZXNzYWdlKSB7CiAgICAgY2FzZSBXTV9TWVNLRVlET1dOOgo=