xkb_keysym_from_name() uses a big lookup table generated by "makekeys" to find keysyms. It does this case-sensitive because we have keys like XKB_KEY_A and XKB_KEY_a. So if a user searches for "a" we must always return the case-sensitive match, which is XKB_KEY_a. However, lets look at XKB_KEY_XF86HomePage. The user currently _must_ enter "XF86HomePage" correctly to get this keysym. If they enter "Xf86HomePage" or "XF86Homepage" or anything else, xkb_keysym_from_name() will fail. This is unreasonable and bogus. Instead we should allow an application to have a case-insensitive search as fallback. This removes policy from xkbcommon and moves it into the application. They can now decide, whether they use a possibly unsafe fallback or stay with the case-sensitive search.
Therefore, this patch modifies "makekeys" to convert all symbols to lower-case and add them into the lookup table, too. The case-sensitive symbols are unchanged and stay in the table, but a user-application can now fall back to lower-case search if a symbol cannot be resolved. This patch does take care that any case-sensitive key overwrites any lower-case key. That is, if "makekeys" adds XKB_KEY_A, it will add it as "A" and "a". If XKB_KEY_a is added later, it will overwrite the previous "a" because this is a case-sensitive match. This guarantees that looking up for "a" always returns KEY_a. This patch also makes sure that case-sensitive keys are inserted into the lookup table first. This guarantees that xkb_keysym_get_name() always returns the first match which is the case-sensitive entry. There are several theoretical pitfalls here: - Assume there is a key Abcd and abcd. If the user searches for ABCD and does not find anything, the fallback will return "abcd", although The user might have preferred "Abcd". An application can circumwent it by simply not using the fallback lower-case search, so this does not affect existing applications. - Assume there is a key AbCd but no other key "abcd" in any case. If the user searches for "abcd", it will return "AbCd" even though they didn't request a case-insensitive match. This _does_ affect existing applications. However, there is currently no key where a user wouldn't be happy about this, because every key where case really changes semantics, is always available in both, upper-case and lower-case and so this doesn't happen. An application can catch this by converting the result back to a name and checking whether the names match. However, I really don't know why anyone would bother? I haven't found any other cases where this changes semantics. There are some pitfalls when doing the case-insensitive fallback match, however, this is optional! The only case were existing application's behavior changes are listed above. However, considering that nearly everyone wants lower-case keys, anyway, this is in my eyes the best solution. Furthermore, new keysyms should be avoided and users should use Unicode symbols to identify keys. This is always unambigious and isn't affected at all by this patch. Signed-off-by: David Herrmann <[email protected]> --- makekeys/makekeys.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 67 insertions(+), 4 deletions(-) diff --git a/makekeys/makekeys.c b/makekeys/makekeys.c index 62d7255..b48e889 100644 --- a/makekeys/makekeys.c +++ b/makekeys/makekeys.c @@ -33,6 +33,7 @@ #include "xkbcommon/xkbcommon.h" +#include <ctype.h> #include <inttypes.h> #include <stdio.h> #include <stdlib.h> @@ -40,7 +41,7 @@ typedef uint32_t Signature; -#define KTNUM 4000 +#define KTNUM (4000 * 2) static struct info { char *name; @@ -115,19 +116,81 @@ main(int argc, char *argv[]) continue; } + /* Every key is added to the list case-sensitive. That is, all + * character case is preserved. But every key is also added with + * every character converted to lower-case. + * This allows an application to search for a key case-sensitive and + * always get a the correct match. But if nothing is matched, an + * application can search for all lower-case as fallback. This might + * yield another result as expected, so it should only be used as + * fallback. + * It has the side-effect that KEY_A is added as "A" and "a" and + * if we parse KEY_a afterwards, there is already an "a". Therefore, + * a case-sensitive insert _always_ overwrites previous inserts and + * a lower-case entry is only added if the _exact_ match does not + * exist, yet. This guarantees that "a" will always point to KEY_a + * and never to KEY_A. But if there are keys KEY_Abcd and KEY_abcd, + * then KEY_ABCD converted to lower-case will point to KEY_abcd, + * even though the user might have looked for Key_Abcd. + * There is one other side-effect if we have KEY_SomeThing and + * KEY_Something. Both do not collide with their true case, but they + * collide if both are converted to lower-case. In this case, only + * one of both is added in lower-case and it's the one that is + * first parsed. However, such keysyms do not exist, yet, and the + * chance that they are added is low. + * In fact, all collisions that currently exist are unambigious so + * we shouldn't bother too much. */ + + /* add key case-sensitive */ name = malloc(strlen(prefix) + strlen(key) + 1); if (!name) { fprintf(stderr, "makekeys: out of memory!\n"); exit(1); } sprintf(name, "%s%s", prefix, key); - info[ksnum].name = name; - info[ksnum].val = val; - ksnum++; + + for (i = 0; i < ksnum; ++i) { + if (!strcmp(info[i].name, name)) + break; + } + + info[i].name = name; + info[i].val = val; + if (i == ksnum) + ksnum++; + if (ksnum == KTNUM) { fprintf(stderr, "makekeys: too many keysyms!\n"); exit(1); } + + /* add key as lower-case */ + name = malloc(strlen(prefix) + strlen(key) + 1); + if (!name) { + fprintf(stderr, "makekeys: out of memory!\n"); + exit(1); + } + + sprintf(name, "%s%s", prefix, key); + for (i = 0; name[i]; ++i) + name[i] = tolower(name[i]); + + for (i = 0; i < ksnum; ++i) { + if (!strcmp(info[i].name, name)) + break; + } + + if (i == ksnum) { + info[ksnum].name = name; + info[ksnum].val = val; + ksnum++; + if (ksnum == KTNUM) { + fprintf(stderr, "makekeys: too many keysyms!\n"); + exit(1); + } + } else { + free(name); + } } fclose(fptr); -- 1.7.12.2 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
