GNOME Bugzilla – Bug 706415
CTRL+Q doesn't work anymore
Last modified: 2016-03-31 13:22:07 UTC
Seems CTRL+Q has regressed recently.
Created attachment 253303 [details] [review] app: Fix CTRL+Q and ALT+A handling For some reason we were doing: event.state & Gdk.ModifierType.MODIFIER_MASK == TARGET_MASK while Gdk.ModifierType.MODIFIER_MASK should not be involved: event.state & TARGET_MASK != 0 This change fixes the regression of CTRL+Q and ALT+A not working against recent Gtk+.
Pushing this fix already but feel free to do a review still. Attachment 253303 [details] pushed as 01c0609 - app: Fix CTRL+Q and ALT+A handling
This change does not look right. Gdk.ModifierType.MODIFIER_MASK should not hurt as it just masks bits that are internal to gdk. Doing 'event.state & TARGET_MASK != 0' will be true even if more keys than just ctrl or alt are pressed, so I suspect shift+ctrl+q will cause boxes to quit now. I suspect that (event.state & Gdk.ModifierType.MODIFIER_MASK) == Gdk.ModifierType.CONTROL_MASK) stopped working because more bits than just Gdk.ModifierType.CONTROL_MASK are set after doing (event.state & Gdk.ModifierType.MODIFIER_MASK) ? If so, which bits? Is it something we should handle, or some gtk+ bug?
(In reply to comment #3) > This change does not look right. Gdk.ModifierType.MODIFIER_MASK should not hurt > as it just masks bits that are internal to gdk. > > Doing 'event.state & TARGET_MASK != 0' will be true even if more keys than just > ctrl or alt are pressed, so I suspect shift+ctrl+q will cause boxes to quit > now. > > I suspect that (event.state & Gdk.ModifierType.MODIFIER_MASK) == > Gdk.ModifierType.CONTROL_MASK) stopped working because more bits than just > Gdk.ModifierType.CONTROL_MASK are set after doing (event.state & > Gdk.ModifierType.MODIFIER_MASK) ? If so, which bits? Is it something we should > handle, or some gtk+ bug? Thanks for setting me right. I'll investigate this further. I already checked GdkModifierType but this enum hasn't been touched in this year at least and this regression only turned up latest this year.
Created attachment 253375 [details] [review] Revert "app: Fix CTRL+Q and ALT+A handling" This reverts commit 01c0609bc3a291df67cd708f1c2bb3b30bfae04a.
Created attachment 253376 [details] [review] app: Take default modifiers into account When checking for particular modifier bit in a key event, we must take into acount any default modifiers that might be set on the event's state: https://developer.gnome.org/gtk3/stable/checklist-modifiers.html This fixes CTRL+Q and ALT+A keyboard shortcuts not working anymore.
Review of attachment 253376 [details] [review]: Thanks for the additional investigation. s/take into acount/take into account/ Any idea why this stopped working? gtk+ used not to report num lock/caps lock, and started doing so recently?
(In reply to comment #7) > Review of attachment 253376 [details] [review]: > > Thanks for the additional investigation. > s/take into acount/take into account/ > Any idea why this stopped working? gtk+ used not to report num lock/caps lock, > and started doing so recently? No clue. I asked on #gtk+ and some folks pointed me to these docs and ebassi said: <ebassi> zeenix: never do strict comparisons with a bitmask <ebassi> 9 times out of 10 it's an error
Attachment 253375 [details] pushed as b42f9a7 - Revert "app: Fix CTRL+Q and ALT+A handling" Attachment 253376 [details] pushed as 2cbbe20 - app: Take default modifiers into account