> Go through all the GTK+ widgets > > - Look for handlers for event signals. > - Determine if they are correctly returning TRUE when > handling events. > - Make up a patch fixing cases which are definitely wrong. > - Make a list of dubious cases that can be checked over > by people who know the widgets in detail. wolfgang contre com and i have been working on this. I have tried my best to do this. I guess that i wouldn't go so far as to say there were any cases that are "definitely wrong" - definite is a pretty strong word. I made a list of all the handlers for the signals that Owen told me to concentrate on, > GtkWidget::button-press-event > GtkWidget::button-release-event > GtkWidget::key-press-event > GtkWidget::key-release-event > GtkWidget::drag-motion > GtkWidget::drag-drop > GtkWidget::delete-event > GtkWidget::selection-request-event > GtkWidget::selection-notify-event > GtkWidget::client-event > GtkWidget::event and went thru each occurence (both default and connected) and did my best. issues: well, the first is the status of anoncvs - basically checking out tends to result in an unbuildable mess. because of this i am working against a two week old copy of gtk+. i copied everything to a new dir and did a cvs update without conflicts on the files that i changed, so i think things should be ok. gtktreeview.c gtktreeviewcolumn.c these both seem under heavy development, so i think that the people working on them ought to make the changes (it looks like someone addressed this in recent changes). i don't understand the widgets so i just left them alone. gtktextview.c same story. gtkctree.c i tried my best on this one, but i have a feeling i might have misunderstood something. someone needs to look at it. button_release_event if the press resulted in TRUE, than it seems that the release ought to give TRUE regaurdless of wether it actually did anything (but then maybe that's just the farmer in me talking). i have tried to follow this as much as possible, but someone ought to look at a few cases carefully and see if i have the right general idea. other than that, things went pretty well. if there are suggestions/etc i will try to work on this some more, but i am pretty busy for the next week atleast. i will attach the list of handlers and a .tgz of the diffs (one per .c file). the patches are made to be applied from the gtk+ toplevel. the changelog is treated as a seperate file - the effort and likelihood of error for me to have integrated the changes into the individual .diff's far exceded my patients and skill (i would have been hand-editting .diff's) - whoever applies these patches just needs to delete the entries for .diff's they choose not to apply. seth
gtkbutton.c: widget_class->button_press_event = gtk_button_button_press; ok always returns true button 1 does a button press, so true ok other buttons do a gtk_widget_grab_focus, so should we return true iff we got focus? gtkbutton.c: widget_class->button_release_event = gtk_button_button_release; we don't use other than button 1, but parallel with button_press gtkcalendar.c: widget_class->button_press_event = gtk_calendar_button_press; changed - return TRUE if handled gtkcalendar.c: widget_class->key_press_event = gtk_calendar_key_press; changed - GDK_space now returns TRUE when it triggers select_day. maybe it should always return TRUE? gtkcellrenderertoggle.c: cell_class->event = gtk_cell_renderer_toggle_event; GDK_BUTTON_PRESS triggers "toggled" and returns TRUE. "toggled" is not yet used/connected anywhere, and is returns void so i assume that it can't fail. else return FALSE looks ok to me gtkclist.c: widget_class->button_press_event = gtk_clist_button_press; changed - return TRUE if handled by either clist_window or column_window uncertain - currently return FALSE if (gdk_pointer_grab), i don't understand the intention of this check looks ok to me gtkclist.c: widget_class->button_release_event = gtk_clist_button_release; return TRUE if handled not a return issue, but if we gdk_pointer_grab in button_press, don't we need gdk_pointer_ungrab here? looks ok to me gtkclist.c: widget_class->drag_drop = gtk_clist_drag_drop; ok gtkclist.c: widget_class->drag_motion = gtk_clist_drag_motion; ok gtkclist.c: gtk_signal_connect (GTK_OBJECT (clist->column[column].button), "event", gtkclist.c- (GtkSignalFunc) column_title_passive_func, NULL); looks ok - it returns TRUE on motion/button/enter/leave, but doesn't have any action. but then maybe thats why its called passive_func gtkcolorsel.c: gtk_signal_connect (GTK_OBJECT (retval), "button_press_event", palette_press, colorsel); changed - changed type from void to boolean and return TRUE ok gtkcolorsel.c: gtk_signal_connect (GTK_OBJECT (button), "button_press_event", mouse_press, colorsel); changed - changed type from void to boolean and return TRUE ok gtkcolorsel.c: gtk_signal_connect (GTK_OBJECT (button), "button_release_event", mouse_release, data); changed - changed type from void to boolean and return TRUE ok gtkcolorsel.c: gtk_signal_connect (GTK_OBJECT (retval), "key_press_event", GTK_SIGNAL_FUNC (palette_activate), colorsel); changed - changed type from void to boolean and return TRUE ok gtkcombo.c: gtk_signal_connect_after (GTK_OBJECT (combo->button), "button_press_event", gtkcombo.c- (GtkSignalFunc) gtk_combo_popup_button_press, combo); changed - fixed type from void to gboolean and return TRUE gtkcombo.c: gtk_signal_connect (GTK_OBJECT (combo->popwin), "button_press_event", gtkcombo.c- GTK_SIGNAL_FUNC (gtk_combo_button_press), combo); ok gtkcombo.c: gtk_signal_connect_after (GTK_OBJECT (combo->list), "button_release_event", gtkcombo.c- GTK_SIGNAL_FUNC (gtk_combo_button_release), combo); changed - we do an /* un-pre-highlight */ where i think we check to see if any of our children highlight. if we find one, we used to return FALSE. i changed this to return TRUE i am just guessing on this one somebody check this gtkcombo.c: gtk_signal_connect (GTK_OBJECT (combo->entry), "key_press_event", gtkcombo.c- (GtkSignalFunc) gtk_combo_entry_key_press, combo); gtkcombo.c: gtk_signal_connect (GTK_OBJECT (combo->popwin), "key_press_event", gtkcombo.c- (GtkSignalFunc) gtk_combo_list_key_press, combo); ok - these two connects use the same function gtkctree.c: widget_class->button_press_event = gtk_ctree_button_press; changed - return TRUE if triggers a collapse/expand gtkctree.c: widget_class->drag_motion = gtk_ctree_drag_motion; help - i don't know what this is doing. it looks to me that it returns TRUE when nothing happens, and FALSE when something happens, which is backwards, but makes me think that i just don't know what's goin on, and as i have never used dnd in any of my code, its not unlikely. gtkcurve.c: gtk_signal_connect (GTK_OBJECT (curve), "event", gtkcurve.c- (GtkSignalFunc) gtk_curve_graph_events, curve); changed - return TRUE on button press/release, motion ok gtkdialog.c: widget_class->key_press_event = gtk_dialog_key_press; ok gtk_signal_connect (GTK_OBJECT (dialog), gtkdialog.c: "delete_event", gtkdialog.c- GTK_SIGNAL_FUNC (gtk_dialog_delete_event_handler), ok gtk_signal_connect (GTK_OBJECT (dialog), gtkdialog.c: "delete_event", gtkdialog.c- GTK_SIGNAL_FUNC (run_delete_handler), ok - returns TRUE to inhibit actual deletion gtkdnd.c: gtk_signal_connect (GTK_OBJECT (widget), "button_press_event", gtkdnd.c- GTK_SIGNAL_FUNC (gtk_drag_source_event_cb), gtkdnd.c gtk_signal_connect (GTK_OBJECT (info->ipc_widget), "button_release_event", gtkdnd.c- GTK_SIGNAL_FUNC (gtk_drag_button_release_cb), info); changed - return TRUE if the active button is used for press/release/motion help - motion triggers gtk_drag_begin, so i return TRUE? gtkdnd.c: gtk_signal_connect (GTK_OBJECT (info->ipc_widget), "key_press_event", gtkdnd.c- GTK_SIGNAL_FUNC (gtk_drag_key_cb), info); gtkdnd.c gtk_signal_connect (GTK_OBJECT (info->ipc_widget), "key_release_event", gtkdnd.c- GTK_SIGNAL_FUNC (gtk_drag_key_cb), info); ok - we do a gtk_drag_update and return TRUE gtkentry.c: widget_class->button_press_event = gtk_entry_button_press; ok gtkentry.c: widget_class->button_release_event = gtk_entry_button_release; ok - not quite parallel with press gtkentry.c: widget_class->drag_motion = gtk_entry_drag_motion; ok gtkentry.c: widget_class->key_press_event = gtk_entry_key_press; ok gtkfilesel.c: gtk_signal_connect (GTK_OBJECT (filesel->selection_entry), "key_press_event", gtkfilesel.c- (GtkSignalFunc) gtk_file_selection_key_press, filesel); ok - emit_stop gtkfontsel.c: gtk_signal_connect (GTK_OBJECT (fontsel->size_entry), "key_press_event", gtkfontsel.c- (GtkSignalFunc) gtk_font_selection_size_key_press, gtkfontsel.c: gtk_signal_connect (GTK_OBJECT (fontsel->font_clist), "key_press_event", gtkfontsel.c- GTK_SIGNAL_FUNC(gtk_font_selection_on_clist_key_press), gtkfontsel.c: "key_press_event", gtkfontsel.c- GTK_SIGNAL_FUNC(gtk_font_selection_on_clist_key_press), gtkfontsel.c: gtk_signal_connect (GTK_OBJECT (fontsel->size_clist), "key_press_event", gtkfontsel.c- GTK_SIGNAL_FUNC(gtk_font_selection_on_clist_key_press), ok - duplicates gtkhandlebox.c: widget_class->button_press_event = gtk_handle_box_button_changed; gtkhandlebox.c: widget_class->button_release_event = gtk_handle_box_button_changed; ok i think ? button_press && child_detached triggers gtk_handle_box_reattach currently we return FALSE, and it looks intentional. gtkhandlebox.c: widget_class->delete_event = gtk_handle_box_delete_event; ok gtkhpaned.c: widget_class->button_press_event = gtk_hpaned_button_press; ok gtkhpaned.c: widget_class->button_release_event = gtk_hpaned_button_release; ok gtkhsv.c: widget_class->button_press_event = gtk_hsv_button_press; ok gtkhsv.c: widget_class->button_release_event = gtk_hsv_button_release; ok gtkinputdialog.c: gtk_signal_connect (GTK_OBJECT(key->entry), "key_press_event", gtkinputdialog.c- GTK_SIGNAL_FUNC (gtk_input_dialog_key_press), key); ok gtklist.c: widget_class->button_press_event = gtk_list_button_press; changed - i return true mostly if (item && !GTK_IS_LIST_ITEM (item)) is this too aggressive? gtklist.c: widget_class->button_release_event = gtk_list_button_release; changed - return TRUE if (list->drag_selection) parallelism is broken re press gtklistitem.c: widget_class->button_press_event = gtk_list_item_button_press; changed - ok gtkmenu.c: widget_class->key_press_event = gtk_menu_key_press; ok gtk_signal_connect (GTK_OBJECT (menu->tearoff_window), gtkmenu.c: "event", gtkmenu.c- GTK_SIGNAL_FUNC (gtk_menu_window_event), ok gtkmenushell.c: widget_class->button_press_event = gtk_menu_shell_button_press; gtkmenushell.c: widget_class->button_release_event = gtk_menu_shell_button_release; changed - ok gtkmenushell.c: widget_class->key_press_event = gtk_menu_shell_key_press; ok gtknotebook.c: widget_class->button_press_event = gtk_notebook_button_press; changed - ok gtknotebook.c: widget_class->button_release_event = gtk_notebook_button_release; ok - but doesn't seem quite parallel with button_press ? gtknotebook.c: widget_class->key_press_event = gtk_notebook_key_press; ok gtkoptionmenu.c: widget_class->button_press_event = gtk_option_menu_button_press; gtkoptionmenu.c: widget_class->key_press_event = gtk_option_menu_key_press; ok gtkplug.c: widget_class->key_press_event = gtk_plug_key_press_event; ok gtkrange.c: widget_class->button_press_event = gtk_range_button_press; gtkrange.c: widget_class->button_release_event = gtk_range_button_release; changed - ok gtkrange.c: widget_class->key_press_event = gtk_range_key_press; ok gtkspinbutton.c: widget_class->button_press_event = gtk_spin_button_button_press; gtkspinbutton.c: widget_class->button_release_event = gtk_spin_button_button_release; changed - ok gtkspinbutton.c: widget_class->key_press_event = gtk_spin_button_key_press; gtkspinbutton.c: widget_class->key_release_event = gtk_spin_button_key_release; ok gtk_signal_connect_object (GTK_OBJECT (menu->tearoff_window), gtktearoffmenuitem.c: "delete_event", gtktearoffmenuitem.c- GTK_SIGNAL_FUNC (gtk_tearoff_menu_item_delete_cb), ok gtktext.c: widget_class->button_press_event = gtk_text_button_press; gtktext.c: widget_class->button_release_event = gtk_text_button_release; gtktext.c: widget_class->key_press_event = gtk_text_key_press; changed - ok gtktextview.c: widget_class->button_press_event = gtk_text_view_button_press_event; gtktextview.c: widget_class->button_release_event = gtk_text_view_button_release_event; gtktextview.c: widget_class->drag_drop = gtk_text_view_drag_drop; gtktextview.c: widget_class->drag_motion = gtk_text_view_drag_motion; gtktextview.c: widget_class->event = gtk_text_view_event; gtktextview.c: widget_class->key_press_event = gtk_text_view_key_press_event; gtktextview.c: widget_class->key_release_event = gtk_text_view_key_release_event; unchanged - someone (hp/owen) needs to check this the cvs has changed drastically since i checked out gtktipsquery.c: widget_class->event = gtk_tips_query_event; ok gtktooltips.c: gtk_signal_connect_after(GTK_OBJECT (widget), "event", gtktooltips.c- (GtkSignalFunc) gtk_tooltips_event_handler, changed - ok gtktree.c: widget_class->button_press_event = gtk_tree_button_press; gtktree.c: widget_class->button_release_event = gtk_tree_button_release; ok gtktreeitem.c: widget_class->button_press_event = gtk_tree_item_button_press; changed - ok i think - true if a button_press and sensitive gtktreeitem.c: gtk_signal_connect(GTK_OBJECT(eventbox), "button_press_event", gtktreeitem.c- (GtkSignalFunc)gtk_tree_item_subtree_button_click, changed - ok - changed type to gint from void and TRUE if sensitive gtktreeview.c: widget_class->button_press_event = gtk_tree_view_button_press; gtktreeview.c: widget_class->button_release_event = gtk_tree_view_button_release; gtktreeview.c: widget_class->drag_drop = gtk_tree_view_drag_drop; gtktreeview.c: widget_class->drag_motion = gtk_tree_view_drag_motion; someone check this - it looks ok to me, but it is new and i don't quite get what is going on gtk_signal_connect (GTK_OBJECT (tree_column->button), gtktreeviewcolumn.c: "event", gtktreeviewcolumn.c- (GtkSignalFunc) gtk_tree_view_passive_func, someone check this - it looks ok to me, but it is new and i don't quite get what is going on gtkvpaned.c: widget_class->button_press_event = gtk_vpaned_button_press; gtkvpaned.c: widget_class->button_release_event = gtk_vpaned_button_release; ok gtkwidget.c: klass->key_press_event = gtk_widget_real_key_press_event; gtkwidget.c: klass->key_release_event = gtk_widget_real_key_release_event; ok gtkwidget.c: klass->selection_notify_event = gtk_selection_notify; gtkwidget.c: klass->selection_request_event = gtk_selection_request; ok - these are in gtkselection.c gtkwindow.c: widget_class->client_event = gtk_window_client_event; ok - this looks like it is for non-action events gtkwindow.c: widget_class->key_press_event = gtk_window_key_press_event; gtkwindow.c: widget_class->key_release_event = gtk_window_key_release_event; ok gtk_signal_connect (GTK_OBJECT (window), gtkwindow.c: "event", gtkwindow.c- GTK_SIGNAL_FUNC (gtk_window_event), ok - someone check this - we return FALSE even though we g_object_ref i think it is ok but ...
Attachment:
diff.byfile.tgz
Description: retval_changes.byfile.tgz