Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Reporter
	Description • 9 years ago

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?)

	Tim Guan-tin Chien [:timdream] (please needinfo) Assignee
	Comment 1 • 9 years ago

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.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Reporter
	Comment 2 • 9 years ago

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

	Tim Guan-tin Chien [:timdream] (please needinfo) Assignee
	Comment 3 • 9 years ago

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

	Tim Guan-tin Chien [:timdream] (please needinfo) Assignee
	Comment 4 • 9 years ago

Re-push to try because of bug 1217622.

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

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Reporter
	Comment 5 • 9 years ago

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!!

	Tim Guan-tin Chien [:timdream] (please needinfo) Assignee
	Comment 6 • 9 years ago

(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".

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Reporter
	Comment 7 • 9 years ago

(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.

	Tim Guan-tin Chien [:timdream] (please needinfo) Assignee
	Comment 8 • 9 years ago

['', 'NumpadComma'] added.

	Pulsebot
	Comment 9 • 9 years ago

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

	Carsten Book [:Tomcat]
	Comment 10 • 9 years ago

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

	Pulsebot
	Comment 11 • 9 years ago

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

	Tim Guan-tin Chien [:timdream] (please needinfo) Assignee
	Comment 12 • 9 years ago

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

	Pulsebot
	Comment 13 • 9 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 14 • 9 years ago
bugherder

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