1214965 - test_bug1137557.html should test .location value of Numpad keys

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The location of following code values should be KeyboardEvent.DOM_KEY_LOCATION_NUMPAD. We should include such test.

> 'Numpad0'
> 'Numpad1'
> 'Numpad2'
> 'Numpad3'
> 'Numpad4'
> 'Numpad5'
> 'Numpad6'
> 'Numpad7'
> 'Numpad8'
> 'Numpad9'
> 'NumpadAdd'
> 'NumpadBackspace'
> 'NumpadClear'
> 'NumpadClearEntry'
> 'NumpadComma'
> 'NumpadDecimal'
> 'NumpadDivide'
> 'NumpadEnter'
> 'NumpadEqual'
> 'NumpadHash'
> 'NumpadMemoryAdd'
> 'NumpadMemoryClear'
> 'NumpadMemoryRecall'
> 'NumpadMemoryStore'
> 'NumpadMemorySubtract'
> 'NumpadMultiply'
> 'NumpadParenLeft'
> 'NumpadParenRight'
> 'NumpadStar' (we've not supported this yet, though)
> 'NumpadSubtract'


(And also the location of left or right modifier keys too?)

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Masayuki, could you tell me what's the expected |key| values of these |code| values? I am trying to follow

http://www.w3.org/TR/DOM-Level-3-Events-code/#key-numpad-section

but I some of them don't have any useful descriptions.

> 'NumpadClear'

'Clear'?

> 'NumpadClearEntry'

'Clear'?

> 'NumpadMemoryAdd'
> 'NumpadMemoryClear'
> 'NumpadMemoryRecall'
> 'NumpadMemoryStore'
> 'NumpadMemorySubtract'

?

If they don't come with usual key values I will simply set it to '?', indicating the key outputs a question mark character.

Thanks.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think that there are no de facto standards of those keys. So, I think that the default value should be 'Unidentified'.

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: bug1214965.patch

It turned out NumpadHash is not supported yet either. It's not referenced anywhere in Gecko.

https://dxr.mozilla.org/mozilla-central/search?q=NumpadHash&redirect=false&case=true

Masayuki, please tell me if this fits the expectation of this bug. Thanks.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0271110a5dd7

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Re-push to try because of bug 1217622.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=98ed254d7b00

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8677898 [details] [diff] [review]
bug1214965.patch

>+  [['+', 'NumpadAdd'],
>+   [',', 'NumpadComma'],
>+   ['.', 'NumpadDecimal'],
>+   ['.', 'NumpadComma'],  // Locale-specific NumpadComma
>+   [',', 'NumpadDecimal'],  // Locale-specific NumpadDecimal

Could you add ['', 'NumpadComma'] case? At least on Windows, comma key on Numpad inputs a character only with some keyboard layout (e.g., Brazilian). If we will support hardware keyboard in IME, the case might be necessary.

>+   ['/', 'NumpadDivide'],
>+   ['=', 'NumpadEqual'],
>+   // ['#', 'NumpadHash'], // Not supported yet.

Indeed.

>+   ['*', 'NumpadMultiply'],
>+   ['(', 'NumpadParenLeft'],
>+   [')', 'NumpadParenRight'],
>+   // ['*', 'NumpadStar'], // Not supported yet.

Ah, yes. But this will be necessary for supporting simple phone. (The key is '*' of the simple phone.) So, please test this.


Thank you for your work!!

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #5)
> >+   // ['*', 'NumpadStar'], // Not supported yet.
> 
> Ah, yes. But this will be necessary for supporting simple phone. (The key is
> '*' of the simple phone.) So, please test this.

Could you clarify how I should test this?

navigator.mozInputMethod.sendKey({key: '*', code: 'NumpadStar'}); will simply throw because Gecko does not currently recognize "NumpadStar".

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #6)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #5)
> > >+   // ['*', 'NumpadStar'], // Not supported yet.
> > 
> > Ah, yes. But this will be necessary for supporting simple phone. (The key is
> > '*' of the simple phone.) So, please test this.
> 
> Could you clarify how I should test this?
> 
> navigator.mozInputMethod.sendKey({key: '*', code: 'NumpadStar'}); will
> simply throw because Gecko does not currently recognize "NumpadStar".

Ah, right. We've not implemented it yet, sorry. I'll cancel the comment out at implementing the code value.

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: bug1214965.patch

['', 'NumpadComma'] added.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cae63161b689

Comment 10

[Carsten Book [:Tomcat]]

sorry backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=16238994&repo=mozilla-inbound

Comment 11

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/724ce80732b9

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: bug1214965.patch

I didn't not address the review comment correctly. I did try it locally ... probably overlooked the error :'( sorry about that.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ec5247a32592

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ec5247a32592