[kmscon-devel] About keyboard shortcuts and consumed modifiers

[David Herrmann]

Hi Ran

On Sun, Mar 10, 2013 at 11:30 PM, Ran Benita <ran234 at gmail.com> wrote:
> On Sun, Mar 10, 2013 at 04:27:07PM +0100, David Herrmann wrote:
>> Hi Ran
>>
>> On Sun, Mar 10, 2013 at 11:21 AM, Ran Benita <ran234 at gmail.com> wrote:
>> > I noticed the last commit added a nice feature for dynamically
>> > increasing/decreasing the font size with a shortcut. The default is
>> > <Ctrl><+> and <Ctrl><->.
>> >
>> > When I tried it I noticed that you need to use <Ctrl><Shift><=> to get
>> > the effect of <Ctrl><+>, because on this keymap the <+> is shifted. I
>> > think though that the expected behavior is that you don't actually need
>> > to use Shift there.. I wrote some explanation on this a while back, I
>> > thought it might intrest you:
>> > http://cgit.freedesktop.org/libxkbcommon/tree/xkbcommon/xkbcommon.h?id=bb620df7aa98c129#n1323
>>
>> I am aware of the problem, but didn't have the time to fix it. Thanks
>> for pointing me to the xkb_*_consumed_*() helpers.
>
> Well in a German layout (if you use that) I see that + is not shifted,
> so you might have missed that.

Yes, indeed.

>> So to fix this bug, I basically want to call
>> xkb_state_mod_mask_remove_consumed() on the keycode+modifiers for each
>> key-event and pass this as "effective_mods" or something like this
>> along, right? And I should call this _after_ xkb_state_update_key(),
>> right?
>
> I might have confused two separate issues in my message.
>
> First to solve the issue with the + and =, I don't see a better solution
> than just adding two bindings, one one + and one with =. This boils down
> to the fact that eventually you are comparing the keysyms. Solving it in
> another way, like probing the keymap levels etc., would just be more
> complicated (but I haven't thought of it too much). Of course for
> letters (e.g. <Ctrl><A> vs. <Ctrl><a>) it's easier, because you just
> convert to lower or upper case. But with + and = it's completely
> arbitrary.

I think I understand that now. So you are basically saying the
ctrl+'-' shortcut is just _wrong_ on your layout because you want
ctrl+'='. Even if I fix the modifier-comparison, you still have to
press "Shift", but that's not what you want, right?

I could fix that by comparing all keysyms of all the levels of the
single key. However, I think that's overkill and we might run into
several other problems here. Keymaps can be weird and what happens if
the user has "+" and "-" on the same key in different levels? That
just gets nasty.
We would have to first check the active level, then the other
levels... Argh, no, I don't want to go that way. If someone has the
time to investigate into that, I'd appreciate it. But I think that's
indeed a waste of time.

I think the fix here is that you simply have to change the shortcut
via --grab-zoom-out="<ctrl>equal,<ctrl>KP_Equal" right?
A default <ctrl>minus seems legit to me. I cannot expect to find
shortcuts that are easy and simple on all layouts. And at least with
correct modifier-comparison, I still allow users to use these
shortcuts (with the drawback that they have tohold Shift or whatever).

Or what do you think is the right fix for this specific problem?
(apart from the mod-comparison fix)

> Second, about the consumed modifiers. I suggest being explicit, i.e. add
> a 'consumed_mods' field to the event, and do the comparison as described
> in the mod_index_is_consumed comment. The function to calculate this
> mask should be similar to shl_xkb_get_mods (or maybe just make this one
> return the two masks at once). Then you can just use the
> xkb_state_mod_index_is_consumed() function. The remove_consumed() function
> wouldn't work with a mask of SHL_*_MASK, because those may be different
> than the xkbcommon modifier indexes which it expects.

Maybe I wasn't clear enough, but that's what I meant. Extending
shl_xkb_get_mods() to return both fields. I was coing for all_mods and
all_but_consumed_mods. But your way "all_mods and consumed_mods" is
maybe the better choice.
Bit-operations are cheap anyway...

>> I think that's the easiest way. Any comments?
>
> Hope I haven't confused you too much :)

I still wonder how to support all the other modifiers that keymaps may
advertise. I currently hardcode the standard modifiers, but what if a
keymap also provides MOD_FOO?
If the user adds "<mod_foo>Keysym" as shortcut, I currently return an
error because the modifier is unknown. However, I should instead look
it up in the layout and use it if available. But how do I communicate
the xkb_mod_index_t between uterm-input and any input-listener? And
what happens when the layout is changed during runtime? Hm.. That
requires some thought...
Anayway, not directly related to the problem.

One more thing. Lets look only at the modifiers. The code is:

@code
(state_modifiers & ~consumed_modifiers & significant_modifiers) ==
shortcut_modifiers
@endcode

This still fails with the following example:
  state_mods = CTRL | SHIFT
  consumed_mods = SHIFT
  significant_mods = CTRL | SHIFT
  shortcut_mods = CTRL | SHIFT

In this case, the SHIFT is consumed but we also require it as part of
the shortcut. But the shift is cleared by "~consumed_mods" and the
comparison will return false.

So I recommend changing this to:

@code
(state_modifiers & ~consumed_modifiers & significant_modifiers) ==
(shortcut_modifiers & ~consumed_mods)
@endcode

This will remove consumed mods also from the shortcut.

Thanks!
David