Comment 16 for bug 332767

Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Thank you for your work there. Some comments on the patch though:

- self_add_actions(self);
+ if ( mn_popups_can_use_actions == 1 )
+ self_add_actions(self);

This should be "if ( mn_popups_can_use_actions (self) == 1 )" shouldn't it?

+ mn_popups_server_caps = notify_get_server_caps();

It seems you leak this list and it's elements here. Each element of the GList should be g_free'd, and the list should be g_list_free'd once you've finished with it.

In fact, it seems that there is already a convenience function in mn-util.c for doing this (mn_g_list_free_deep_custom), so you should use this to free your list.

+ public bool can_use_actions(void)
+ {
+ /* Returns True if notification server will make use of actions, False if not */
+ return mn_popups_server_caps_actions;
+ }

This function should really return a gboolean, to keep it in the same style as the existing code.

+ /* Notify-OSD */
+ if (notify_get_server_info(&notify_name, &notify_vendor, &notify_version, &notify_spec_version))
+ {
+ if (! strcmp(notify_name, "notify-osd"))
+ {
+ gtk_widget_set_sensitive(selfp->popups_position_section_label, FALSE);
+ gtk_widget_set_sensitive(selfp->popups_position_attached_radio, FALSE);
+ gtk_widget_set_sensitive(selfp->popups_position_free_radio, FALSE);
+ gtk_widget_set_sensitive(selfp->popups_expiration_section_label, FALSE);
+ gtk_widget_set_sensitive(selfp->popups_expiration_default_radio, FALSE);
+ gtk_widget_set_sensitive(selfp->popups_expiration_never_radio, FALSE);
+ gtk_widget_set_sensitive(selfp->popups_expiration_after_radio, FALSE);
+ gtk_widget_set_sensitive(selfp->popups_expiration_scale, FALSE);
+ }
+ }

It seems you just make these insensitive, but the spec says that they should not be visible.

Thanks again for your effort there :)