Closed Bug 1300937 Opened 3 years ago Closed 3 years ago

test_keycodes.xul should check KeyboardEvent.key value (and KeyboardEvent.code too?)

Categories

(Core :: Widget, defect, P4)

defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox51 --- fix-optional
firefox52 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: intl, Whiteboard: tpi:+)

Attachments

(4 files)

Priority: -- → P4
Whiteboard: tpi:+
Comment on attachment 8790783 [details]
Bug 1300937 part.1 Check KeyboardEvent.key and KeyboardEvent.code in test_keycodes.xul

https://reviewboard.mozilla.org/r/78450/#review77076

rs+
Attachment #8790783 - Flags: review?(bugs) → review+
Comment on attachment 8790784 [details]
Bug 1300937 part.2 Automated tests which synthesize native key events on Windows should specify scan code value explicitly

https://reviewboard.mozilla.org/r/78452/#review77430

::: testing/mochitest/tests/SimpleTest/NativeKeyCodes.js:16
(Diff revision 1)
> +// FYI: Don't define scan code here for printable keys, numeric keys and
> +//      IME keys because they depend on active keyboard layout.
>  
> -const WIN_VK_LBUTTON                    = 0x01;
> -const WIN_VK_RBUTTON                    = 0x02;
> -const WIN_VK_CANCEL                     = 0x03;
> +const WIN_VK_LBUTTON                    = 0x00000001;
> +const WIN_VK_RBUTTON                    = 0x00000002;
> +const WIN_VK_CANCEL                     = 0xE0460003;

I just rely on that all this (code & 0xFFFF0000 >> 16) stuff is correct there. I don't know where to look at that is right.
Comment on attachment 8790784 [details]
Bug 1300937 part.2 Automated tests which synthesize native key events on Windows should specify scan code value explicitly

https://reviewboard.mozilla.org/r/78452/#review77432
Attachment #8790784 - Flags: review?(bugs) → review+
Comment on attachment 8790785 [details]
Bug 1300937 part.3 NativeKeyCodes.js should specify scan code to WIN_VK_ABNT_C1 explicitly for avoiding (perhaps) a bug of MapVirtualKeyEx() API

https://reviewboard.mozilla.org/r/78454/#review77438
Attachment #8790785 - Flags: review?(bugs) → review+
Comment on attachment 8790784 [details]
Bug 1300937 part.2 Automated tests which synthesize native key events on Windows should specify scan code value explicitly

https://reviewboard.mozilla.org/r/78452/#review77430

> I just rely on that all this (code & 0xFFFF0000 >> 16) stuff is correct there. I don't know where to look at that is right.

All of them are mapped in this file (CODE_MAP_WIN):
https://dxr.mozilla.org/mozilla-central/source/widget/NativeKeyToDOMCodeName.h

Anyway, if this mapping in NativeKeyCodes.js were wrong, it'd be detected by test_keycodes.xul in most keys.
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/6a8bf7596f42
part.1 Check KeyboardEvent.key and KeyboardEvent.code in test_keycodes.xul r=smaug
https://hg.mozilla.org/integration/autoland/rev/be88a60abb7a
part.2 Automated tests which synthesize native key events on Windows should specify scan code value explicitly r=smaug
https://hg.mozilla.org/integration/autoland/rev/e94d6d577103
part.3 NativeKeyCodes.js should specify scan code to WIN_VK_ABNT_C1 explicitly for avoiding (perhaps) a bug of MapVirtualKeyEx() API r=smaug
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=3540333&repo=autoland
Flags: needinfo?(masayuki)
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f8ea39a95d8a
Backed out changeset e94d6d577103 
https://hg.mozilla.org/integration/autoland/rev/ef298f2e9e09
Backed out changeset be88a60abb7a 
https://hg.mozilla.org/integration/autoland/rev/5305fb3b56f2
Backed out changeset 6a8bf7596f42 for m-oth test failures
Odd. Why did those passed on tryserver??

I guess some patches might not be the latest patches... I'll check it tomorrow.
Flags: needinfo?(masayuki)
Hmm, according to the log, it fails only computing scan code values for extended keys.
Oh, I see. On tryserver, mochitest-other never runs on WinXP but not so on autoland server.
> Windows XP 32-bit autoland debug test mochitest-other	e94d6d5771038e904d83722fad5718f4f68603a5 	Warnings
> Windows 7 32-bit autoland debug test mochitest-other	e94d6d5771038e904d83722fad5718f4f68603a5 	Success
> Windows XP 32-bit autoland opt test mochitest-other	e94d6d5771038e904d83722fad5718f4f68603a5 	Warnings
> Windows 8 64-bit autoland opt test mochitest-other	e94d6d5771038e904d83722fad5718f4f68603a5 	Success
> Windows 7 32-bit autoland opt test mochitest-other	e94d6d5771038e904d83722fad5718f4f68603a5 	Success
> Windows 8 64-bit autoland debug test mochitest-other e94d6d5771038e904d83722fad5718f4f68603a5 	Success

Failed only on WinXP.
Oh... The orange detects an existing bug of KeyboardEvent.code on WinXP.

I'll add a patch for fixing it as part.4.
Comment on attachment 8791883 [details]
Bug 1300937 part.4 NativeKey::GetScanCodeWithExtendedFlag() should return 0xE0XX even on WinXP or WinServer2003

https://reviewboard.mozilla.org/r/79174/#review78332
Attachment #8791883 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/70f946d5d528
part.1 Check KeyboardEvent.key and KeyboardEvent.code in test_keycodes.xul r=smaug
https://hg.mozilla.org/integration/autoland/rev/a1f7f2079792
part.2 Automated tests which synthesize native key events on Windows should specify scan code value explicitly r=smaug
https://hg.mozilla.org/integration/autoland/rev/04e2229c5548
part.3 NativeKeyCodes.js should specify scan code to WIN_VK_ABNT_C1 explicitly for avoiding (perhaps) a bug of MapVirtualKeyEx() API r=smaug
https://hg.mozilla.org/integration/autoland/rev/6ed836888946
part.4 NativeKey::GetScanCodeWithExtendedFlag() should return 0xE0XX even on WinXP or WinServer2003 r=m_kato
Duplicate of this bug: 1305845
Mark 51 as fix-optional. If it's worth uplift to 51, feel free to nominate it.
You need to log in before you can comment on or make changes to this bug.