Merge lp:~cjcurran/indicator-sound/058-fixes into lp:~indicator-applet-developers/indicator-sound/trunk_3

[Conor Curran]

Fixes both bugs attached

[Kalle Valo (kvalo)]

Few questions:

o Casts always make suspicious. Why is cast needed for function pointers, eg this one:

  DbusmenuClientTypeHandler)new_volume_slider_widget

o "gboolean user_data" is very confusing. Either it should be of type gpointer or, if it's really a boolean, it should have some other name.

review: Needs Information

[Conor Curran (cjcurran)]

Well spotted, sorry silly of me.
Fixed in revision 192

[Kalle Valo (kvalo)]

Thanks, now looks good to me. Approved.

review: Approve

=== modified file 'src/indicator-sound.c'
--- src/indicator-sound.c	2011-01-27 22:10:48 +0000
+++ src/indicator-sound.c	2011-01-28 13:00:46 +0000
@@ -77,19 +77,22 @@
 //custom widget realisation methods
 static gboolean new_volume_slider_widget (DbusmenuMenuitem * newitem,
                                           DbusmenuMenuitem * parent,
-                                          DbusmenuClient * client);
+                                          DbusmenuClient * client,
+                                          gpointer user_data);
 static gboolean new_transport_widget (DbusmenuMenuitem * newitem,
                                       DbusmenuMenuitem * parent,
-                                      DbusmenuClient * client);
+                                      DbusmenuClient * client,
+                                      gpointer user_data);                                     
 static gboolean new_metadata_widget (DbusmenuMenuitem * newitem,
                                      DbusmenuMenuitem * parent,
-                                     DbusmenuClient * client);
+                                     DbusmenuClient * client,
+                                     gpointer user_data);
 static gboolean new_title_widget (DbusmenuMenuitem * newitem,
                                   DbusmenuMenuitem * parent,
-                                  DbusmenuClient * client);
+                                  DbusmenuClient * client,
+                                  gpointer user_data);
 
 // DBUS communication
-
 static GDBusNodeInfo *node_info = NULL;
 static GDBusInterfaceInfo *interface_info = NULL;
 static void create_connection_to_service (GObject *source_object,
@@ -185,10 +188,18 @@
         
   DbusmenuGtkClient *client = dbusmenu_gtkmenu_get_client(menu);
   g_object_set_data (G_OBJECT (client), "indicator", io);
-  dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), DBUSMENU_VOLUME_MENUITEM_TYPE, new_volume_slider_widget);
-  dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), DBUSMENU_TRANSPORT_MENUITEM_TYPE, new_transport_widget);
-  dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), DBUSMENU_METADATA_MENUITEM_TYPE, new_metadata_widget);
-  dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), DBUSMENU_TITLE_MENUITEM_TYPE, new_title_widget);
+  dbusmenu_client_add_type_handler (DBUSMENU_CLIENT(client),
+                                    DBUSMENU_VOLUME_MENUITEM_TYPE,
+                                    new_volume_slider_widget);
+  dbusmenu_client_add_type_handler (DBUSMENU_CLIENT(client),
+                                    DBUSMENU_TRANSPORT_MENUITEM_TYPE,
+                                    new_transport_widget);
+  dbusmenu_client_add_type_handler (DBUSMENU_CLIENT(client),
+                                    DBUSMENU_METADATA_MENUITEM_TYPE,
+                                    new_metadata_widget);
+  dbusmenu_client_add_type_handler (DBUSMENU_CLIENT(client),
+                                    DBUSMENU_TITLE_MENUITEM_TYPE,
+                                    new_title_widget);
   // Note: Not ideal but all key handling needs to be managed here and then 
   // delegated to the appropriate widget. 
   g_signal_connect (menu, "key-press-event", G_CALLBACK(key_press_cb), io);
@@ -286,7 +297,8 @@
 static gboolean
 new_transport_widget (DbusmenuMenuitem * newitem,
                       DbusmenuMenuitem * parent,
-                      DbusmenuClient * client)
+                      DbusmenuClient * client,
+                      gpointer user_data)
 {
   g_debug("indicator-sound: new_transport_bar() called ");
 
@@ -314,7 +326,8 @@
 static gboolean
 new_metadata_widget (DbusmenuMenuitem * newitem,
                      DbusmenuMenuitem * parent,
-                     DbusmenuClient * client)
+                     DbusmenuClient * client,
+                     gpointer user_data)
 {
   g_debug("indicator-sound: new_metadata_widget");
 
@@ -335,7 +348,8 @@
 static gboolean
 new_title_widget(DbusmenuMenuitem * newitem,
                  DbusmenuMenuitem * parent,
-                 DbusmenuClient * client)
+                 DbusmenuClient * client,
+                 gpointer user_data)
 {
   g_return_val_if_fail(DBUSMENU_IS_MENUITEM(newitem), FALSE);
   g_return_val_if_fail(DBUSMENU_IS_GTKCLIENT(client), FALSE);
@@ -358,7 +372,8 @@
 static gboolean
 new_volume_slider_widget(DbusmenuMenuitem * newitem,
                          DbusmenuMenuitem * parent,
-                         DbusmenuClient * client)
+                         DbusmenuClient * client,
+                         gpointer user_data)
 {
   g_debug("indicator-sound: new_volume_slider_widget");
 

=== modified file 'src/transport-widget.c'
--- src/transport-widget.c	2011-01-27 18:11:32 +0000
+++ src/transport-widget.c	2011-01-28 13:00:46 +0000
@@ -129,10 +129,10 @@
                                                  TransportWidgetState update);
 static void transport_widget_select (GtkItem* menu, gpointer Userdata);
 static void transport_widget_deselect (GtkItem* menu, gpointer Userdata);
+static TransportWidgetEvent transport_widget_collision_detection (gint x, gint y);
 
 
 /// Init functions //////////////////////////////////////////////////////////
-
 static void
 transport_widget_class_init (TransportWidgetClass *klass)
 { 
@@ -301,30 +301,23 @@
   TransportWidgetPrivate* priv = TRANSPORT_WIDGET_GET_PRIVATE ( TRANSPORT_WIDGET(menuitem) );
   
   priv->motion_event = TRANSPORT_NADA;
-    cairo_t *cr;
-    cr = gdk_cairo_create (menuitem->window);
-    draw ( menuitem, cr );
-    cairo_destroy ( cr );
-
+  priv->current_command = TRANSPORT_NADA;
+  gtk_widget_queue_draw (GTK_WIDGET(menuitem));
+  
   return TRUE;
 }
 
-/* keyevents */
 static gboolean
 transport_widget_button_press_event (GtkWidget *menuitem, 
-                                    GdkEventButton *event)
+                                     GdkEventButton *event)
 {
   g_return_val_if_fail ( IS_TRANSPORT_WIDGET(menuitem), FALSE );
   TransportWidgetPrivate* priv = TRANSPORT_WIDGET_GET_PRIVATE ( TRANSPORT_WIDGET(menuitem) );
   TransportWidgetEvent result = transport_widget_determine_button_event ( TRANSPORT_WIDGET(menuitem),
                                                                             event);
-    
   if(result != TRANSPORT_NADA){
     priv->current_command = result;
-    cairo_t *cr;
-    cr = gdk_cairo_create (menuitem->window);
-    draw ( menuitem, cr );
-    cairo_destroy ( cr );
+    gtk_widget_queue_draw (GTK_WIDGET(menuitem));
   }
   return TRUE;
 }
@@ -333,14 +326,12 @@
 transport_widget_button_release_event (GtkWidget *menuitem, 
                                        GdkEventButton *event)
 {
-    //g_debug("TransportWidget::menu_release_event");
   g_return_val_if_fail(IS_TRANSPORT_WIDGET(menuitem), FALSE);
   TransportWidget* transport = TRANSPORT_WIDGET(menuitem);
   TransportWidgetPrivate * priv = TRANSPORT_WIDGET_GET_PRIVATE ( transport ); 
   TransportWidgetEvent result = transport_widget_determine_button_event ( transport,
                                                                           event );
-  if(result != TRANSPORT_NADA){
-    //g_debug("TransportWidget::menu_press_event - going to send value %i", (int)result);
+  if (result != TRANSPORT_NADA && priv->current_command == result){
     GVariant* new_transport_state = g_variant_new_int32 ((int)result);
     dbusmenu_menuitem_handle_event ( priv->twin_item,
                                      "Transport state change",
@@ -376,17 +367,8 @@
     TransportWidgetPrivate * priv = TRANSPORT_WIDGET_GET_PRIVATE ( transport );
     priv->current_command = transport_event;
     priv->key_event = transport_event;
-/*    printf("transport_widget_react_to_key_press_event: before drawing\n");*/
     gtk_widget_realize ( GTK_WIDGET(transport) );
-
-    printf ( "transport widget - react to key press event -> is the window null: %i",
-              gtk_widget_get_window (GTK_WIDGET (transport) ) == NULL );
-    cairo_t *cr;
-
-    printf("transport_widget_react_to_key_press_event: before drawing\n");
-    cr = gdk_cairo_create ( GTK_WIDGET(transport)->window );
-    draw ( GTK_WIDGET(transport), cr );
-    cairo_destroy (cr);
+    gtk_widget_queue_draw (GTK_WIDGET(transport) );
   } 
 }
 
@@ -396,7 +378,6 @@
 {
   if(transport_event != TRANSPORT_NADA){
     TransportWidgetPrivate * priv = TRANSPORT_WIDGET_GET_PRIVATE ( transport );     
-    //g_debug("TransportWidget::menu_press_event - going to send value %i", (int)result);
     GVariant* new_transport_event = g_variant_new_int32((int)transport_event);  
     dbusmenu_menuitem_handle_event ( priv->twin_item,
                                      "Transport state change",
@@ -412,53 +393,41 @@
 {
   TransportWidgetPrivate * priv = TRANSPORT_WIDGET_GET_PRIVATE ( transport );     
   priv->has_focus = focus;  
-  g_debug("new focus update = %i", focus);
 }
 
 static TransportWidgetEvent
 transport_widget_determine_button_event( TransportWidget* button,
                                          GdkEventButton* event )
 {
-  //g_debug("event x coordinate = %f", event->x);
-  //g_debug("event y coordinate = %f", event->y);
-  TransportWidgetEvent button_event = TRANSPORT_NADA;
-  // For now very simple rectangular collision detection
-  if(event->x > 67 && event->x < 112
-     && event->y > 12 && event->y < 40){
-    button_event = TRANSPORT_PREVIOUS;
-  }
-  else if(event->x > 111 && event->x < 153
-     && event->y > 5 && event->y < 47){
-    button_event = TRANSPORT_PLAY_PAUSE;  
-  }
-  else if(event->x > 152 && event->x < 197
-     && event->y > 12 && event->y < 40){
-    button_event = TRANSPORT_NEXT;
-  } 
-  return button_event;  
+  return transport_widget_collision_detection (event->x, event->y);
 }
 
 static TransportWidgetEvent
 transport_widget_determine_motion_event( TransportWidget* button,
                                          GdkEventMotion* event )
 {
-/*  g_debug("event x coordinate = %f", event->x);*/
-/*  g_debug("event y coordinate = %f", event->y);*/
-  TransportWidgetEvent motion_event = TRANSPORT_NADA;
-  // For now very simple rectangular collision detection
-  if(event->x > 67 && event->x < 112
-     && event->y > 12 && event->y < 40){
-    motion_event = TRANSPORT_PREVIOUS;
-  }
-  else if(event->x > 111 && event->x < 153
-     && event->y > 5 && event->y < 47){
-    motion_event = TRANSPORT_PLAY_PAUSE;  
-  }
-  else if(event->x > 152 && event->x < 197
-     && event->y > 12 && event->y < 40){
-    motion_event = TRANSPORT_NEXT;
-  } 
-  return motion_event;  
+  return transport_widget_collision_detection (event->x, event->y);
+}
+
+static TransportWidgetEvent
+transport_widget_collision_detection ( gint x,
+                                       gint y )
+{
+  TransportWidgetEvent event = TRANSPORT_NADA;
+  
+  if (x > 67 && x < 112
+      && y > 12 && y < 40){
+    event = TRANSPORT_PREVIOUS;
+  }
+  else if (x > 111 && x < 153
+           && y > 5 && y < 47){
+    event = TRANSPORT_PLAY_PAUSE;  
+  }
+  else if (x > 152 && x < 197
+           && y > 12 && y < 40){
+    event = TRANSPORT_NEXT;
+  }   
+  return event;
 }
 
 static void 
@@ -467,17 +436,11 @@
 {
   g_return_if_fail(IS_TRANSPORT_WIDGET(button));
   TransportWidgetPrivate* priv = TRANSPORT_WIDGET_GET_PRIVATE(button);  
-  if(priv->current_command != TRANSPORT_NADA &&
-          command != TRANSPORT_NADA){           
-    priv->current_command = command;
-  }
-  cairo_t *cr;  
-  cr = gdk_cairo_create ( GTK_WIDGET(button)->window );
 
   priv->current_command = TRANSPORT_NADA;
   priv->key_event = TRANSPORT_NADA;
-  draw ( GTK_WIDGET(button), cr );
-  cairo_destroy (cr);
+
+  gtk_widget_queue_draw (GTK_WIDGET(button));
 }
 
 /// internal helper functions //////////////////////////////////////////////////