* Re: [Qemu-devel] [PATCH v2] gtk: Fix accelerator filtering [not found] ` <512B86E7.80603@siemens.com> @ 2013-03-24 18:06 ` Jan Kiszka 2013-03-25 12:51 ` Anthony Liguori 2013-05-07 21:03 ` Jordan Justen 0 siblings, 2 replies; 7+ messages in thread From: Jan Kiszka @ 2013-03-24 18:06 UTC (permalink / raw) Cc: Kevin Wolf, Anthony Liguori, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1308 bytes --] On 2013-02-25 16:44, Jan Kiszka wrote: > On 2013-02-25 16:39, Anthony Liguori wrote: >> Jan Kiszka <jan.kiszka@siemens.com> writes: >> >>> This is in fact very simply: When the input in grabbed, everything >>> should be exclusively passed to the guest - except it has our magic >>> CTRL-ALT modifier set. Then let GTK filter out those accels that are in >>> use. When checking the modifier state, we just need to filter out NUM >>> and CAPS lock. >> >> Can you explain what you're fixing? > > That it's not filtering what it is supposed to. > >> >> We shouldn't hard code modifiers like this. The reason you give >> accelerators paths like this is so that they can be overridden by a >> user. >> >> That's why I filtered by path. Once we're running, we shouldn't assume >> that accelerators use the modifiers we started with. > > Your path-based filtering does not work as it uses an unsupported > internal function (see my other mail). > > We can make the modifier configurable via QEMU means (command line > parameter, gconfig, whatever). But let's get the basics working first. The bug still exists, my patch still applies. Unless you have some idea for a better solution, please apply this for now so that CTRL-q inside a guest doesn't kill more kittens. Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 263 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] gtk: Fix accelerator filtering 2013-03-24 18:06 ` [Qemu-devel] [PATCH v2] gtk: Fix accelerator filtering Jan Kiszka @ 2013-03-25 12:51 ` Anthony Liguori 2013-05-07 21:03 ` Jordan Justen 1 sibling, 0 replies; 7+ messages in thread From: Anthony Liguori @ 2013-03-25 12:51 UTC (permalink / raw) To: Jan Kiszka; +Cc: Kevin Wolf, qemu-devel Jan Kiszka <jan.kiszka@web.de> writes: > On 2013-02-25 16:44, Jan Kiszka wrote: >> On 2013-02-25 16:39, Anthony Liguori wrote: >>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>> >>>> This is in fact very simply: When the input in grabbed, everything >>>> should be exclusively passed to the guest - except it has our magic >>>> CTRL-ALT modifier set. Then let GTK filter out those accels that are in >>>> use. When checking the modifier state, we just need to filter out NUM >>>> and CAPS lock. >>> >>> Can you explain what you're fixing? >> >> That it's not filtering what it is supposed to. >> >>> >>> We shouldn't hard code modifiers like this. The reason you give >>> accelerators paths like this is so that they can be overridden by a >>> user. >>> >>> That's why I filtered by path. Once we're running, we shouldn't assume >>> that accelerators use the modifiers we started with. >> >> Your path-based filtering does not work as it uses an unsupported >> internal function (see my other mail). >> >> We can make the modifier configurable via QEMU means (command line >> parameter, gconfig, whatever). But let's get the basics working first. > > The bug still exists, my patch still applies. Unless you have some idea > for a better solution, please apply this for now so that CTRL-q inside a > guest doesn't kill more kittens. Hi Jan, Your patch breaks overriding accelerators which as I said before is a critical accessibility feature. The current code works for me but I realize it's using an unsupported interface. I'll spend some time today trying to find a work around. But we definitely cannot assume that the accelerators are using any specific modifiers. Regards, Anthony Liguori > > Jan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] gtk: Fix accelerator filtering 2013-03-24 18:06 ` [Qemu-devel] [PATCH v2] gtk: Fix accelerator filtering Jan Kiszka 2013-03-25 12:51 ` Anthony Liguori @ 2013-05-07 21:03 ` Jordan Justen 2013-05-07 22:42 ` Jan Kiszka 1 sibling, 1 reply; 7+ messages in thread From: Jordan Justen @ 2013-05-07 21:03 UTC (permalink / raw) To: Jan Kiszka, Anthony Liguori; +Cc: Kevin Wolf, qemu-devel On Sun, Mar 24, 2013 at 11:06 AM, Jan Kiszka <jan.kiszka@web.de> wrote: > On 2013-02-25 16:44, Jan Kiszka wrote: >> On 2013-02-25 16:39, Anthony Liguori wrote: >>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>> >>>> This is in fact very simply: When the input in grabbed, everything >>>> should be exclusively passed to the guest - except it has our magic >>>> CTRL-ALT modifier set. Then let GTK filter out those accels that are in >>>> use. When checking the modifier state, we just need to filter out NUM >>>> and CAPS lock. >>> >>> Can you explain what you're fixing? >> >> That it's not filtering what it is supposed to. >> >>> >>> We shouldn't hard code modifiers like this. The reason you give >>> accelerators paths like this is so that they can be overridden by a >>> user. >>> >>> That's why I filtered by path. Once we're running, we shouldn't assume >>> that accelerators use the modifiers we started with. >> >> Your path-based filtering does not work as it uses an unsupported >> internal function (see my other mail). >> >> We can make the modifier configurable via QEMU means (command line >> parameter, gconfig, whatever). But let's get the basics working first. > > The bug still exists, my patch still applies. Unless you have some idea > for a better solution, please apply this for now so that CTRL-q inside a > guest doesn't kill more kittens. I finally built qemu with gtk support, and in general it seems like a great improvement over SDL. ...except ctrl-q to quit the VM. Why is binding a hotkey to quit a good idea at all? It seems kind of like attaching your computer's power to a wall-switch. :) -Jordan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] gtk: Fix accelerator filtering 2013-05-07 21:03 ` Jordan Justen @ 2013-05-07 22:42 ` Jan Kiszka 2013-07-17 6:54 ` Jan Kiszka 0 siblings, 1 reply; 7+ messages in thread From: Jan Kiszka @ 2013-05-07 22:42 UTC (permalink / raw) To: Jordan Justen, Anthony Liguori; +Cc: Kevin Wolf, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1952 bytes --] On 2013-05-07 23:03, Jordan Justen wrote: > On Sun, Mar 24, 2013 at 11:06 AM, Jan Kiszka <jan.kiszka@web.de> wrote: >> On 2013-02-25 16:44, Jan Kiszka wrote: >>> On 2013-02-25 16:39, Anthony Liguori wrote: >>>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>>> >>>>> This is in fact very simply: When the input in grabbed, everything >>>>> should be exclusively passed to the guest - except it has our magic >>>>> CTRL-ALT modifier set. Then let GTK filter out those accels that are in >>>>> use. When checking the modifier state, we just need to filter out NUM >>>>> and CAPS lock. >>>> >>>> Can you explain what you're fixing? >>> >>> That it's not filtering what it is supposed to. >>> >>>> >>>> We shouldn't hard code modifiers like this. The reason you give >>>> accelerators paths like this is so that they can be overridden by a >>>> user. >>>> >>>> That's why I filtered by path. Once we're running, we shouldn't assume >>>> that accelerators use the modifiers we started with. >>> >>> Your path-based filtering does not work as it uses an unsupported >>> internal function (see my other mail). >>> >>> We can make the modifier configurable via QEMU means (command line >>> parameter, gconfig, whatever). But let's get the basics working first. >> >> The bug still exists, my patch still applies. Unless you have some idea >> for a better solution, please apply this for now so that CTRL-q inside a >> guest doesn't kill more kittens. > > I finally built qemu with gtk support, and in general it seems like a > great improvement over SDL. > > ...except ctrl-q to quit the VM. Why is binding a hotkey to quit a > good idea at all? It seems kind of like attaching your computer's > power to a wall-switch. :) Yeah, this bug should really be fixed in some way before 1.5 is released because GTK will be default. Anthony, what is the status of your experiments with alternative solutions? Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 263 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] gtk: Fix accelerator filtering 2013-05-07 22:42 ` Jan Kiszka @ 2013-07-17 6:54 ` Jan Kiszka 2013-07-17 13:02 ` Anthony Liguori 0 siblings, 1 reply; 7+ messages in thread From: Jan Kiszka @ 2013-07-17 6:54 UTC (permalink / raw) To: Anthony Liguori; +Cc: Kevin Wolf, Jordan Justen, qemu-devel [-- Attachment #1: Type: text/plain, Size: 2244 bytes --] On 2013-05-08 00:42, Jan Kiszka wrote: > On 2013-05-07 23:03, Jordan Justen wrote: >> On Sun, Mar 24, 2013 at 11:06 AM, Jan Kiszka <jan.kiszka@web.de> wrote: >>> On 2013-02-25 16:44, Jan Kiszka wrote: >>>> On 2013-02-25 16:39, Anthony Liguori wrote: >>>>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>>>> >>>>>> This is in fact very simply: When the input in grabbed, everything >>>>>> should be exclusively passed to the guest - except it has our magic >>>>>> CTRL-ALT modifier set. Then let GTK filter out those accels that are in >>>>>> use. When checking the modifier state, we just need to filter out NUM >>>>>> and CAPS lock. >>>>> >>>>> Can you explain what you're fixing? >>>> >>>> That it's not filtering what it is supposed to. >>>> >>>>> >>>>> We shouldn't hard code modifiers like this. The reason you give >>>>> accelerators paths like this is so that they can be overridden by a >>>>> user. >>>>> >>>>> That's why I filtered by path. Once we're running, we shouldn't assume >>>>> that accelerators use the modifiers we started with. >>>> >>>> Your path-based filtering does not work as it uses an unsupported >>>> internal function (see my other mail). >>>> >>>> We can make the modifier configurable via QEMU means (command line >>>> parameter, gconfig, whatever). But let's get the basics working first. >>> >>> The bug still exists, my patch still applies. Unless you have some idea >>> for a better solution, please apply this for now so that CTRL-q inside a >>> guest doesn't kill more kittens. >> >> I finally built qemu with gtk support, and in general it seems like a >> great improvement over SDL. >> >> ...except ctrl-q to quit the VM. Why is binding a hotkey to quit a >> good idea at all? It seems kind of like attaching your computer's >> power to a wall-switch. :) > > Yeah, this bug should really be fixed in some way before 1.5 is released > because GTK will be default. Anthony, what is the status of your > experiments with alternative solutions? Re-ping on this, now for 1.6 (with the option to backport the fix). What defines the modifiers to be used for the accelerators? Is there an interface to query them (to avoid my original hard-coding)? Jan [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 263 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] gtk: Fix accelerator filtering 2013-07-17 6:54 ` Jan Kiszka @ 2013-07-17 13:02 ` Anthony Liguori 2013-07-22 7:04 ` [Qemu-devel] [PATCH v3] " Jan Kiszka 0 siblings, 1 reply; 7+ messages in thread From: Anthony Liguori @ 2013-07-17 13:02 UTC (permalink / raw) To: Jan Kiszka; +Cc: Kevin Wolf, Jordan Justen, qemu-devel Jan Kiszka <jan.kiszka@web.de> writes: > On 2013-05-08 00:42, Jan Kiszka wrote: >> On 2013-05-07 23:03, Jordan Justen wrote: >> Yeah, this bug should really be fixed in some way before 1.5 is released >> because GTK will be default. Anthony, what is the status of your >> experiments with alternative solutions? > > Re-ping on this, now for 1.6 (with the option to backport the fix). > > What defines the modifiers to be used for the accelerators? Is there an > interface to query them (to avoid my original hard-coding)? I hate to let the bug fester for another release although this fix breaks accelerator remapping. I guess that's better than nothing so please rebase. I'll try to fix it properly on top of your patch. Regards, Anthony Liguori > > Jan ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v3] gtk: Fix accelerator filtering 2013-07-17 13:02 ` Anthony Liguori @ 2013-07-22 7:04 ` Jan Kiszka 0 siblings, 0 replies; 7+ messages in thread From: Jan Kiszka @ 2013-07-22 7:04 UTC (permalink / raw) To: Anthony Liguori; +Cc: Kevin Wolf, Jordan Justen, qemu-devel This is in fact very simply: When the input in grabbed, everything should be exclusively passed to the guest - except it has our magic CTRL-ALT modifier set. Then let GTK filter out those accels that are in use. When checking the modifier state, we just need to filter out NUM and CAPS lock. Note: Filtering based on hard-coded modifiers breaks overriding accelerators. Needs to be fixed at a later point. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- v3: Rebased over master ui/gtk.c | 39 ++++++++++++++++++--------------------- 1 files changed, 18 insertions(+), 21 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index caea10d..5ce8ac9 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -98,6 +98,10 @@ static inline void gdk_drawable_get_size(GdkWindow *w, gint *ww, gint *wh) #define GDK_KEY_minus GDK_minus #endif +#define HOTKEY_MODIFIERS (GDK_CONTROL_MASK | GDK_MOD1_MASK) +#define IGNORE_MODIFIER_MASK \ + (GDK_MODIFIER_MASK & ~(GDK_LOCK_MASK | GDK_MOD2_MASK)) + static const int modifier_keycode[] = { /* shift, control, alt keys, meta keys, both left & right */ 0x2a, 0x36, 0x1d, 0x9d, 0x38, 0xb8, 0xdb, 0xdd, @@ -473,22 +477,10 @@ static void gd_mouse_mode_change(Notifier *notify, void *data) static gboolean gd_window_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque) { GtkDisplayState *s = opaque; - GtkAccelGroupEntry *entries; - guint n_entries = 0; - gboolean propagate_accel = TRUE; gboolean handled = FALSE; - entries = gtk_accel_group_query(s->accel_group, key->keyval, - key->state, &n_entries); - if (n_entries) { - const char *quark = g_quark_to_string(entries[0].accel_path_quark); - - if (gd_is_grab_active(s) && strstart(quark, "<QEMU>/File/", NULL)) { - propagate_accel = FALSE; - } - } - - if (!handled && propagate_accel) { + if (!gd_is_grab_active(s) || + (key->state & IGNORE_MODIFIER_MASK) == HOTKEY_MODIFIERS) { handled = gtk_window_activate_key(GTK_WINDOW(widget), key); } if (handled) { @@ -1197,7 +1189,7 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL vc->menu_item = gtk_radio_menu_item_new_with_mnemonic(group, label); group = gtk_radio_menu_item_get_group(GTK_RADIO_MENU_ITEM(vc->menu_item)); gtk_menu_item_set_accel_path(GTK_MENU_ITEM(vc->menu_item), path); - gtk_accel_map_add_entry(path, GDK_KEY_2 + index, GDK_CONTROL_MASK | GDK_MOD1_MASK); + gtk_accel_map_add_entry(path, GDK_KEY_2 + index, HOTKEY_MODIFIERS); vc->terminal = vte_terminal_new(); @@ -1354,7 +1346,8 @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, GtkAccelGroup *accel_g gtk_image_menu_item_new_from_stock(GTK_STOCK_FULLSCREEN, NULL); gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->full_screen_item), "<QEMU>/View/Full Screen"); - gtk_accel_map_add_entry("<QEMU>/View/Full Screen", GDK_KEY_f, GDK_CONTROL_MASK | GDK_MOD1_MASK); + gtk_accel_map_add_entry("<QEMU>/View/Full Screen", GDK_KEY_f, + HOTKEY_MODIFIERS); gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->full_screen_item); separator = gtk_separator_menu_item_new(); @@ -1363,19 +1356,22 @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, GtkAccelGroup *accel_g s->zoom_in_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_IN, NULL); gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_in_item), "<QEMU>/View/Zoom In"); - gtk_accel_map_add_entry("<QEMU>/View/Zoom In", GDK_KEY_plus, GDK_CONTROL_MASK | GDK_MOD1_MASK); + gtk_accel_map_add_entry("<QEMU>/View/Zoom In", GDK_KEY_plus, + HOTKEY_MODIFIERS); gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->zoom_in_item); s->zoom_out_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_OUT, NULL); gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_out_item), "<QEMU>/View/Zoom Out"); - gtk_accel_map_add_entry("<QEMU>/View/Zoom Out", GDK_KEY_minus, GDK_CONTROL_MASK | GDK_MOD1_MASK); + gtk_accel_map_add_entry("<QEMU>/View/Zoom Out", GDK_KEY_minus, + HOTKEY_MODIFIERS); gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->zoom_out_item); s->zoom_fixed_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_100, NULL); gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_fixed_item), "<QEMU>/View/Zoom Fixed"); - gtk_accel_map_add_entry("<QEMU>/View/Zoom Fixed", GDK_KEY_0, GDK_CONTROL_MASK | GDK_MOD1_MASK); + gtk_accel_map_add_entry("<QEMU>/View/Zoom Fixed", GDK_KEY_0, + HOTKEY_MODIFIERS); gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->zoom_fixed_item); s->zoom_fit_item = gtk_check_menu_item_new_with_mnemonic(_("Zoom To _Fit")); @@ -1390,7 +1386,8 @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, GtkAccelGroup *accel_g s->grab_item = gtk_check_menu_item_new_with_mnemonic(_("_Grab Input")); gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->grab_item), "<QEMU>/View/Grab Input"); - gtk_accel_map_add_entry("<QEMU>/View/Grab Input", GDK_KEY_g, GDK_CONTROL_MASK | GDK_MOD1_MASK); + gtk_accel_map_add_entry("<QEMU>/View/Grab Input", GDK_KEY_g, + HOTKEY_MODIFIERS); gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->grab_item); separator = gtk_separator_menu_item_new(); @@ -1400,7 +1397,7 @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, GtkAccelGroup *accel_g group = gtk_radio_menu_item_get_group(GTK_RADIO_MENU_ITEM(s->vga_item)); gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->vga_item), "<QEMU>/View/VGA"); - gtk_accel_map_add_entry("<QEMU>/View/VGA", GDK_KEY_1, GDK_CONTROL_MASK | GDK_MOD1_MASK); + gtk_accel_map_add_entry("<QEMU>/View/VGA", GDK_KEY_1, HOTKEY_MODIFIERS); gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->vga_item); for (i = 0; i < nb_vcs; i++) { -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-07-22 7:04 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1361367806-4599-1-git-send-email-aliguori@us.ibm.com> [not found] ` <1361367806-4599-10-git-send-email-aliguori@us.ibm.com> [not found] ` <5127B788.40701@siemens.com> [not found] ` <5127BBC1.5070607@siemens.com> [not found] ` <512B7B8B.1000402@siemens.com> [not found] ` <87zjys9zcv.fsf@codemonkey.ws> [not found] ` <512B86E7.80603@siemens.com> 2013-03-24 18:06 ` [Qemu-devel] [PATCH v2] gtk: Fix accelerator filtering Jan Kiszka 2013-03-25 12:51 ` Anthony Liguori 2013-05-07 21:03 ` Jordan Justen 2013-05-07 22:42 ` Jan Kiszka 2013-07-17 6:54 ` Jan Kiszka 2013-07-17 13:02 ` Anthony Liguori 2013-07-22 7:04 ` [Qemu-devel] [PATCH v3] " Jan Kiszka
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.