GNOME Bugzilla – Bug 376750
bug in gedit_window_key_press_event
Last modified: 2006-12-03 22:42:11 UTC
As described in the comment for gedit_window_key_press_event(), current implementation causes problem if focused_widget is GeditView. For example, if focus is in a text widget and a Shift key is pressed, it will cause same key_press event in GtkTextView twice with following sequence. 1. gtk_widget_event (focused_widget, (GdkEvent*) event) in gedit_window_key_press_event() is called and return FALSE In the event, gtk_source_view_key_press_event() is called and return FALSE (with gtk_text_view_key_press_event() return FALSE) 2. Since 1. returns FALSE, GTK_WIDGET_CLASS (gedit_window_parent_class)->key_press_event (widget, event) call gtk_source_view_key_press_event(). If XIM is uses for input method for gtktextview, this causes infinite loop since XFilterEvent is called twice with the same event. So GTK_WIDGET_CLASS (gedit_window_parent_class)->key_press_event (widget, event) should not be called if the focused widget is GeditView. Please apply attached patch.
Created attachment 76813 [details] [review] Patch to fix key press event in geditview
Patch looks good.
Sorry, my patch broke keyboard shortcut for the menu (like Control-F). I'll reconsider how to solve...
And I'm not sure why gedit_window_key_press_event() calls gtk_widget_event() for the focused_widget. Just return GTK_WIDGET_CLASS (gedit_window_parent_class)->key_press_event (widget, event) seems fine for me. What does it mean "an entry on a panel" in its comment? ... This is unfortunate and means that pressing ctrl+V in an entry on a panel ends up pasting text in the TextView. ...
See also bug #331369, it looks related to me
"an entry on a panel" is a GtkEntry put in a side or bottom panel.
Created attachment 76905 [details] [review] revised patch > "an entry on a panel" is a GtkEntry put in a side or bottom panel. OK. But how can I create GtkEntry in side panel? I've checked "Statusbar" and "SidePane". Anyway, I've revised the patch. It only calls gtk_widget_event for the focused_widget when it is not a textview. Can you review it?
Aha, now understand using Python Console from "BottomPane". As you said, just call GTK_WIDGET_CLASS (gedit_window_parent_class)->key_press_event (widget, event) causes the problem with Control-V in the BottomPane. I've tested my revised patch and it solves "double keypress event with a shift key" problem in GeditView, but same problem still exists in the BottomPane. I think the "double keypress event" which is occurred when gtk_im_context_filter_keypress() returns FALSE cannot be solved with current implementation of gedit_window_key_press_event(). How about connecting "key_press_event" to the focused widget when its focus is changed to the text widget of bottom/side pane?
Created attachment 76929 [details] [review] Patch to use gtk_window_activate_key() 3rd try to fix the problem. It uses gtk_window_activate_key() instead of GTK_WIDGET_CLASS (gedit_window_parent_class)->key_press_event () so that avoid calling gtk_im_context_filter_keypress twice. I've tested this code with XIM, and text input in the main textview and the python console in the bottom pane works fine for me.
Created attachment 76932 [details] [review] better patch Added check for focused_widget if !handled.
Created attachment 77165 [details] [review] proposed patch What about this patch? Can you test if it works? If it does, I think that it's the proper way to fix it.
(In reply to comment #11) > Created an attachment (id=77165) [edit] > proposed patch > > What about this patch? Can you test if it works? If it does, I think that it's > the proper way to fix it. Does it compile? I got following error. gedit-window.c: In function 'gedit_window_key_press_event': gedit-window.c:282: error: 'GtkWindowClass' has no member named 'key_press_event'
No, you are right. My patch was totally bogus and untested, sorry, I got confused. However I don't think that your patch is totally right: GTK_WINDOW_CLASS (gedit_window_parent_class)->key_press_event (widget, event) makes sure that the parent handler is called, that is gtk_window_key_press_event(). Let's take a look at that function: static gint gtk_window_key_press_event (GtkWidget *widget, GdkEventKey *event) { GtkWindow *window = GTK_WINDOW (widget); gboolean handled = FALSE; /* handle mnemonics and accelerators */ if (!handled) handled = gtk_window_activate_key (window, event); /* handle focus widget key events */ if (!handled) handled = gtk_window_propagate_key_event (window, event); /* Chain up, invokes binding set */ if (!handled) handled = GTK_WIDGET_CLASS (gtk_window_parent_class)->key_press_event (widget, event); return handled; } So your patch correctly avoids to call gtk_window_propagate_key_event twice, but in the case that gtk_window_activate_key doesn't return true, it doesn't call into the "grand parent handler", so it ignores the binding set. I think we should change gedit_window_key_press_event to look like gtk_window_key_press_event, only with gtk_window_propagate_key_event done before gtk_window_activate_key, and then make sure to chain up to the grand parent handler, skipping GtkWindow's key-press-event handler
Created attachment 77167 [details] [review] patch can you test this one? Thanks in advance
(In reply to comment #14) I've tested the patch and works fine. Thanks! What is the grand_parent_class in this case? Just curious.
> What is the grand_parent_class in this case? Just curious. It's GtkWidgetClass
(In reply to comment #16) > > What is the grand_parent_class in this case? Just curious. > > It's GtkWidgetClass I see. So it calls gtk_bindings_activate_event(). Now I think the problem is resolved. Thanks again.
ops, forgot to commit :-) Now it's in cvs.
Have you committed to gnome-216 branch too?
> Have you committed to gnome-216 branch too? Now yes.