#52434 - Lock accelerators by default

[Owen Taylor <otaylor redhat com>]

I took a quick look at this bug - the idea being that accelerators
should only be changeable for menu items if the entity creating
the menu item wants them to be changeable.

Changing gtkwidget.c so accelerators are locked by default
is easy enough - patch appended.

However this isn't quite right because we don't really want
prevent the accelerators on the widget being changed programatically,
just from being changed by the user.

I think the right thing to do might be to change things so:

 a) gtk_widget_add_accelerator/remove_accelerator() ignore the
    locked flag and always work.

 b) gtkmenu.c explicitely checks the locked flag (already does so) 
    before changing an accelerator.

 c) gtkmenu.c checks to see if there are any locked accelerators with
    the same key before setting the accelerator on a widget to avoid
    removing accelerators that you can't add back.  (Requires a small
    GtkAccelGroup API addition. Ugly, yes, but internally ugly)

c) is not all that important, IMO, since I think having a mix of
changeable and non-changeable accelerators on the same toplevel
is pretty broken. But its a corner case we probably should handle.

Anyways, I think you might have said you had some interest in working
on this, so if that's the case, I'll let you decide what we should
do. Just thought I'd throw out my thoughts since I spent a bit of time
looking on it.

Regards,
                                        Owen

Index: gtk/gtkitemfactory.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkitemfactory.c,v
retrieving revision 1.37
diff -u -r1.37 gtkitemfactory.c
--- gtk/gtkitemfactory.c	2001/03/24 00:07:16	1.37
+++ gtk/gtkitemfactory.c	2001/03/29 23:57:39
@@ -1221,6 +1221,7 @@
 						    type_id != quark_type_title),
 			   "GtkWidget::parent", parent,
 			   NULL);
+  gtk_widget_unlock_accelerators (widget);
   if (option_menu && !option_menu->menu_item)
     gtk_option_menu_set_history (option_menu, 0);
 
Index: gtk/gtkprivate.h
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkprivate.h,v
retrieving revision 1.22
diff -u -r1.22 gtkprivate.h
--- gtk/gtkprivate.h	2001/03/08 06:14:42	1.22
+++ gtk/gtkprivate.h	2001/03/29 23:57:39
@@ -49,7 +49,8 @@
   PRIVATE_GTK_IN_REPARENT       = 1 <<  6,
   PRIVATE_GTK_DIRECTION_SET     = 1 <<  7,   /* If the reading direction is not DIR_NONE */
   PRIVATE_GTK_DIRECTION_LTR     = 1 <<  8,   /* If the reading direction is DIR_LTR */
-  PRIVATE_GTK_ANCHORED          = 1 <<  9    /* If widget has a GtkWindow ancestor */
+  PRIVATE_GTK_ANCHORED          = 1 <<  9,   /* If widget has a GtkWindow ancestor */
+  PRIVATE_GTK_ACCELS_LOCKED     = 1 <<  10   /* If accelerators are locked in widget */
 } GtkPrivateFlags;
 
 /* Macros for extracting a widgets private_flags from GtkWidget.
@@ -64,6 +65,7 @@
 #define GTK_WIDGET_DIRECTION_SET(obj)	  ((GTK_PRIVATE_FLAGS (obj) & PRIVATE_GTK_DIRECTION_SET) != 0)
 #define GTK_WIDGET_DIRECTION_LTR(obj)     ((GTK_PRIVATE_FLAGS (obj) & PRIVATE_GTK_DIRECTION_LTR) != 0)
 #define GTK_WIDGET_ANCHORED(obj)          ((GTK_PRIVATE_FLAGS (obj) & PRIVATE_GTK_ANCHORED) != 0)
+#define GTK_WIDGET_ACCELS_LOCKED(obj)     ((GTK_PRIVATE_FLAGS (obj) & PRIVATE_GTK_ACCELS_LOCKED) != 0)
 
 /* Macros for setting and clearing private widget flags.
  * we use a preprocessor string concatenation here for a clear
Index: gtk/gtkwidget.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkwidget.c,v
retrieving revision 1.201
diff -u -r1.201 gtkwidget.c
--- gtk/gtkwidget.c	2001/03/29 23:02:26	1.201
+++ gtk/gtkwidget.c	2001/03/29 23:57:39
@@ -168,10 +168,21 @@
 static gboolean gtk_widget_real_focus_out_event   (GtkWidget     *widget,
 						   GdkEventFocus *event);
 
-static void gtk_widget_style_set		 (GtkWidget	    *widget,
-						  GtkStyle          *previous_style);
-static void gtk_widget_direction_changed	 (GtkWidget	    *widget,
-						  GtkTextDirection   previous_direction);
+static void gtk_widget_style_set               (GtkWidget        *widget,
+						GtkStyle         *previous_style);
+static void gtk_widget_direction_changed       (GtkWidget        *widget,
+						GtkTextDirection  previous_direction);
+static void gtk_widget_real_add_accelerator    (GtkWidget        *widget,
+						guint             accel_signal_id,
+						GtkAccelGroup    *accel_group,
+						guint             accel_key,
+						GdkModifierType   accel_mods,
+						GtkAccelFlags     accel_flags);
+static void gtk_widget_real_remove_accelerator (GtkWidget        *widget,
+						GtkAccelGroup    *accel_group,
+						guint             accel_key,
+						GdkModifierType   accel_mods);
+
 static void gtk_widget_real_grab_focus           (GtkWidget         *focus_widget);
 
 static GdkColormap*  gtk_widget_peek_colormap      (void);
@@ -313,8 +324,8 @@
   klass->hierarchy_changed = NULL;
   klass->style_set = gtk_widget_style_set;
   klass->direction_changed = gtk_widget_direction_changed;
-  klass->add_accelerator = (void*) gtk_accel_group_handle_add;
-  klass->remove_accelerator = (void*) gtk_accel_group_handle_remove;
+  klass->add_accelerator = gtk_widget_real_add_accelerator;
+  klass->remove_accelerator = gtk_widget_real_remove_accelerator;
   klass->activate_mnemonic = gtk_widget_real_activate_mnemonic;
   klass->grab_focus = gtk_widget_real_grab_focus;
   klass->event = NULL;
@@ -1087,6 +1098,7 @@
 			GTK_PARENT_SENSITIVE |
 			(composite_child_stack ? GTK_COMPOSITE_CHILD : 0) |
 			GTK_DOUBLE_BUFFERED);
+  GTK_PRIVATE_SET_FLAG (widget, GTK_ACCELS_LOCKED);
 
   widget->style = gtk_widget_peek_style ();
   gtk_style_ref (widget->style);
@@ -2090,41 +2102,13 @@
      }
 }
 
-static void
-gtk_widget_stop_add_accelerator (GtkWidget *widget)
-{
-  g_return_if_fail (widget != NULL);
-  g_return_if_fail (GTK_IS_WIDGET (widget));
-
-  gtk_signal_emit_stop (GTK_OBJECT (widget), widget_signals[ADD_ACCELERATOR]);
-}
-
-static void
-gtk_widget_stop_remove_accelerator (GtkWidget *widget)
-{
-  g_return_if_fail (widget != NULL);
-  g_return_if_fail (GTK_IS_WIDGET (widget));
-
-  gtk_signal_emit_stop (GTK_OBJECT (widget), widget_signals[REMOVE_ACCELERATOR]);
-}
-
 void
 gtk_widget_lock_accelerators (GtkWidget *widget)
 {
   g_return_if_fail (widget != NULL);
   g_return_if_fail (GTK_IS_WIDGET (widget));
-  
-  if (!gtk_widget_accelerators_locked (widget))
-    {
-      gtk_signal_connect (GTK_OBJECT (widget),
-			  "add_accelerator",
-			  GTK_SIGNAL_FUNC (gtk_widget_stop_add_accelerator),
-			  NULL);
-      gtk_signal_connect (GTK_OBJECT (widget),
-			  "remove_accelerator",
-			  GTK_SIGNAL_FUNC (gtk_widget_stop_remove_accelerator),
-			  NULL);
-    }
+
+  GTK_PRIVATE_SET_FLAG (widget, GTK_ACCELS_LOCKED);
 }
 
 void
@@ -2132,28 +2116,16 @@
 {
   g_return_if_fail (widget != NULL);
   g_return_if_fail (GTK_IS_WIDGET (widget));
-  
-  if (gtk_widget_accelerators_locked (widget))
-    {
-      gtk_signal_disconnect_by_func (GTK_OBJECT (widget),
-				     GTK_SIGNAL_FUNC (gtk_widget_stop_add_accelerator),
-				     NULL);
-      gtk_signal_disconnect_by_func (GTK_OBJECT (widget),
-				     GTK_SIGNAL_FUNC (gtk_widget_stop_remove_accelerator),
-				     NULL);
-    }
+
+  GTK_PRIVATE_UNSET_FLAG (widget, GTK_ACCELS_LOCKED);
 }
 
 gboolean
 gtk_widget_accelerators_locked (GtkWidget *widget)
 {
   g_return_val_if_fail (GTK_IS_WIDGET (widget), FALSE);
-  
-  return gtk_signal_handler_pending_by_func (GTK_OBJECT (widget),
-					     widget_signals[ADD_ACCELERATOR],
-					     TRUE,
-					     GTK_SIGNAL_FUNC (gtk_widget_stop_add_accelerator),
-					     NULL) > 0;
+
+  return GTK_WIDGET_ACCELS_LOCKED (widget);
 }
 
 void
@@ -3555,6 +3527,44 @@
 			      GtkTextDirection  previous_direction)
 {
   gtk_widget_queue_resize (widget);
+}
+
+static void
+gtk_widget_real_add_accelerator (GtkWidget        *widget,
+				 guint             accel_signal_id,
+				 GtkAccelGroup    *accel_group,
+				 guint             accel_key,
+				 GdkModifierType   accel_mods,
+				 GtkAccelFlags     accel_flags)
+{
+  /* We now only stop interactive changing through the menus when an accelerator
+   * is locked.
+   */
+#if 0  
+  if (gtk_widget_accelerators_locked (widget))
+    return;
+#endif  
+
+  gtk_accel_group_handle_add (GTK_OBJECT (widget), accel_signal_id, accel_group,
+			      accel_key, accel_mods, accel_flags);
+}
+
+static void
+gtk_widget_real_remove_accelerator (GtkWidget        *widget,
+				    GtkAccelGroup    *accel_group,
+				    guint             accel_key,
+				    GdkModifierType   accel_mods)
+{
+  /* We now only stop interactive changing through the menus when an accelerator
+   * is locked.
+   */
+#if 0
+  if (gtk_widget_accelerators_locked (widget))
+    return;
+#endif  
+
+  gtk_accel_group_handle_remove (GTK_OBJECT (widget), accel_group,
+				 accel_key, accel_mods);
 }
 
 static void