Merge lp:~3v1n0/libappindicator/scroll-event-support into lp:libappindicator

[Marco Trevisan (Treviño)]

libappindicator now when it gets a dbus call from indicator-application about the incoming scroll event, sends a "scroll-event" signal to the appindicator application.

Depends on:
lp:~3v1n0/libindicator/scroll-event-support
lp:~3v1n0/indicator-application/scroll-event-support

This is part of the Indicators "scroll-event" support patch, see bug #708180

[Ted Gould (ted)]

http://www.notmart.org/misc/statusnotifieritem/statusnotifieritem.html#Scroll

Same as indicator-application, should export the SNI method.

I also don't understand this code:

117 +
118 + char * name = g_dbus_proxy_get_name_owner(app->priv->watcher_proxy);
119 + if (name != NULL) {
120 + g_free(name);
121 + }

Why is the name being gotten here?

Lastly the reserved don't need to be taken away when there is already a slot reserved for it. You're taking away from the class structure twice:

- void (*app_indicator_reserved_6)(void);

review: Needs Fixing

[Marco Trevisan (Treviño) (3v1n0)]

> http://www.notmart.org/misc/statusnotifieritem/statusnotifieritem.html#Scroll
>
> Same as indicator-application, should export the SNI method.

Ok, I'll do it.

> I also don't understand this code:
>
> 117 +
> 118 + char * name = g_dbus_proxy_get_name_owner(app->priv->watcher_proxy);
> 119 + if (name != NULL) {
> 120 + g_free(name);
> 121 + }
> Why is the name being gotten here?

Oops, it was for debugging, I'll move it away.

> Lastly the reserved don't need to be taken away when there is already a slot
> reserved for it. You're taking away from the class structure twice:
>
> - void (*app_indicator_reserved_6)(void);

That's true... I removed it at the beginning, since I didn't know which one use. I'll reset it.

[lp:~3v1n0/libappindicator/scroll-event-support]

181. By Marco Trevisan (Treviño) on 2011-01-27

app-indicator: remove old debug code.

182. By Marco Trevisan (Treviño) on 2011-01-27

app-indicator: don't use two AppIndicator slots, reset one.

183. By Marco Trevisan (Treviño) on 2011-01-29

Using the SNI method "Scroll" instead of XAyatanaScrollAction

The indicator signal will use anyway the same syntax.

[Marco Trevisan (Treviño) (3v1n0)]

Fixed, now I'm using that method name and signature (wich uses a string for defining the orientation and a signed delta as scroll-event value), however I kept the scroll-entry signal of the AppIndicator as firstly implemented: using an integer for the delta and an unsigned int for the direction (which respect the GdkScrollDirection values in fact).

Is this fine? Or should I change the scroll-event signal signature too?

[lp:~3v1n0/libappindicator/scroll-event-support]

184. By Marco Trevisan (Treviño) on 2011-02-03

Merge against new trunk

[Ted Gould (ted)]

review: Approve

[Marco Trevisan (Treviño) (3v1n0)]

Thanks... Ah, just for having a better interface (especially for vala or mono bindings), wouldn't be better to use GdkScrollDirection instead of guint when doing the signal (since that is used in sources)?

So defining the scroll_event signal as:
   void (* scroll_event) (AppIndicator * indicator, gint delta, GdkScrollDirection direction, gpointer user_data);

I guess it would allow a more clear interface for programmers.

Let me know if I should change that, thanks.

review: Needs Information

[Ted Gould (ted)]

Sure, I think that'd be a good fix. Put it in a different branch though as I merged this one and it'll get confusing if it's not in a different branch :)

=== modified file 'bindings/mono/libappindicator-api.metadata'
--- bindings/mono/libappindicator-api.metadata	2010-09-20 18:46:30 +0000
+++ bindings/mono/libappindicator-api.metadata	2011-02-03 11:51:50 +0000
@@ -9,6 +9,8 @@
 	<attr path="/api/namespace/object[@cname='AppIndicator']/signal[@field_name='new_label']" name="cname">new-label</attr>
 	<attr path="/api/namespace/object[@cname='AppIndicator']/signal[@field_name='connection_changed']" name="name">ConnectionChanged</attr>
 	<attr path="/api/namespace/object[@cname='AppIndicator']/signal[@field_name='connection_changed']" name="cname">connection-changed</attr>
+	<attr path="/api/namespace/object[@cname='AppIndicator']/signal[@field_name='scroll_event']" name="name">ScrollEvent</attr>
+	<attr path="/api/namespace/object[@cname='AppIndicator']/signal[@field_name='scroll_event']" name="cname">scroll-event</attr>
 	<attr path="/api/namespace/object[@cname='AppIndicator']/signal[@field_name='new_icon']" name="name">NewIcon</attr>
 	<attr path="/api/namespace/object[@cname='AppIndicator']/signal[@field_name='new_icon']" name="cname">new-icon</attr>
 	<attr path="/api/namespace/object[@cname='AppIndicator']/signal[@field_name='new_icon_theme_path']" name="name">NewIconThemePath</attr>

=== modified file 'src/app-indicator.c'
--- src/app-indicator.c	2011-01-25 15:44:43 +0000
+++ src/app-indicator.c	2011-02-03 11:51:50 +0000
@@ -103,6 +103,7 @@
 	NEW_LABEL,
 	CONNECTION_CHANGED,
     NEW_ICON_THEME_PATH,
+    SCROLL_EVENT,
 	LAST_SIGNAL
 };
 
@@ -170,19 +171,22 @@
 static gboolean fallback_timer_expire (gpointer data);
 static GtkStatusIcon * fallback (AppIndicator * self);
 static void status_icon_status_wrapper (AppIndicator * self, const gchar * status, gpointer data);
+static gboolean scroll_event_wrapper(GtkWidget *status_icon, GdkEventScroll *event, gpointer user_data);
 static void status_icon_changes (AppIndicator * self, gpointer data);
 static void status_icon_activate (GtkStatusIcon * icon, gpointer data);
+static void status_icon_menu_activate (GtkStatusIcon *status_icon, guint button, guint activate_time, gpointer user_data);
 static void unfallback (AppIndicator * self, GtkStatusIcon * status_icon);
 static gchar * append_panel_icon_suffix (const gchar * icon_name);
 static void watcher_owner_changed (GObject * obj, GParamSpec * pspec, gpointer user_data);
 static void client_menu_changed (GtkWidget *widget, GtkWidget *child, AppIndicator *indicator);
 static void theme_changed_cb (GtkIconTheme * theme, gpointer user_data);
 static GVariant * bus_get_prop (GDBusConnection * connection, const gchar * sender, const gchar * path, const gchar * interface, const gchar * property, GError ** error, gpointer user_data);
+static void bus_method_call (GDBusConnection * connection, const gchar * sender, const gchar * path, const gchar * interface, const gchar * method, GVariant * params, GDBusMethodInvocation * invocation, gpointer user_data);
 static void bus_creation (GObject * obj, GAsyncResult * res, gpointer user_data);
 static void bus_watcher_ready (GObject * obj, GAsyncResult * res, gpointer user_data);
 
 static const GDBusInterfaceVTable item_interface_table = {
-	method_call:    NULL, /* No methods on this object */
+	method_call:    bus_method_call,
 	get_property:   bus_get_prop,
 	set_property:   NULL /* No properties that can be set */
 };
@@ -469,6 +473,21 @@
 	                                  g_cclosure_marshal_VOID__STRING,
 	                                  G_TYPE_NONE, 1, G_TYPE_STRING);
 
+	/**
+		AppIndicator::scroll-event:
+		@arg0: The #AppIndicator object
+
+		Signaled when there is a new icon set for the
+		object.
+	*/
+	signals[SCROLL_EVENT] = g_signal_new (APP_INDICATOR_SIGNAL_SCROLL_EVENT,
+	                                  G_TYPE_FROM_CLASS(klass),
+	                                  G_SIGNAL_RUN_LAST,
+	                                  G_STRUCT_OFFSET (AppIndicatorClass, scroll_event),
+	                                  NULL, NULL,
+	                                  _application_service_marshal_VOID__INT_UINT,
+	                                  G_TYPE_NONE, 2, G_TYPE_INT, G_TYPE_UINT);
+
 	/* DBus interfaces */
 	if (item_node_info == NULL) {
 		GError * error = NULL;
@@ -899,6 +918,43 @@
 	return;
 }
 
+static void
+bus_method_call (GDBusConnection * connection, const gchar * sender,
+                 const gchar * path, const gchar * interface,
+                 const gchar * method, GVariant * params,
+                 GDBusMethodInvocation * invocation, gpointer user_data)
+{
+	g_return_if_fail(IS_APP_INDICATOR(user_data));
+
+	AppIndicator * app = APP_INDICATOR(user_data);
+	GVariant * retval = NULL;
+
+	if (g_strcmp0(method, "Scroll") == 0) {
+		guint direction;
+		gint delta;
+		const gchar *orientation;
+
+		g_variant_get(params, "(i&s)", &delta, &orientation);
+
+		if (g_strcmp0(orientation, "horizontal") == 0) {
+			direction = (delta >= 0) ? GDK_SCROLL_RIGHT : GDK_SCROLL_LEFT;
+		} else if (g_strcmp0(orientation, "vertical") == 0) {
+			direction = (delta >= 0) ? GDK_SCROLL_DOWN : GDK_SCROLL_UP;
+		} else {
+			g_dbus_method_invocation_return_value(invocation, retval);
+			return;
+		}
+
+		delta = ABS(delta);
+		g_signal_emit(app, signals[SCROLL_EVENT], 0, delta, direction);
+
+	} else {
+		g_warning("Calling method '%s' on the app-indicator and it's unknown", method);
+	}
+
+	g_dbus_method_invocation_return_value(invocation, retval);
+}
+
 /* DBus is asking for a property so we should figure out what it
    wants and try and deliver. */
 static GVariant *
@@ -1053,7 +1109,8 @@
 	if (priv->watcher_proxy == NULL) {
 		/* Build Watcher Proxy */
 		g_dbus_proxy_new(priv->connection,
-		                 G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES | G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS, /* We don't use these, don't bother with them */
+		                 G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES|
+		                 G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS, /* We don't use these, don't bother with them */
 		                 watcher_interface_info,
 		                 NOTIFICATION_WATCHER_DBUS_ADDR,
 		                 NOTIFICATION_WATCHER_DBUS_OBJ,
@@ -1315,6 +1372,8 @@
 	status_icon_changes(self, icon);
 
 	g_signal_connect(G_OBJECT(icon), "activate", G_CALLBACK(status_icon_activate), self);
+	g_signal_connect(G_OBJECT(icon), "popup-menu", G_CALLBACK(status_icon_menu_activate), self);
+	g_signal_connect(G_OBJECT(icon), "scroll-event", G_CALLBACK(scroll_event_wrapper), self);
 
 	return icon;
 }
@@ -1327,6 +1386,18 @@
 	return status_icon_changes(self, data);
 }
 
+/* A wrapper for redirecting the scroll events to the app-indicator from status
+   icon widget. */
+static gboolean
+scroll_event_wrapper(GtkWidget *status_icon, GdkEventScroll *event, gpointer data)
+{
+	g_return_val_if_fail(IS_APP_INDICATOR(data), FALSE);
+	AppIndicator * app = APP_INDICATOR(data);
+	g_signal_emit(app, signals[SCROLL_EVENT], 0, 1, event->direction);
+
+	return FALSE;
+}
+
 /* This tracks changes to either the status or the icons
    that are associated with the app indicator */
 static void
@@ -1388,6 +1459,14 @@
 	return;
 }
 
+/* Handles the right-click action by the status icon by showing
+   the menu in a popup. */
+static void
+status_icon_menu_activate (GtkStatusIcon *status_icon, guint button, guint activate_time, gpointer user_data)
+{
+	status_icon_activate(status_icon, user_data);
+}
+
 /* Removes the status icon as the application indicator area
    is now up and running again. */
 static void
@@ -1395,6 +1474,7 @@
 {
 	g_signal_handlers_disconnect_by_func(G_OBJECT(self), status_icon_status_wrapper, status_icon);
 	g_signal_handlers_disconnect_by_func(G_OBJECT(self), status_icon_changes, status_icon);
+	g_signal_handlers_disconnect_by_func(G_OBJECT(self), scroll_event_wrapper, status_icon);
 	gtk_status_icon_set_visible(status_icon, FALSE);
 	g_object_unref(G_OBJECT(status_icon));
 	return;

=== modified file 'src/app-indicator.h'
--- src/app-indicator.h	2010-11-08 16:25:40 +0000
+++ src/app-indicator.h	2011-02-03 11:51:50 +0000
@@ -107,12 +107,18 @@
 
 	String identifier for the #AppIndicator::new-icon-theme-path signal.
 */
+/**
+	APP_INDICATOR_SIGNAL_SCROLL_EVENT:
+
+	String identifier for the #AppIndicator::scroll-event signal.
+*/
 #define APP_INDICATOR_SIGNAL_NEW_ICON            "new-icon"
 #define APP_INDICATOR_SIGNAL_NEW_ATTENTION_ICON  "new-attention-icon"
 #define APP_INDICATOR_SIGNAL_NEW_STATUS          "new-status"
 #define APP_INDICATOR_SIGNAL_NEW_LABEL           "new-label"
 #define APP_INDICATOR_SIGNAL_CONNECTION_CHANGED  "connection-changed"
 #define APP_INDICATOR_SIGNAL_NEW_ICON_THEME_PATH "new-icon-theme-path"
+#define APP_INDICATOR_SIGNAL_SCROLL_EVENT        "scroll-event"
 
 /**
 	AppIndicatorCategory:
@@ -163,7 +169,7 @@
 	@new_icon_theme_path: Slot for #AppIndicator::new-icon-theme-path
 	@new_label: Slot for #AppIndicator::new-label.
 	@connection_changed: Slot for #AppIndicator::connection-changed.
-	@app_indicator_reserved_sw: Reserved for future use.
+	@scroll-event: Slot for #AppIndicator::scroll-event
 	@app_indicator_reserved_ats: Reserved for future use.
 	@fallback: Function that gets called to make a #GtkStatusIcon when
 		there is no Application Indicator area available.
@@ -203,7 +209,12 @@
 	void (* connection_changed)     (AppIndicator * indicator,
 	                                 gboolean          connected,
 	                                 gpointer          user_data);
-	void (*app_indicator_reserved_sw)(void);
+
+	void (* scroll_event)           (AppIndicator * indicator,
+	                                 gint                  delta,
+	                                 guint             direction,
+	                                 gpointer          user_data);
+
 	void (*app_indicator_reserved_ats)(void);
 
 	/* Overridable Functions */

=== modified file 'src/application-service-marshal.list'
--- src/application-service-marshal.list	2010-08-18 19:02:47 +0000
+++ src/application-service-marshal.list	2011-02-03 11:51:50 +0000
@@ -21,3 +21,4 @@
 VOID: INT, STRING
 VOID: STRING, STRING
 VOID: BOOL, STRING, OBJECT
+VOID: INT, UINT

=== modified file 'src/notification-item.xml'
--- src/notification-item.xml	2010-08-11 20:55:25 +0000
+++ src/notification-item.xml	2011-02-03 11:51:50 +0000
@@ -17,7 +17,10 @@
 		<property name="XAyatanaOrderingIndex" type="u" access="read" />
 
 <!-- Methods -->
-		<!-- None currently -->
+		<method name="Scroll">
+			<arg type="i" name="delta" direction="in" />
+			<arg type="s" name="orientation" direction="in" />
+		</method>
 
 <!-- Signals -->
 		<signal name="NewIcon">
@@ -34,6 +37,5 @@
 			<arg type="s" name="label" direction="out" />
 			<arg type="s" name="guide" direction="out" />
 		</signal>
-
 	</interface>
 </node>