Closed
Bug 377478
Opened 17 years ago
Closed 17 years ago
Implement a way to get the toggled keyboard states (e.g., Caps Lock, Num Lock and Scroll Lock)
Categories
(Core :: Widget, enhancement)
Core
Widget
Tracking
()
RESOLVED
FIXED
People
(Reporter: masayuki, Assigned: masayuki)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 13 obsolete files)
18.44 KB,
patch
|
masayuki
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
Bug 259059 is very interesting bug for usability. I don't know how to implement it on front end. But I have an idea for the back end part. # Note that we need to be fixed bug 358899. # Because this patch conflicts to the patch. I confirmed that the patch works fine on WinXP/Fedora6/MacOS X 10.4. I hope that some testers test the patch on different environments. Especially on MacOS X 10.3.
Assignee | ||
Updated•17 years ago
|
Summary: Implement to the way for getting the toggled keyboard state (e.g., Caps Lock, Num Lock and Scroll Lock) → Implement the way for getting the toggled keyboard states (e.g., Caps Lock, Num Lock and Scroll Lock)
Assignee | ||
Comment 1•17 years ago
|
||
The editors out the keyboard states at getting the focus. You can check it for testing the patch.
Comment 2•17 years ago
|
||
Nice work. The gtk2 bits doesn't work for me though. I'm using Kubuntu 6.10 and on this system the /dev/console file is only readable by root: # ls -l /dev/console crw------- 1 root root 5, 1 2007-04-14 11:39 /dev/console Also, I don't think using /dev/console would work when using a remote X display anyway. I think we can use gdk_keyboard_get_modifiers instead: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/widget/src/gtk2/nsWindow.cpp&rev=1.211&root=/cvsroot&mark=1534-1542#1534
Comment 3•17 years ago
|
||
This works for CapsLock and NumLock... I'm not sure what to do about ScrollLock though...
Assignee | ||
Comment 4•17 years ago
|
||
Thank you Mats! It's great. We should test this patch on MacOS X 10.3 too. If it works fine on MacOS X 10.3, it will go to review process. # It seems that the major mapping is the Scroll Lock to mod5. But it is customizable, and I think that we should prefer the system mapping.
Attachment #261544 -
Attachment is obsolete: true
Attachment #261549 -
Attachment is obsolete: true
Assignee | ||
Comment 5•17 years ago
|
||
And note that we don't support the Scroll Lock in current trunk, so, we need to improve the mapping issue on GTK2 before we support scroll lock.
I tried patch rv0.1. The state of "Caps Lock" is received. But the state of "Num Lock" is not received. There might be a problem of the difference between a usual keyboard and the note type machine. Of course, the difference of OS version is thought. Mac OS X 10.3.9 PowerBook G4
LastModifierFlags was output. There was no change in LastModifierFlags though "Num Lock" was pushed. The change appears to LastModifierFlags when keys other than "Num Lock" are pushed of course. "Num Lock" cannot be at least detected with PowerBook by this method. "Num Lock" is ON. but the change is not in lastModifierFlags. lastModifierFlags:256 NumLock: OFF CapsLock: OFF "Caps Lock" is ON. lastModifierFlags:65792 NumLock: OFF CapsLock: ON
Updated•17 years ago
|
Summary: Implement the way for getting the toggled keyboard states (e.g., Caps Lock, Num Lock and Scroll Lock) → Implement a way to get the toggled keyboard states (e.g., Caps Lock, Num Lock and Scroll Lock)
Assignee | ||
Comment 8•17 years ago
|
||
Sorry, for the delay. Can somebody test this on MacOS X 10.3?
Attachment #261552 -
Attachment is obsolete: true
Assignee | ||
Comment 9•17 years ago
|
||
Er, on 10.3, the patch doesn't return the NUM_LOCK state. But on other versions, it returns the NUM_LOCK state.
Comment 10•17 years ago
|
||
(In reply to comment #8) > Created an attachment (id=263505) [details] > Patch rv1.0 > Can somebody test this on MacOS X 10.3? > This patch was tried. The status of "Caps Lock" has returned normally. Mac OS X 10.3.9
Assignee | ||
Comment 11•17 years ago
|
||
Thank you, the NUM_LOCK state is not printed? If so, it works fine.
Comment 12•17 years ago
|
||
(In reply to comment #11) > Thank you, the NUM_LOCK state is not printed? If so, it works fine. > The display of NumLock doesn't change by turning off. There is no problem.
Comment 13•17 years ago
|
||
(In reply to comment #12) > (In reply to comment #11) > > Thank you, the NUM_LOCK state is not printed? If so, it works fine. > > > The display of NumLock doesn't change by turning off. > There is no problem. > oops... The check on the version of OS passes by because the status of NumLock is displayed. + if (nsToolkit::OSXVersion() == MAC_OS_X_VERSION_10_3_HEX) { + // XXX 10.3 doesn't return the NUM_LOCK state. + *aGottenStates = nsIKBStateControl::CAPS_LOCK_STATE; + } else {
Comment 14•17 years ago
|
||
(In reply to comment #13) > + if (nsToolkit::OSXVersion() == MAC_OS_X_VERSION_10_3_HEX) { > + // XXX 10.3 doesn't return the NUM_LOCK state. > + *aGottenStates = nsIKBStateControl::CAPS_LOCK_STATE; > + } else { > NsToolkit::OSXVersion() was displayed in debug. The result is as follows. OSXVersion:1039 I think that the check on the version of OS should do as follows. if (nsToolkit::OSXVersion() < MAC_OS_X_VERSION_10_4_HEX
Assignee | ||
Comment 15•17 years ago
|
||
Thank you, Hiro-san. I have a few works for GTK2, after it, I'll request the reviews, thanks.
Attachment #263505 -
Attachment is obsolete: true
Assignee | ||
Comment 16•17 years ago
|
||
Let's go to review process. Mats: Would you review the GTK2 part? I think that the relation between mod_mask and actual keys should be gotten dynamically. But I cannot find the way, therefore, I create the prefs for the customized environments.
Attachment #263585 -
Attachment is obsolete: true
Attachment #264607 -
Flags: review?(mats.palmgren)
Assignee | ||
Comment 17•17 years ago
|
||
Comment on attachment 264607 [details] [diff] [review] Patch rv1.2 Ere: Would you review the Win32 part? The win32 part is simple :-)
Attachment #264607 -
Flags: review?(emaijala)
Assignee | ||
Comment 18•17 years ago
|
||
Comment on attachment 264607 [details] [diff] [review] Patch rv1.2 Josh: Would you review the cocoa part? We cannot get the keyboard LED state dynamically. Therefore, I use the dummy event for getting the states. It works fine on 10.4. But on 10.3, the NumLock state is not set to the dummy events. But we can get the CapsLock state by this way, it is important for the blocking bug.
Attachment #264607 -
Flags: review?(joshmoz)
Comment 19•17 years ago
|
||
Comment on attachment 264607 [details] [diff] [review] Patch rv1.2 r=emaijala for the Windows part.
Attachment #264607 -
Flags: review?(emaijala) → review+
Comment 20•17 years ago
|
||
Comment on attachment 264607 [details] [diff] [review] Patch rv1.2 (In reply to comment #16) > I think that the relation between mod_mask and actual keys should be gotten > dynamically. It can be found with XGetKeyboardMapping and XGetModifierMapping: http://www.die.net/doc/linux/man/man3/xchangekeyboardmapping.3.html I've made a patch for that, so I'm marking review- for the GTK2 part.
Attachment #264607 -
Flags: review?(mats.palmgren) → review-
Comment 21•17 years ago
|
||
Attachment #264632 -
Flags: review?(masayuki)
Comment 22•17 years ago
|
||
BTW, you can use the xmodmap to add/remove mappings for testing, like so: xmodmap -e 'add mod3 = Scroll_Lock' xmodmap -e 'remove mod3 = Scroll_Lock' On my Ubuntu edgy system Scroll_Lock wasn't mapped at all by default. Same for a default install of Fedora 6.
Assignee | ||
Comment 23•17 years ago
|
||
Comment on attachment 264632 [details] [diff] [review] New gtk2 patch great! thank you Mats!
Attachment #264632 -
Flags: review?(masayuki) → review+
Assignee | ||
Comment 24•17 years ago
|
||
merging the attachment 264607 [details] [diff] [review] and attachment 264632 [details] [diff] [review].
Attachment #264607 -
Attachment is obsolete: true
Attachment #264632 -
Attachment is obsolete: true
Attachment #264673 -
Flags: review?(joshmoz)
Attachment #264607 -
Flags: review?(joshmoz)
Assignee | ||
Comment 25•17 years ago
|
||
I tested the Mat's patch on Fedora6/CentOS4. And I got feedbacks from jp testers, they are tested on Ubuntu and Debian. The patch is great.
Comment 26•17 years ago
|
||
Comment on attachment 264673 [details] [diff] [review] Patch rv1.3 I think it would be best to use carbon here instead of messing around with fake cocoa events. From the Event Manager reference: GetCurrentEventKeyModifiers or GetCurrentKeyModifiers You probably want the latter one.
Attachment #264673 -
Flags: review?(joshmoz) → review-
Assignee | ||
Comment 27•17 years ago
|
||
Thank you, josh. The API works fine on 10.4. I need the confirming on 10.3.
Attachment #264673 -
Attachment is obsolete: true
Comment 28•17 years ago
|
||
(In reply to comment #27) > Created an attachment (id=264879) [details] > Patch rv1.4 > > Thank you, josh. The API works fine on 10.4. I need the confirming on 10.3. > Nakano-san, if bug380499 doesn't do FIXED, it is not possible to confirm it with 10.3...
Assignee | ||
Comment 29•17 years ago
|
||
(In reply to comment #28) > (In reply to comment #27) > > Created an attachment (id=264879) [details] [details] > > Patch rv1.4 > > > > Thank you, josh. The API works fine on 10.4. I need the confirming on 10.3. > > > > Nakano-san, if bug380499 doesn't do FIXED, it is not possible to confirm it > with 10.3... Cannot use --disable-airbag? # sorry, I'll be out of town until next week...
Comment 30•17 years ago
|
||
(In reply to comment #28) > Nakano-san, if bug380499 doesn't do FIXED, it is not possible to confirm it > with 10.3... > The patch was confirmed with Camino. The result in 10.3 is the same as Patch rv1.3. (Only Caps Lock obtains the state. ) I think that you may disregard Num Lock as much as Patch rv1.3 in 10.3.
Assignee | ||
Comment 31•17 years ago
|
||
Thank you, Hiro-san for your test. And sorry for the delay. Josh: Would you rereview this?
Attachment #264879 -
Attachment is obsolete: true
Attachment #265811 -
Flags: review?(joshmoz)
Comment 32•17 years ago
|
||
Comment on attachment 265811 [details] [diff] [review] Patch rv1.5 nsChildView::GetToggledKeyStates A couple questions about this method. Can we just cache aGottenStates since it is always the same? Maybe it isn't going to be called often enough for it to matter? Secondly, say aStates has the caps lock flag set when it gets passed in. You don't unset it if caps lock isn't down. Is this intended, and if so could you please document it?
Assignee | ||
Comment 33•17 years ago
|
||
(In reply to comment #32) > (From update of attachment 265811 [details] [diff] [review]) > nsChildView::GetToggledKeyStates > > A couple questions about this method. Can we just cache aGottenStates since it > is always the same? Maybe it isn't going to be called often enough for it to > matter? Yes. aGottenStates is not related to any status on Mac, therefore, we can do. > Secondly, say aStates has the caps lock flag set when it gets passed in. You > don't unset it if caps lock isn't down. Is this intended, and if so could you > please document it? I cannot understand what you said. This method returns the states of keyboard LEDs for CapsLock and NumLock.
Comment 34•17 years ago
|
||
(In reply to comment #32) > (From update of attachment 265811 [details] [diff] [review]) > nsChildView::GetToggledKeyStates > > Can we just cache aGottenStates since it is always the same? It's just an assignment of a const value - why complicate the code with caching? > Secondly, say aStates has the caps lock flag set when it gets passed in. You > don't unset it if caps lock isn't down. GetToggledKeyStates() starts out by doing "*aStates = 0" on all platforms where it's implemented as far as I can tell.
Comment 35•17 years ago
|
||
Comment on attachment 265811 [details] [diff] [review] Patch rv1.5 Sorry, I missed the part where zero gets assigned to the outparam.
Attachment #265811 -
Flags: review?(joshmoz) → review+
Assignee | ||
Comment 36•17 years ago
|
||
Comment on attachment 265811 [details] [diff] [review] Patch rv1.5 roc: would you sr this? # the editor part will drop at checking-in.
Attachment #265811 -
Flags: superreview?(roc)
Comment 37•17 years ago
|
||
Is there any way to get the status of Scroll Lock as well? I am an extension developer, and it would be very useful to me if I can check to see if the user has scroll lock turned on or not. Thanks in advance for looking at this issue.
Assignee | ||
Comment 38•17 years ago
|
||
See the patch, the patch can get the scroll lock state on except Mac. But the patch *cannot* use from extensions directly.
+ * 'ON', i.e., the relative LED of the keyboard should be turned on. "is", not "should be" +NS_IMETHODIMP nsWidget::GetToggledKeyStates(PRUint32* aStates, + PRUint32* aGottenStates) { + return NS_ERROR_NOT_IMPLEMENTED; + } Can't these just return 0,0, which means "we don't know anything"? Otherwise the patch looks fine. But I wonder if a slightly better interface would take a *_LOCK_STATE enum as a parameter and just return whether that is on, off, or unknown. That would avoid some messing around with masks.
Assignee | ||
Comment 40•17 years ago
|
||
roc: How about this style?
Attachment #265811 -
Attachment is obsolete: true
Attachment #268263 -
Flags: superreview?(roc)
Attachment #268263 -
Flags: review+
Attachment #265811 -
Flags: superreview?(roc)
Yeah, I like this better. Maybe the result could just be a PRBool and we could return NS_ERROR_UNIMPLEMENTED if the state is unknown? (Or NS_ERROR_FAILURE depending on the reason?)
Assignee | ||
Comment 42•17 years ago
|
||
er, O.K. I'll attach the new patch tomorrow.
Assignee | ||
Comment 43•17 years ago
|
||
Attachment #268263 -
Attachment is obsolete: true
Attachment #268320 -
Flags: superreview?(roc)
Attachment #268320 -
Flags: review+
Attachment #268263 -
Flags: superreview?(roc)
Assignee | ||
Comment 44•17 years ago
|
||
Sorry for the spam, updating a comment.
Attachment #268320 -
Attachment is obsolete: true
Attachment #268321 -
Flags: superreview?(roc)
Attachment #268321 -
Flags: review+
Attachment #268320 -
Flags: superreview?(roc)
+ *aLEDState = (modifierFlags & key) ? PR_TRUE : PR_FALSE; just use (modifierFlags & key) != 0 (and again below, twice). + if (syms[j] == GDK_Caps_Lock) { use "switch (syms[j])" + if (aKeyCode != NS_VK_CAPS_LOCK && + aKeyCode != NS_VK_NUM_LOCK && + aKeyCode != NS_VK_SCROLL_LOCK) + return NS_ERROR_NOT_IMPLEMENTED; Remove this code. We'll detect this later in the switch anyway. + switch (aKeyCode) { + case NS_VK_CAPS_LOCK: key = VK_CAPITAL; break; + case NS_VK_NUM_LOCK: key = VK_NUMLOCK; break; + case NS_VK_SCROLL_LOCK: key = VK_SCROLL; break; + default: + return NS_ERROR_NOT_IMPLEMENTED; + } Aren't these constants equal? I think MapFromNativeToDOM requires that they are, so you can remove this code. I guess that means Windows can support detecting any key state in this code, but that's fine.
Assignee | ||
Comment 46•17 years ago
|
||
Attachment #268321 -
Attachment is obsolete: true
Attachment #268486 -
Flags: superreview?(roc)
Attachment #268486 -
Flags: review+
Attachment #268321 -
Flags: superreview?(roc)
Attachment #268486 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Comment 47•17 years ago
|
||
checked-in.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 48•16 years ago
|
||
I would like to be able to access this functionality from (Chrome) JS, and extend it to include modifier keys (like shift, alt, ctrl, meta). Am I right in thinking that right now, it's not directly usable by Chrome JS?
Yes.
You need to log in
before you can comment on or make changes to this bug.
Description
•