Closed
Bug 1214965
Opened 9 years ago
Closed 9 years ago
test_bug1137557.html should test .location value of Numpad keys
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
RESOLVED
FIXED
mozilla44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: masayuki, Assigned: timdream)
References
Details
(Keywords: inputmethod)
Attachments
(1 file, 2 obsolete files)
6.70 KB,
patch
|
timdream
:
review+
|
Details | Diff | Splinter Review |
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?)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → timdream
Status: NEW → ASSIGNED
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.
Flags: needinfo?(masayuki)
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'.
Flags: needinfo?(masayuki)
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
Attachment #8677898 -
Flags: review?(masayuki)
Assignee | ||
Comment 4•9 years ago
|
||
Re-push to try because of bug 1217622. https://treeherder.mozilla.org/#/jobs?repo=try&revision=98ed254d7b00
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!!
Attachment #8677898 -
Flags: review?(masayuki) → review+
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".
Flags: needinfo?(masayuki)
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.
Flags: needinfo?(masayuki)
Assignee | ||
Comment 8•9 years ago
|
||
['', 'NumpadComma'] added.
Attachment #8677898 -
Attachment is obsolete: true
Attachment #8678725 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 10•9 years ago
|
||
sorry backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=16238994&repo=mozilla-inbound
Flags: needinfo?(timdream)
Comment 11•9 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/724ce80732b9
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.
Attachment #8678725 -
Attachment is obsolete: true
Flags: needinfo?(timdream)
Attachment #8679225 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 13•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec5247a32592
Keywords: checkin-needed
Comment 14•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ec5247a32592
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in
before you can comment on or make changes to this bug.
Description
•