fixed the collapsing and expanding of the metadata widget, removed some unnecessary gtk3 stuff I thought that was necessary, tidied metadatawidget plus added a timeout for the metadata prop update query

3 files changed, 43 insertions, 95 deletions

diff --git a/src/metadata-widget.c b/src/metadata-widget.c
index d522358..f3f856c 100644
--- a/src/metadata-widget.c
+++ b/src/metadata-widget.c
@@ -59,14 +59,6 @@ static void metadata_widget_finalize      (GObject *object);
 static void metadata_widget_set_style     (GtkWidget* button, GtkStyle* style);
 static void metadata_widget_set_twin_item (MetadataWidget* self,
                                            DbusmenuMenuitem* twin_item);
-static void metadata_get_preferred_width (GtkWidget *widget,
-            			                  gint      *minimal_width,
-                        			      gint      *natural_width);
-static void metadata_get_preferred_height (GtkWidget *widget,
-            			                   gint      *minimal_width,
-                        			       gint      *natural_width);
-                        			      
-
 // keyevent consumers
 static gboolean metadata_widget_button_release_event (GtkWidget *menuitem, 
                                                     GdkEventButton *event);
@@ -85,7 +77,7 @@ static void metadata_widget_selection_received_event_callback( GtkWidget
                                                                 guint             time,
                                                                 gpointer          user_data);
 
-#if GTK_CHECK_VERSION(3, 0, 0)  
+#if GTK_CHECK_VERSION(3, 0, 0)
 static gboolean metadata_widget_icon_triangle_draw_cb_gtk_3 (GtkWidget *image,
 															 cairo_t* cr,
 															 gpointer user_data);
@@ -115,36 +107,12 @@ metadata_widget_class_init (MetadataWidgetClass *klass)
 
   widget_class->button_release_event = metadata_widget_button_release_event;
   
-  #if GTK_CHECK_VERSION(3, 0, 0)  
-  widget_class->get_preferred_width = metadata_get_preferred_width;
-  widget_class->get_preferred_height = metadata_get_preferred_height;
-  #endif
   g_type_class_add_private (klass, sizeof (MetadataWidgetPrivate));
 
   gobject_class->dispose = metadata_widget_dispose;
   gobject_class->finalize = metadata_widget_finalize;
 }
 
-#if GTK_CHECK_VERSION(3, 0, 0)  
-static void
-metadata_get_preferred_width (GtkWidget *widget,
-                              gint      *minimal_width,
-                              gint      *natural_width)
-{
-  *minimal_width = *natural_width = 200;
-}
-
-static void
-metadata_get_preferred_height (GtkWidget *widget,
-                               gint      *minimal_height,
-                               gint      *natural_height)
-{
-  //MetadataWidget* self = METADATA_WIDGET (widget);
-  //MetadataWidgetPrivate * priv = METADATA_WIDGET_GET_PRIVATE(self);	
-  *minimal_height = *natural_height = 95;//priv->current_height;
-}
-#endif
-
 static void
 metadata_widget_init (MetadataWidget *self)
 {
@@ -712,10 +680,7 @@ metadata_widget_property_update(DbusmenuMenuitem* item, gchar* property,
   else if(g_ascii_strcasecmp(DBUSMENU_METADATA_MENUITEM_ARTURL, property) == 0){
     g_string_erase(priv->image_path, 0, -1);
     g_string_overwrite(priv->image_path, 0, g_variant_get_string (value, NULL));
-    // if its a remote image queue a redraw incase the download took too long
-    //if (g_str_has_prefix(g_variant_get_string (value, NULL), g_get_user_cache_dir())){
     gtk_widget_queue_draw(GTK_WIDGET(mitem));
-    //}
   }
   else if (g_ascii_strcasecmp (DBUSMENU_METADATA_MENUITEM_PLAYER_NAME, property) == 0){
     gtk_label_set_label (GTK_LABEL (priv->player_label),
@@ -734,33 +699,14 @@ metadata_widget_handle_resizing (MetadataWidget* self)
 {
   MetadataWidgetPrivate * priv = METADATA_WIDGET_GET_PRIVATE(self);
   
-  g_debug ("SHOW/HIDE TRACK DETAILS - %i",	dbusmenu_menuitem_property_get_bool (priv->twin_item, DBUSMENU_METADATA_MENUITEM_HIDE_TRACK_DETAILS));  
+  //g_debug ("SHOW/HIDE TRACK DETAILS - %i",	dbusmenu_menuitem_property_get_bool (priv->twin_item, DBUSMENU_METADATA_MENUITEM_HIDE_TRACK_DETAILS));  
   
   if (dbusmenu_menuitem_property_get_bool (priv->twin_item,
                                            DBUSMENU_METADATA_MENUITEM_HIDE_TRACK_DETAILS) == TRUE){
-    // TODO
-    // revert to hide                                             
-    #if GTK_CHECK_VERSION(3, 0, 0)  
-    gtk_widget_show (priv->meta_data_h_box);
-    #else
     gtk_widget_hide (priv->meta_data_h_box);
-    gtk_widget_hide (priv->artist_label);
-    gtk_widget_hide (priv->piece_label);
-    gtk_widget_hide (priv->container_label); 
-    gtk_widget_hide (priv->album_art); 
-    gtk_widget_hide (priv->meta_data_v_box);
-    #endif 	
   }
   else{
-    #if GTK_CHECK_VERSION(3, 0, 0)  
-    gtk_widget_show (priv->meta_data_h_box);
-    #else    
     gtk_widget_show (priv->meta_data_h_box);     
-    gtk_widget_show (priv->artist_label);
-    gtk_widget_show (priv->piece_label);
-    gtk_widget_show (priv->container_label); 
-    gtk_widget_show (priv->album_art); 
-    #endif 
   }
   gtk_widget_queue_draw(GTK_WIDGET(self));      
 }
diff --git a/src/mpris2-controller.vala b/src/mpris2-controller.vala
index 2975066..8eeac08 100644
--- a/src/mpris2-controller.vala
+++ b/src/mpris2-controller.vala
@@ -19,10 +19,6 @@ with this program.  If not, see <http://www.gnu.org/licenses/>.
 using Dbusmenu;
 using Transport;
 
-/*
- This class will entirely replace mpris-controller.vala hence why there is no
- point in trying to get encorporate both into the same object model. 
- */
 public class Mpris2Controller : GLib.Object
 {
   public MprisRoot mpris2_root {get; construct;}    
@@ -60,12 +56,22 @@ public class Mpris2Controller : GLib.Object
       critical("Problems connecting to the session bus - %s", e.message);
     }
   }
-
+  /*
+   * property_changed_cb
+   * Called when a property changed signal is emitted from any of mpris
+   * objects on the bus.
+   * Note that the signal will be received by each instance for each player
+   * and at that moment there is no way to know what player that signal 
+   * came from therefore it is necessary to query each relevant property
+   * to update the respective dbusmenuitem property inorder to keep the UI in sync
+   * Please also note due to some race condition in the depths of gdbus
+   * a timeout is needed between receiving the prop update and query the respective property.
+   * This can be seen at various points below.
+   */
   public void property_changed_cb ( string interface_source,
                                     HashTable<string, Variant?> changed_properties,
                                     string[] invalid )
   {
-    //debug("properties-changed for interface %s and owner %s", interface_source, this.owner.dbus_name);
     if ( changed_properties == null ||
         interface_source.has_prefix ( MPRIS_PREFIX ) == false ){
       warning("Property-changed hash is null or this is an interface that doesn't concern us");
@@ -73,31 +79,18 @@ public class Mpris2Controller : GLib.Object
     }
     Variant? play_v = changed_properties.lookup("PlaybackStatus");
     if(play_v != null){
-      // Race condition sometimes appears with the playback status 
-      // 200ms timeout ensures we have the correct playback status at all times.
       string state = this.player.PlaybackStatus;
-      //debug("in the property update and the playback status = %s and update = %s", state, (string)play_v);
       Timeout.add ( 200, ensure_correct_playback_status );
       Transport.State p = (Transport.State)this.determine_play_state(state);
       (this.owner.custom_items[PlayerController.widget_order.TRANSPORT] as TransportMenuitem).change_play_state(p);
     }
     Variant? meta_v = changed_properties.lookup("Metadata");
-    if(meta_v != null){
-      GLib.HashTable<string, Variant?> changed_updates = clean_metadata();      
-      PlayerItem metadata = this.owner.custom_items[PlayerController.widget_order.METADATA];
-      metadata.reset (MetadataMenuitem.relevant_attributes_for_ui());
-      metadata.update ( changed_updates, 
-                        MetadataMenuitem.relevant_attributes_for_ui());
-      MetadataMenuitem md = this.owner.custom_items[PlayerController.widget_order.METADATA] as MetadataMenuitem;      
-      bool collapsing = !metadata.populated(MetadataMenuitem.relevant_attributes_for_ui());
-      md.should_collapse(collapsing);
-      
-      debug ("Should metadata collapse %s", collapsing.to_string());
+    if(meta_v != null)
+    {
+      Timeout.add ( 200, ensure_correct_metadata );
     }
     Variant? playlist_v = changed_properties.lookup("ActivePlaylist");
     if ( playlist_v != null && this.owner.use_playlists == true ){
-      // Once again A GDBus race condition, the property_changed signal is sent
-      // before the value is set on the respective property.
       Timeout.add (300, this.fetch_active_playlist);
     }
     Variant? playlist_count_v = changed_properties.lookup("PlaylistCount");
@@ -116,9 +109,23 @@ public class Mpris2Controller : GLib.Object
       md.alter_label (this.mpris2_root.Identity);
     }
   }
-                                      
-  private bool ensure_correct_playback_status(){
-    //debug("TEST playback status = %s", this.player.PlaybackStatus);
+         
+  private bool ensure_correct_metadata ()
+  {
+    GLib.HashTable<string, Variant?> changed_updates = clean_metadata();      
+    PlayerItem metadata = this.owner.custom_items[PlayerController.widget_order.METADATA];
+    metadata.reset (MetadataMenuitem.relevant_attributes_for_ui());
+    metadata.update ( changed_updates, 
+                      MetadataMenuitem.relevant_attributes_for_ui());
+    MetadataMenuitem md = this.owner.custom_items[PlayerController.widget_order.METADATA] as MetadataMenuitem;      
+    bool collapsing = !metadata.populated(MetadataMenuitem.relevant_attributes_for_ui());
+    md.should_collapse(collapsing);
+    
+    return false;
+  }
+  
+  private bool ensure_correct_playback_status()
+  {
     Transport.State p = (Transport.State)this.determine_play_state(this.player.PlaybackStatus);
     (this.owner.custom_items[PlayerController.widget_order.TRANSPORT] as TransportMenuitem).change_play_state(p);
     return false;
@@ -130,19 +137,16 @@ public class Mpris2Controller : GLib.Object
     
     Variant? artist_v = this.player.Metadata.lookup("xesam:artist");
     if(artist_v != null){
-      Variant? v_artists = this.player.Metadata.lookup("xesam:artist");
-      //debug("artists is of type %s", v_artists.get_type_string ());
       string display_artists;
-      if(v_artists.get_type_string() == "s"){
-        //debug("SPOTIFY is that you ?");
-        display_artists = v_artists.get_string();
+      // Accomodate Spotify (should return 'as' and not 's') 
+      if(artist_v.get_type_string() == "s"){
+        display_artists = artist_v.get_string();
       }
       else{
-       string[] artists = v_artists.dup_strv();
+        string[] artists = artist_v.dup_strv();
         display_artists = string.joinv(", ", artists);
       }
       changed_updates.replace("xesam:artist", display_artists);
-      //debug("artist : %s", (string)changed_updates.lookup("xesam:artist"));
     }
     return changed_updates;
   }
@@ -181,7 +185,6 @@ public class Mpris2Controller : GLib.Object
 
   public void transport_update(Transport.Action command)
   {
-    //debug("transport_event input = %i", (int)command);
     if(command == Transport.Action.PLAY_PAUSE){
       this.player.PlayPause.begin();              
     }
@@ -192,11 +195,9 @@ public class Mpris2Controller : GLib.Object
       this.player.Next.begin();
     }
     else if(command == Transport.Action.REWIND){
-     //debug("transport_event rewind = %i", (int)command);
      this.player.Seek.begin(-500000);
     }
     else if(command == Transport.Action.FORWIND){
-     //debug("transport_event input = %i", (int)command);
      this.player.Seek.begin(400000);
     }
   }
@@ -230,12 +231,10 @@ public class Mpris2Controller : GLib.Object
                                                               false);
     }
     catch (IOError e){
-      //debug("Could not fetch playlists because %s", e.message);
       return;
     }
     
     if( current_playlists != null ){
-      //debug( "Size of the playlist array = %i", current_playlists.length );
       PlaylistsMenuitem playlists_item = this.owner.custom_items[PlayerController.widget_order.PLAYLISTS] as PlaylistsMenuitem;
       playlists_item.update(current_playlists);
     }
@@ -248,7 +247,8 @@ public class Mpris2Controller : GLib.Object
   private bool fetch_active_playlist()
   {    
     if (this.playlists.ActivePlaylist.valid == false){
-      //debug(" We don't have an active playlist");
+      // TODO 
+      // What happens here ?
     }    
     PlaylistsMenuitem playlists_item = this.owner.custom_items[PlayerController.widget_order.PLAYLISTS] as PlaylistsMenuitem;
     playlists_item.active_playlist_update ( this.playlists.ActivePlaylist.details );
@@ -261,7 +261,7 @@ public class Mpris2Controller : GLib.Object
       this.playlists.ActivatePlaylist.begin(path);
     }
     catch(IOError e){
-      //debug("Could not activate playlist %s because %s", (string)path, e.message);
+      warning ("Could not activate playlist %s because %s", (string)path, e.message);
     }
   }
 }
diff --git a/src/player-item.vala b/src/player-item.vala
index 162dbea..7867653 100644
--- a/src/player-item.vala
+++ b/src/player-item.vala
@@ -61,6 +61,8 @@ public class PlayerItem : Dbusmenu.Menuitem
       //debug("search key = %s", search_key);
       Variant? v = data.lookup(search_key);
       
+      if (v == null) continue;
+      
       if (v.is_of_type ( VariantType.STRING )){
         string update = v.get_string().strip();
         //debug("with value : %s", update);