After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 690664 - x11: Ignore num lock / scroll lock for event state
x11: Ignore num lock / scroll lock for event state
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2012-12-23 09:11 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2013-01-14 17:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
x11: Ignore num lock / scroll lock for event state (4.84 KB, patch)
2012-12-23 09:11 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
x11: Ignore num lock / scroll lock for event state (5.28 KB, patch)
2013-01-07 19:45 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2012-12-23 09:11:50 UTC
See patch. We have similar code in mutter, and I don't know
any reason not to do this.
Comment 1 Jasper St. Pierre (not reading bugmail) 2012-12-23 09:11:53 UTC
Created attachment 232142 [details] [review]
x11: Ignore num lock / scroll lock for event state

This will do nothing but confuse.
Comment 2 Emmanuele Bassi (:ebassi) 2013-01-06 15:30:39 UTC
it seems that GDK is not doing this either; is there any particular reason why this should happen, except "do nothing but confuse"? why is it that Metacity/Mutter requires this code?
Comment 3 Emmanuele Bassi (:ebassi) 2013-01-06 15:36:18 UTC
the patch ooks good to me otherwise, but I'd rather have a detailed rationale in the commit message.
Comment 4 Daniel Stone 2013-01-06 16:12:29 UTC
Hmm, do you actually have Scroll Lock mapped to a modifier? It will set a virtual modifier, but you ideally want to ignore those and only look at the core modifiers, since there's a bunch of really esoteric virtual modifiers.  You'll still need to ignore Num Lock from the real modifiers though; doing so is entirely sensible.
Comment 5 Jasper St. Pierre (not reading bugmail) 2013-01-06 16:28:43 UTC
(In reply to comment #2)
> it seems that GDK is not doing this either; is there any particular reason why
> this should happen, except "do nothing but confuse"? why is it that
> Metacity/Mutter requires this code?

I assume that GDK doesn't do it out of legacy concerns, but I'm unsure.

The answer is that the state of Num Lock and Scroll Lock shouldn't affect code like this:

    if (clutter_event_get_state (event) == CLUTTER_CONTROL_MASK)
      /* do a thing */
    else
      /* do another thing */

(In reply to comment #4)
> Hmm, do you actually have Scroll Lock mapped to a modifier? It will set a
> virtual modifier, but you ideally want to ignore those and only look at the
> core modifiers, since there's a bunch of really esoteric virtual modifiers. 
> You'll still need to ignore Num Lock from the real modifiers though; doing so
> is entirely sensible.

I don't know. All I know is that mutter does this, and apparently has done this since the keybindings code was introduced in 2002:

http://git.gnome.org/browse/mutter/commit/?id=e4e200a1dc6ee6e459e5c68916669a48eda33d20

If we don't need it, we don't need it.
Comment 6 Jasper St. Pierre (not reading bugmail) 2013-01-07 19:45:20 UTC
Created attachment 232922 [details] [review]
x11: Ignore num lock / scroll lock for event state

As x11 considers num lock and scroll lock to be modifiers, code that
checks for an exact modifier combination will fail if naively done when
num lock or scroll lock are turned on. Applications that want to ignore
these modifiers will need to use XKB to manually mask out the modifier
state.

As it is very unlikely that applications will want to care about the
state of num lock or scroll lock for key press/key release events, mask
out the num lock and scroll lock keys automatically.
Comment 7 Emmanuele Bassi (:ebassi) 2013-01-14 17:47:09 UTC
Review of attachment 232922 [details] [review]:

looks good. it would be nice to have a notice in the "Release notes" section of the README.in file that notes that on X11 we mask out these locks from the modifiers starting from the 1.14 release.
Comment 8 Jasper St. Pierre (not reading bugmail) 2013-01-14 17:56:35 UTC
Attachment 232922 [details] pushed as 4691878 - x11: Ignore num lock / scroll lock for event state


I've done that, thanks!