StreetLight
/
libxkbcommon
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

symbols: fix real/alias key merge ordering bug 

Background:

The CopySymbolsDef has a comment on a couple of lines which supposedly
fixed a bug:

    /*
     * kt_index[i] may have been set by a previous run (if we have two
     * layouts specified). Let's not overwrite it with the ONE_LEVEL
     * default group if we dont even have keys for this group anyway.
     *
     * FIXME: There should be a better fix for this.
     */
    if (!darray_empty(groupi->levels))
        key->kt_index[i] = types[i];

But neither the comment nor the fix make any sense, because the kt_index
is indexed per group, i.e. each group gets its own type.
The original xkbcomp commit which added this (36fecff58) points to this
bug: https://bugzilla.redhat.com/show_bug.cgi?id=436626
which complains about -layout "ru,us" -variant "phonetic," not working
properly. And indeed when we try:
    sudo ./test/interactive -l ru,us -v
the first group doesn't get any syms for the main keys.

The problem (Clearly the fix above is useless):

The ru(phonetic) map is specified using aliases, e.g. LatQ, LatW instead
of AD01, AD02, etc. When combined with another layout which uses the
real names (AD01, AD02), the symbols code should recognize they are the
same key and merge them into one KeyInfo. The current code does that,
but it doesn't catch the case where the alias was processes *before* the
real one; so we get two KeyInfo's and the later one wins. So e.g. the
ru(phonetic) symbols are ignored.

The fix:

Before adding a new KeyInfo to the keys array, always replace its name
by the real name, which avoids the entire issue. Luckily this is done
pretty late so most error messages should still show the alias name.

Signed-off-by: Ran Benita <ran234@gmail.com>
master
Ran Benita 2012-09-12 23:51:19 +03:00
parent 1d6bffc904
commit 3abfe83e11
2 changed files with 41 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
src/xkbcomp/symbols.c
View File

@ -493,15 +493,19 @@ AddKeySymbols(SymbolsInfo *info, KeyInfo *keyi)
unsigned long real_name;
KeyInfo *iter, *new;
/*
* Don't keep aliases in the keys array; this guarantees that
* searching for keys to merge with by straight comparison (see the
* following loop) is enough, and we won't get multiple KeyInfo's
* for the same key because of aliases.
*/
if (FindKeyNameForAlias(info->keymap, keyi->name, &real_name))
keyi->name = real_name;
darray_foreach(iter, info->keys)
if (iter->name == keyi->name)
return MergeKeys(info, iter, keyi);
if (FindKeyNameForAlias(info->keymap, keyi->name, &real_name))
darray_foreach(iter, info->keys)
if (iter->name == real_name)
return MergeKeys(info, iter, keyi);
darray_resize0(info->keys, darray_size(info->keys) + 1);
new = &darray_item(info->keys, darray_size(info->keys) - 1);
return CopyKeyInfo(keyi, new, true);
@ -1499,11 +1503,14 @@ CopySymbolsDef(SymbolsInfo *info, KeyInfo *keyi)
xkb_group_index_t i, nGroups;
xkb_level_index_t width, tmp;
struct xkb_key_type * type;
bool haveActions, autoType;
unsigned types[XKB_NUM_GROUPS];
bool haveActions;
unsigned int symIndex = 0;
key = FindNamedKey(keymap, keyi->name, true);
/*
* The name is guaranteed to be real and not an alias (see
* AddKeySymbols), so 'false' is safe here.
*/
key = FindNamedKey(keymap, keyi->name, false);
if (!key) {
log_vrb(info->keymap->ctx, 5,
"Key %s not found in keycodes; Symbols ignored\n",
@ -1515,12 +1522,11 @@ CopySymbolsDef(SymbolsInfo *info, KeyInfo *keyi)
width = 0;
for (i = nGroups = 0; i < XKB_NUM_GROUPS; i++) {
GroupInfo *groupi = &keyi->groups[i];
bool autoType = false;
if (i + 1 > nGroups && groupi->defined)
nGroups = i + 1;
autoType = false;
if (!haveActions) {
LevelInfo *leveli;
darray_foreach(leveli, groupi->levels) {
@ -1547,7 +1553,7 @@ CopySymbolsDef(SymbolsInfo *info, KeyInfo *keyi)
LongKeyNameText(keyi->name));
}
if (FindNamedType(keymap, groupi->type, &types[i])) {
if (FindNamedType(keymap, groupi->type, &key->kt_index[i])) {
if (!autoType || darray_size(groupi->levels) > 2)
key->explicit_groups |= (1 << i);
}
@ -1561,11 +1567,11 @@ CopySymbolsDef(SymbolsInfo *info, KeyInfo *keyi)
* Index 0 is guaranteed to contain something, usually
* ONE_LEVEL or at least some default one-level type.
*/
types[i] = 0;
key->kt_index[i] = 0;
}
/* if the type specifies fewer levels than the key has, shrink the key */
type = &keymap->types[types[i]];
type = &keymap->types[key->kt_index[i]];
if (type->num_levels < darray_size(groupi->levels)) {
log_vrb(info->keymap->ctx, 1,
"Type \"%s\" has %d levels, but %s has %d levels; "
@ -1598,16 +1604,6 @@ CopySymbolsDef(SymbolsInfo *info, KeyInfo *keyi)
for (i = 0; i < nGroups; i++) {
GroupInfo *groupi = &keyi->groups[i];
/* assign kt_index[i] to the index of the type in map->types.
* kt_index[i] may have been set by a previous run (if we have two
* layouts specified). Let's not overwrite it with the ONE_LEVEL
* default group if we dont even have keys for this group anyway.
*
* FIXME: There should be a better fix for this.
*/
if (!darray_empty(groupi->levels))
key->kt_index[i] = types[i];
if (!darray_empty(groupi->syms)) {
/* fill key to "width" symbols*/
for (tmp = 0; tmp < width; tmp++) {

23
test/keyseq.c
View File

@ -137,7 +137,8 @@ main(void)
struct xkb_keymap *keymap;
assert(ctx);
keymap = test_compile_rules(ctx, "evdev", "evdev", "us,il", NULL,
keymap = test_compile_rules(ctx, "evdev", "evdev",
"us,il,ru", ",,phonetic",
"grp:alt_shift_toggle,grp:menu_toggle");
assert(keymap);
@ -200,6 +201,7 @@ main(void)
KEY_K, BOTH, XKB_KEY_hebrew_lamed, NEXT,
KEY_F, BOTH, XKB_KEY_hebrew_kaph, NEXT,
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Next_Group, NEXT,
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Next_Group, NEXT,
KEY_O, BOTH, XKB_KEY_o, FINISH));
assert(test_key_seq(keymap,
@ -272,6 +274,25 @@ main(void)
KEY_NUMLOCK, BOTH, XKB_KEY_Num_Lock, NEXT,
KEY_KP2, BOTH, XKB_KEY_KP_Down, FINISH));
/* Test that the aliases in the ru(phonetic) symbols map work. */
assert(test_key_seq(keymap,
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Next_Group, NEXT,
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Next_Group, NEXT,
KEY_1, BOTH, XKB_KEY_1, NEXT,
KEY_Q, BOTH, XKB_KEY_Cyrillic_ya, NEXT,
KEY_LEFTSHIFT, DOWN, XKB_KEY_Shift_L, NEXT,
KEY_1, BOTH, XKB_KEY_exclam, NEXT,
KEY_Q, BOTH, XKB_KEY_Cyrillic_YA, NEXT,
KEY_LEFTSHIFT, UP, XKB_KEY_Shift_L, NEXT,
KEY_V, BOTH, XKB_KEY_Cyrillic_zhe, NEXT,
KEY_CAPSLOCK, BOTH, XKB_KEY_Caps_Lock, NEXT,
KEY_1, BOTH, XKB_KEY_1, NEXT,
KEY_V, BOTH, XKB_KEY_Cyrillic_ZHE, NEXT,
KEY_RIGHTSHIFT, DOWN, XKB_KEY_Shift_R, NEXT,
KEY_V, BOTH, XKB_KEY_Cyrillic_zhe, NEXT,
KEY_RIGHTSHIFT, UP, XKB_KEY_Shift_R, NEXT,
KEY_V, BOTH, XKB_KEY_Cyrillic_ZHE, FINISH));
xkb_map_unref(keymap);
xkb_context_unref(ctx);
return 0;