Merge lp:~themuso/libindicator/accessible-name into lp:libindicator/0.4

[Luke Yelavich]

Accessible name support. Still not sure whether the documentatino formatting is right, or whether it needs more elaboration.

[Ted Gould (ted)]

Probably should make the function prototype and struct property const so that users know they shouldn't free them. Otherwise looks good.

review: Approve

[Rodrigo Moya (rodrigo-moya)]

Luke, Ted told me you didn't want to merge this in before you got other pieces. So, what is blocking this? I have a branch (https://code.launchpad.net/~rodrigo-moya/unity/indicators-a11y/+merge/48943) that is missing something like this to provide a good description for the indicator objects.

Some comments though:

The get_accessible_name seems to be related to the IndicatorObject, not the IndicatorObjectEntry's, so why do you have this:

62 struct _IndicatorObjectEntry {
63 GtkLabel * label;
64 GtkImage * image;
65 GtkMenu * menu;
66 + gchar * accessible_name;
67 };

wouldn't it be better to just have a indicator_object_get_accessible_name function for the IndicatorObject? Also, maybe just call it get_description, instead of get_accessible_name?

[Ted Gould (ted)]

On Tue, 2011-02-08 at 17:26 +0000, Rodrigo Moya wrote:
> The get_accessible_name seems to be related to the IndicatorObject,
> not the IndicatorObjectEntry's, so why do you have this:

It needs to be related to the entires as those each represent an item on
the panel. The object represents a collection of items on the panel.
So for things like the sound menu there is a 1:1 ratio of objects to
entries, but for application indicators it's 1:n.

[Luke Yelavich (themuso)]

On Wed, Feb 09, 2011 at 04:26:52AM EST, Rodrigo Moya wrote:
> wouldn't it be better to just have a indicator_object_get_accessible_name function for the IndicatorObject? Also, maybe just call it get_description, instead of get_accessible_name?

Yes, I have actually renamed it in my code, now called accessible_desc, and related changes in indicator-application/libappindicator.

In terms of readiness, I was almost there, till I realised that we will likely have to add another signal, because even if an indicator updates the information that get_accessible_desc harvists, the unity panel and indicator applet still won't know something has changed, short of being signaled. Using such a signal is not a problem for indicator-application, because indicator-application sends an indicator entry pointer through to the displayer, i.e the applet/unity panel.

My problem that I am running into with this new signal is with the system indicators, as they don't deal with constructing their own IndicatorObjectEntry structure, i.e they use the default get_entries method from indicator-object, get_entries_default(Indicator Object * io). Even if another method is added to indicator-object allowing this to be worked around somehow, some indicators like indicator-sound are modular to the point where its not possible to call the accessibility description update signal from particular objects that make up the indicator, short of extending method calls to pass the indicator object pointer through to where its needed etc.

I hope what I have written above makes sense, I know what i mean in my head of course, but explaining it is something else :) so let me know if you need me to clarrify anything above. Of course I could extend the indicators as necessary to make sure the indicator object pointer is where it is needed, but I don't want to do that until I know there is no other way of doing it. If either of you have any suggestions, I'm all ears.

Luke

[lp:~themuso/libindicator/accessible-name]

396. By Luke Yelavich on 2011-02-01

Merge from trunk

397. By Luke Yelavich on 2011-02-01

use const gchar for variable and prototype

398. By Luke Yelavich on 2011-02-08

* accessible_name -> accessible_desc to better reflect the use of the content.
* Add accessible-desc-update signal so that indicators can tell
  indicator-applet/unity that the accessible description has changed

399. By Luke Yelavich on 2011-02-08

accessible_name -> accessible_desc in tests as well

=== modified file 'libindicator/indicator-object.c'
--- libindicator/indicator-object.c	2011-01-26 23:12:42 +0000
+++ libindicator/indicator-object.c	2011-02-14 02:38:08 +0000
@@ -61,6 +61,7 @@
 	SCROLL_ENTRY,
 	MENU_SHOW,
 	SHOW_NOW_CHANGED,
+	ACCESSIBLE_DESC_UPDATE,
 	LAST_SIGNAL
 };
 
@@ -91,6 +92,7 @@
 	klass->get_label =  NULL;
 	klass->get_menu =   NULL;
 	klass->get_image =  NULL;
+	klass->get_accessible_desc = NULL;
 
 	klass->get_entries = get_entries_default;
 	klass->get_location = NULL;
@@ -221,6 +223,24 @@
 	                                          _indicator_object_marshal_VOID__POINTER_BOOLEAN,
 	                                          G_TYPE_NONE, 2, G_TYPE_POINTER, G_TYPE_BOOLEAN);
 
+	/**
+		IndicatorObject::accessible-desc-update::
+		@arg0: The #IndicatorObject object
+		@arg1: A pointer to the #IndicatorObjectEntry whos
+			accessible description has been updated.
+
+		Signaled when an indicator's accessible description
+		has been updated, so that the displayer of the
+		indicator can fetch the new description.
+	*/
+	signals[ACCESSIBLE_DESC_UPDATE] = g_signal_new (INDICATOR_OBJECT_SIGNAL_ACCESSIBLE_DESC_UPDATE,
+	                                     G_TYPE_FROM_CLASS(klass),
+	                                     G_SIGNAL_RUN_LAST,
+	                                     G_STRUCT_OFFSET (IndicatorObjectClass, accessible_desc_update),
+	                                     NULL, NULL,
+	                                     g_cclosure_marshal_VOID__POINTER,
+	                                     G_TYPE_NONE, 1, G_TYPE_POINTER, G_TYPE_NONE);
+
 
 	return;
 }
@@ -236,6 +256,7 @@
 	self->priv->entry.menu = NULL;
 	self->priv->entry.label = NULL;
 	self->priv->entry.image = NULL;
+	self->priv->entry.accessible_desc = NULL;
 
 	self->priv->gotten_entries = FALSE;
 
@@ -416,6 +437,14 @@
 			return NULL;
 		}
 
+		if (class->get_accessible_desc) {
+			priv->entry.accessible_desc = class->get_accessible_desc(io);
+		}
+
+		if (priv->entry.accessible_desc == NULL) {
+			g_warning("IndicatorObject class does not have an accessible description.");
+		}
+
 		priv->gotten_entries = TRUE;
 	}
 
 
=== modified file 'libindicator/indicator-object.h'
--- libindicator/indicator-object.h	2011-01-27 11:24:00 +0000
+++ libindicator/indicator-object.h	2011-02-14 02:38:08 +0000
@@ -57,6 +57,8 @@
 #define INDICATOR_OBJECT_SIGNAL_MENU_SHOW_ID      (g_signal_lookup(INDICATOR_OBJECT_SIGNAL_MENU_SHOW, INDICATOR_OBJECT_TYPE))
 #define INDICATOR_OBJECT_SIGNAL_SHOW_NOW_CHANGED  "show-now-changed"
 #define INDICATOR_OBJECT_SIGNAL_SHOW_NOW_CHANGED_ID (g_signal_lookup(INDICATOR_OBJECT_SIGNAL_SHOW_NOW_CHANGED, INDICATOR_OBJECT_TYPE))
+#define INDICATOR_OBJECT_SIGNAL_ACCESSIBLE_DESC_UPDATE "accessible-desc-update"
+#define INDICATOR_OBJECT_SIGNAL_ACCESSIBLE_DESC_UPDATE_ID (g_signal_lookup(INDICATOR_OBJECT_SIGNAL_ACCESSIBLE_DESC_UPDATE, INDICATOR_OBJECT_TYPE))
 
 typedef struct _IndicatorObject        IndicatorObject;
 typedef struct _IndicatorObjectClass   IndicatorObjectClass;
@@ -75,6 +77,8 @@
 	@get_menu: Gets the image for this object.  Should be set
 		to #NULL if @get_entries is set.  Should NOT ref the
 		object.
+	@get_accessible_desc: Gets the accessible descriptionfor this
+		object.
 	@get_entries: Gets all of the entires for this object returning
 		a #GList of #IndicatorObjectEntries.  The list should be
 		under the ownership of the caller but the entires will
@@ -93,6 +97,7 @@
 	@entry_moved: Slot for #IndicatorObject::entry-moved
 	@menu_show: Slot for #IndicatorObject::menu-show
 	@show_now_changed: Slot for #IndicatorObject::show-now-changed
+	@accessible_desc_update: Slot for #IndicatorObject::accessible-desc-update
 */
 struct _IndicatorObjectClass {
 	GObjectClass parent_class;
@@ -101,6 +106,7 @@
 	GtkLabel * (*get_label) (IndicatorObject * io);
 	GtkImage * (*get_image) (IndicatorObject * io);
 	GtkMenu  * (*get_menu)  (IndicatorObject * io);
+	const gchar * (*get_accessible_desc) (IndicatorObject * io);
 
 	GList *    (*get_entries) (IndicatorObject * io);
 	guint      (*get_location) (IndicatorObject * io, IndicatorObjectEntry * entry);
@@ -117,6 +123,7 @@
 	void       (*menu_show)     (IndicatorObject * io, IndicatorObjectEntry * entry, guint timestamp, gpointer user_data);
 	void       (*show_now_changed) (IndicatorObject * io, IndicatorObjectEntry * entry, gboolean show_now_state, gpointer user_data);
 	void       (*scroll_entry)  (IndicatorObject * io, IndicatorObjectEntry * entry, gint delta, IndicatorScrollDirection direction);
+	void (*accessible_desc_update) (IndicatorObject * io, IndicatorObjectEntry * entry, gpointer user_data);
 
 	/* Reserved */
 	void       (*reserved1)     (void);
@@ -142,11 +149,14 @@
 	@label: The label to be shown on the panel
 	@image: The image to be shown on the panel
 	@menu: The menu to be added to the menubar
+	@accessible_desc: The accessible description
+		of the indicator
 */
 struct _IndicatorObjectEntry {
 	GtkLabel * label;
 	GtkImage * image;
 	GtkMenu  * menu;
+	const gchar * accessible_desc;
 };
 
 GType indicator_object_get_type (void);
 
=== modified file 'tests/dummy-indicator-null.c'
--- tests/dummy-indicator-null.c	2009-11-04 03:31:04 +0000
+++ tests/dummy-indicator-null.c	2011-02-14 02:38:08 +0000
@@ -46,6 +46,11 @@
 {
 	return NULL;
 }
+const gchar *
+get_accessible_desc (IndicatorObject * io)
+{
+	return NULL;
+}
 
 static void dummy_indicator_null_class_init (DummyIndicatorNullClass *klass);
 static void dummy_indicator_null_init       (DummyIndicatorNull *self);
@@ -67,6 +72,7 @@
 	io_class->get_label = get_label;
 	io_class->get_image = get_icon;
 	io_class->get_menu = get_menu;
+	io_class->get_accessible_desc = get_accessible_desc;
 
 	return;
 }
 
=== modified file 'tests/dummy-indicator-signaler.c'
--- tests/dummy-indicator-signaler.c	2010-01-15 22:11:42 +0000
+++ tests/dummy-indicator-signaler.c	2011-02-14 02:38:08 +0000
@@ -50,6 +50,12 @@
 	return main_menu;
 }
 
+const gchar *
+get_accessible_desc (IndicatorObject * io)
+{
+	return "Signaler Item";
+}
+
 static void dummy_indicator_signaler_class_init (DummyIndicatorSignalerClass *klass);
 static void dummy_indicator_signaler_init       (DummyIndicatorSignaler *self);
 static void dummy_indicator_signaler_dispose    (GObject *object);
@@ -70,6 +76,7 @@
 	io_class->get_label = get_label;
 	io_class->get_image = get_icon;
 	io_class->get_menu = get_menu;
+	io_class->get_accessible_desc = get_accessible_desc;
 
 	return;
 }
 
=== modified file 'tests/dummy-indicator-simple.c'
--- tests/dummy-indicator-simple.c	2009-11-04 03:31:04 +0000
+++ tests/dummy-indicator-simple.c	2011-02-14 02:38:08 +0000
@@ -50,6 +50,12 @@
 	return main_menu;
 }
 
+const gchar *
+get_accessible_desc (IndicatorObject * io)
+{
+	return "Simple Item";
+}
+
 static void dummy_indicator_simple_class_init (DummyIndicatorSimpleClass *klass);
 static void dummy_indicator_simple_init       (DummyIndicatorSimple *self);
 static void dummy_indicator_simple_dispose    (GObject *object);
@@ -70,6 +76,7 @@
 	io_class->get_label = get_label;
 	io_class->get_image = get_icon;
 	io_class->get_menu = get_menu;
+	io_class->get_accessible_desc = get_accessible_desc;
 
 	return;
 }