From 701a0dabd0a911ced7907da30eec851723f20f5e Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 25 Aug 2020 16:32:49 +0100 Subject: [PATCH 1/8] a11y: Different value types cannot be equal Bail out early, instead of going deep into the GtkAccessibleValue type equal() implementation, where we expect both accessible values to have the same type. --- gtk/gtkaccessiblevalue.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gtk/gtkaccessiblevalue.c b/gtk/gtkaccessiblevalue.c index 5ea1db0488..2cefba0bc4 100644 --- a/gtk/gtkaccessiblevalue.c +++ b/gtk/gtkaccessiblevalue.c @@ -185,6 +185,9 @@ gtk_accessible_value_equal (const GtkAccessibleValue *value_a, if (value_a == NULL || value_b == NULL) return FALSE; + if (value_a->value_class != value_b->value_class) + return FALSE; + if (value_a->value_class->equal == NULL) return FALSE; From 911a71c7052312f7990c71e84a1f688c03b9c174 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Wed, 5 Aug 2020 15:03:00 +0100 Subject: [PATCH 2/8] a11y: Allow bulk attribute update with the GValue API Like we do for the varargs API. --- gtk/gtkaccessible.c | 156 ++++++++++++++++++++++++-------------------- gtk/gtkaccessible.h | 15 +++-- 2 files changed, 96 insertions(+), 75 deletions(-) diff --git a/gtk/gtkaccessible.c b/gtk/gtkaccessible.c index f8dc8640af..b136f1a946 100644 --- a/gtk/gtkaccessible.c +++ b/gtk/gtkaccessible.c @@ -174,10 +174,11 @@ out: /** * gtk_accessible_update_state_value: * @self: a #GtkAccessible - * @state: a #GtkAccessibleState - * @value: a #GValue with the value for @state + * @n_states: the number of accessible states to set + * @states: (array length=n_states): an array of #GtkAccessibleState + * @values: (array length=n_states): an array of #GValues, one for each state * - * Updates an accessible state. + * Updates an array of accessible states. * * This function should be called by #GtkWidget types whenever an accessible * state change must be communicated to assistive technologies. @@ -186,35 +187,40 @@ out: */ void gtk_accessible_update_state_value (GtkAccessible *self, - GtkAccessibleState state, - const GValue *value) + int n_states, + GtkAccessibleState states[], + const GValue values[]) { - GtkATContext *context; - g_return_if_fail (GTK_IS_ACCESSIBLE (self)); + g_return_if_fail (n_states > 0); - context = gtk_accessible_get_at_context (self); + GtkATContext *context = gtk_accessible_get_at_context (self); if (context == NULL) return; - GError *error = NULL; - GtkAccessibleValue *real_value = - gtk_accessible_value_collect_for_state_value (state, value, &error); - - if (error != NULL) + for (int i = 0; i < n_states; i++) { - g_critical ("Unable to collect the value for state “%s”: %s", - gtk_accessible_state_get_attribute_name (state), - error->message); - g_error_free (error); - return; + GtkAccessibleState state = states[i]; + const GValue *value = &(values[i]); + GError *error = NULL; + GtkAccessibleValue *real_value = + gtk_accessible_value_collect_for_state_value (state, value, &error); + + if (error != NULL) + { + g_critical ("Unable to collect the value for state “%s”: %s", + gtk_accessible_state_get_attribute_name (state), + error->message); + g_error_free (error); + break; + } + + gtk_at_context_set_accessible_state (context, state, real_value); + + if (real_value != NULL) + gtk_accessible_value_unref (real_value); } - gtk_at_context_set_accessible_state (context, state, real_value); - - if (real_value != NULL) - gtk_accessible_value_unref (real_value); - gtk_at_context_update (context); } @@ -312,10 +318,11 @@ out: /** * gtk_accessible_update_property_value: * @self: a #GtkAccessible - * @property: a #GtkAccessibleProperty - * @value: a #GValue with the value for @property + * @n_properties: the number of accessible properties to set + * @properties: (array length=n_properties): an array of #GtkAccessibleProperty + * @values: (array length=n_properties): an array of #GValues, one for each property * - * Updates an accessible property. + * Updates an array of accessible properties. * * This function should be called by #GtkWidget types whenever an accessible * property change must be communicated to assistive technologies. @@ -324,35 +331,40 @@ out: */ void gtk_accessible_update_property_value (GtkAccessible *self, - GtkAccessibleProperty property, - const GValue *value) + int n_properties, + GtkAccessibleProperty properties[], + const GValue values[]) { - GtkATContext *context; - g_return_if_fail (GTK_IS_ACCESSIBLE (self)); + g_return_if_fail (n_properties > 0); - context = gtk_accessible_get_at_context (self); + GtkATContext *context = gtk_accessible_get_at_context (self); if (context == NULL) return; - GError *error = NULL; - GtkAccessibleValue *real_value = - gtk_accessible_value_collect_for_property_value (property, value, &error); - - if (error != NULL) + for (int i = 0; i < n_properties; i++) { - g_critical ("Unable to collect the value for property “%s”: %s", - gtk_accessible_property_get_attribute_name (property), - error->message); - g_error_free (error); - return; + GtkAccessibleProperty property = properties[i]; + const GValue *value = &(values[i]); + GError *error = NULL; + GtkAccessibleValue *real_value = + gtk_accessible_value_collect_for_property_value (property, value, &error); + + if (error != NULL) + { + g_critical ("Unable to collect the value for property “%s”: %s", + gtk_accessible_property_get_attribute_name (property), + error->message); + g_error_free (error); + break; + } + + gtk_at_context_set_accessible_property (context, property, real_value); + + if (real_value != NULL) + gtk_accessible_value_unref (real_value); } - gtk_at_context_set_accessible_property (context, property, real_value); - - if (real_value != NULL) - gtk_accessible_value_unref (real_value); - gtk_at_context_update (context); } @@ -441,10 +453,11 @@ out: /** * gtk_accessible_update_relation_value: * @self: a #GtkAccessible - * @relation: a #GtkAccessibleRelation - * @value: a #GValue with the value for @relation + * @n_relations: the number of accessible relations to set + * @relations: (array length=n_relations): an array of #GtkAccessibleRelation + * @values: (array length=n_relations): an array of #GValues, one for each relation * - * Updates an accessible relation. + * Updates an array of accessible relations. * * This function should be called by #GtkWidget types whenever an accessible * relation change must be communicated to assistive technologies. @@ -453,35 +466,40 @@ out: */ void gtk_accessible_update_relation_value (GtkAccessible *self, - GtkAccessibleRelation relation, - const GValue *value) + int n_relations, + GtkAccessibleRelation relations[], + const GValue values[]) { - GtkATContext *context; - g_return_if_fail (GTK_IS_ACCESSIBLE (self)); + g_return_if_fail (n_relations > 0); - context = gtk_accessible_get_at_context (self); + GtkATContext *context = gtk_accessible_get_at_context (self); if (context == NULL) return; - GError *error = NULL; - GtkAccessibleValue *real_value = - gtk_accessible_value_collect_for_relation_value (relation, value, &error); - - if (error != NULL) + for (int i = 0; i < n_relations; i++) { - g_critical ("Unable to collect the value for relation “%s”: %s", - gtk_accessible_relation_get_attribute_name (relation), - error->message); - g_error_free (error); - return; + GtkAccessibleRelation relation = relations[i]; + const GValue *value = &(values[i]); + GError *error = NULL; + GtkAccessibleValue *real_value = + gtk_accessible_value_collect_for_relation_value (relation, value, &error); + + if (error != NULL) + { + g_critical ("Unable to collect the value for relation “%s”: %s", + gtk_accessible_relation_get_attribute_name (relation), + error->message); + g_error_free (error); + break; + } + + gtk_at_context_set_accessible_relation (context, relation, real_value); + + if (real_value != NULL) + gtk_accessible_value_unref (real_value); } - gtk_at_context_set_accessible_relation (context, relation, real_value); - - if (real_value != NULL) - gtk_accessible_value_unref (real_value); - gtk_at_context_update (context); } diff --git a/gtk/gtkaccessible.h b/gtk/gtkaccessible.h index 0aec20974f..c8b00cd35b 100644 --- a/gtk/gtkaccessible.h +++ b/gtk/gtkaccessible.h @@ -52,16 +52,19 @@ void gtk_accessible_update_relation (GtkAccessible ...); GDK_AVAILABLE_IN_ALL void gtk_accessible_update_state_value (GtkAccessible *self, - GtkAccessibleState state, - const GValue *value); + int n_states, + GtkAccessibleState states[], + const GValue values[]); GDK_AVAILABLE_IN_ALL void gtk_accessible_update_property_value (GtkAccessible *self, - GtkAccessibleProperty property, - const GValue *value); + int n_properties, + GtkAccessibleProperty properties[], + const GValue values[]); GDK_AVAILABLE_IN_ALL void gtk_accessible_update_relation_value (GtkAccessible *self, - GtkAccessibleRelation relation, - const GValue *value); + int n_relations, + GtkAccessibleRelation relations[], + const GValue values[]); GDK_AVAILABLE_IN_ALL void gtk_accessible_reset_state (GtkAccessible *self, From 1338dcddcb3a7174049da75c030a79cc0f50905d Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Wed, 5 Aug 2020 15:05:06 +0100 Subject: [PATCH 3/8] a11y: GtkATContext.update_state() was renamed to update() --- gtk/gtkatcontext.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gtk/gtkatcontext.c b/gtk/gtkatcontext.c index c18a6cff37..9cdbd0e9f3 100644 --- a/gtk/gtkatcontext.c +++ b/gtk/gtkatcontext.c @@ -447,7 +447,7 @@ gtk_at_context_update (GtkATContext *self) * * If @value is %NULL, the state is unset. * - * This function will accumulate state changes until gtk_at_context_update_state() + * This function will accumulate state changes until gtk_at_context_update() * is called. */ void @@ -509,7 +509,7 @@ gtk_at_context_get_accessible_state (GtkATContext *self, * * If @value is %NULL, the property is unset. * - * This function will accumulate property changes until gtk_at_context_update_state() + * This function will accumulate property changes until gtk_at_context_update() * is called. */ void @@ -571,7 +571,7 @@ gtk_at_context_get_accessible_property (GtkATContext *self, * * If @value is %NULL, the relation is unset. * - * This function will accumulate relation changes until gtk_at_context_update_state() + * This function will accumulate relation changes until gtk_at_context_update() * is called. */ void From 32a1cd13c8abd3015af5923344d463eee6a6d3a5 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Wed, 5 Aug 2020 18:04:34 +0100 Subject: [PATCH 4/8] a11y: Notify callers when an attributes set changes We can use that information inside the ATContext. --- gtk/gtkaccessibleattributeset.c | 37 +++++++++++++++++++++++--- gtk/gtkaccessibleattributesetprivate.h | 4 +-- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/gtk/gtkaccessibleattributeset.c b/gtk/gtkaccessibleattributeset.c index abdc0950ed..f49a9cf911 100644 --- a/gtk/gtkaccessibleattributeset.c +++ b/gtk/gtkaccessibleattributeset.c @@ -114,13 +114,26 @@ gtk_accessible_attribute_set_unref (GtkAccessibleAttributeSet *self) * * If you want to remove @attribute from the set, use gtk_accessible_attribute_set_remove() * instead. + * + * Returns: %TRUE if the set was modified, and %FALSE otherwise */ -void +gboolean gtk_accessible_attribute_set_add (GtkAccessibleAttributeSet *self, int attribute, GtkAccessibleValue *value) { - g_return_if_fail (attribute >= 0 && attribute < self->n_attributes); + g_return_val_if_fail (attribute >= 0 && attribute < self->n_attributes, FALSE); + + if (value != NULL) + { + if (gtk_accessible_value_equal (value, self->attribute_values[attribute])) + return FALSE; + } + else + { + if (!_gtk_bitmask_get (self->attributes_set, attribute)) + return FALSE; + } g_clear_pointer (&(self->attribute_values[attribute]), gtk_accessible_value_unref); @@ -130,18 +143,34 @@ gtk_accessible_attribute_set_add (GtkAccessibleAttributeSet *self, self->attribute_values[attribute] = (* self->default_func) (attribute); self->attributes_set = _gtk_bitmask_set (self->attributes_set, attribute, TRUE); + + return TRUE; } -void +/*< private > + * gtk_accessible_attribute_set_remove: + * @self: a #GtkAccessibleAttributeSet + * @attribute: the attribute to be removed + * + * Resets the @attribute in the given #GtkAccessibleAttributeSet to its default value. + * + * Returns: %TRUE if the set was modified, and %FALSE otherwise + */ +gboolean gtk_accessible_attribute_set_remove (GtkAccessibleAttributeSet *self, int attribute) { - g_return_if_fail (attribute >= 0 && attribute < self->n_attributes); + g_return_val_if_fail (attribute >= 0 && attribute < self->n_attributes, FALSE); + + if (!_gtk_bitmask_get (self->attributes_set, attribute)) + return FALSE; g_clear_pointer (&(self->attribute_values[attribute]), gtk_accessible_value_unref); self->attribute_values[attribute] = (* self->default_func) (attribute); self->attributes_set = _gtk_bitmask_set (self->attributes_set, attribute, FALSE); + + return TRUE; } gboolean diff --git a/gtk/gtkaccessibleattributesetprivate.h b/gtk/gtkaccessibleattributesetprivate.h index 57a1e63e35..83694c7ede 100644 --- a/gtk/gtkaccessibleattributesetprivate.h +++ b/gtk/gtkaccessibleattributesetprivate.h @@ -37,10 +37,10 @@ void gtk_accessible_attribute_set_unref gsize gtk_accessible_attribute_set_get_length (GtkAccessibleAttributeSet *self); -void gtk_accessible_attribute_set_add (GtkAccessibleAttributeSet *self, +gboolean gtk_accessible_attribute_set_add (GtkAccessibleAttributeSet *self, int attribute, GtkAccessibleValue *value); -void gtk_accessible_attribute_set_remove (GtkAccessibleAttributeSet *self, +gboolean gtk_accessible_attribute_set_remove (GtkAccessibleAttributeSet *self, int state); gboolean gtk_accessible_attribute_set_contains (GtkAccessibleAttributeSet *self, int state); From 797b3bd1b11167b100add40e5f5b5f5815ab85f8 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Wed, 5 Aug 2020 18:05:46 +0100 Subject: [PATCH 5/8] a11y: Do not notify of empty state changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the ATContext state hasn't changed—for instance, if the accessible attributes have been set to their default value, or have been set to the same value—do not emit an accessible state change. State changes can be arbitrarily expensive, so we want to ensure that they are meaningful. --- gtk/gtkatcontext.c | 37 +++++++++++++++++++++++++++++++------ gtk/gtkatcontextprivate.h | 4 ++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/gtk/gtkatcontext.c b/gtk/gtkatcontext.c index 9cdbd0e9f3..e1e2edd599 100644 --- a/gtk/gtkatcontext.c +++ b/gtk/gtkatcontext.c @@ -425,6 +425,12 @@ gtk_at_context_update (GtkATContext *self) { g_return_if_fail (GTK_IS_AT_CONTEXT (self)); + /* There's no point in notifying of state changes if there weren't any */ + if (self->updated_properties == 0 && + self->updated_relations == 0 && + self->updated_states == 0) + return; + GtkAccessibleStateChange changed_states = gtk_accessible_attribute_set_get_changed (self->states); GtkAccessiblePropertyChange changed_properties = @@ -435,6 +441,10 @@ gtk_at_context_update (GtkATContext *self) g_signal_emit (self, obj_signals[STATE_CHANGE], 0, changed_states, changed_properties, changed_relations, self->states, self->properties, self->relations); + + self->updated_properties = 0; + self->updated_relations = 0; + self->updated_states = 0; } /*< private > @@ -457,10 +467,15 @@ gtk_at_context_set_accessible_state (GtkATContext *self, { g_return_if_fail (GTK_IS_AT_CONTEXT (self)); + gboolean res = FALSE; + if (value != NULL) - gtk_accessible_attribute_set_add (self->states, state, value); + res = gtk_accessible_attribute_set_add (self->states, state, value); else - gtk_accessible_attribute_set_remove (self->states, state); + res = gtk_accessible_attribute_set_remove (self->states, state); + + if (res) + self->updated_states |= (1 << state); } /*< private > @@ -519,10 +534,15 @@ gtk_at_context_set_accessible_property (GtkATContext *self, { g_return_if_fail (GTK_IS_AT_CONTEXT (self)); + gboolean res = FALSE; + if (value != NULL) - gtk_accessible_attribute_set_add (self->properties, property, value); + res = gtk_accessible_attribute_set_add (self->properties, property, value); else - gtk_accessible_attribute_set_remove (self->properties, property); + res = gtk_accessible_attribute_set_remove (self->properties, property); + + if (res) + self->updated_properties |= (1 << property); } /*< private > @@ -581,10 +601,15 @@ gtk_at_context_set_accessible_relation (GtkATContext *self, { g_return_if_fail (GTK_IS_AT_CONTEXT (self)); + gboolean res = FALSE; + if (value != NULL) - gtk_accessible_attribute_set_add (self->relations, relation, value); + res = gtk_accessible_attribute_set_add (self->relations, relation, value); else - gtk_accessible_attribute_set_remove (self->relations, relation); + res = gtk_accessible_attribute_set_remove (self->relations, relation); + + if (res) + self->updated_relations |= (1 << relation); } /*< private > diff --git a/gtk/gtkatcontextprivate.h b/gtk/gtkatcontextprivate.h index 2ac85bde2d..2b916187fd 100644 --- a/gtk/gtkatcontextprivate.h +++ b/gtk/gtkatcontextprivate.h @@ -90,6 +90,10 @@ struct _GtkATContext GtkAccessibleAttributeSet *states; GtkAccessibleAttributeSet *properties; GtkAccessibleAttributeSet *relations; + + GtkAccessibleStateChange updated_states; + GtkAccessiblePropertyChange updated_properties; + GtkAccessibleRelationChange updated_relations; }; struct _GtkATContextClass From fd568e63c2a22af24f7b963e87c4e775434551e4 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 25 Aug 2020 11:30:46 +0100 Subject: [PATCH 6/8] Properly document GtkAccessible:accessible-role Use a gtk-doc stanza, instead of the GParamSpec strings. --- gtk/gtkaccessible.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gtk/gtkaccessible.c b/gtk/gtkaccessible.c index b136f1a946..79b56f6eeb 100644 --- a/gtk/gtkaccessible.c +++ b/gtk/gtkaccessible.c @@ -55,6 +55,13 @@ G_DEFINE_INTERFACE (GtkAccessible, gtk_accessible, G_TYPE_OBJECT) static void gtk_accessible_default_init (GtkAccessibleInterface *iface) { + /** + * GtkAccessible:accessible-role: + * + * The accessible role of the given #GtkAccessible implementation. + * + * The accessible role cannot be changed once set. + */ GParamSpec *pspec = g_param_spec_enum ("accessible-role", "Accessible Role", From 256c9c9873b1e12ca76b24d35361b87f61558724 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 25 Aug 2020 11:31:26 +0100 Subject: [PATCH 7/8] Annotate GValue-variants methods of GtkAccessible The variadic arguments methods cannot be used by language bindings, which means we can let them use their names when calling the GValue-based methods. --- gtk/gtkaccessible.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gtk/gtkaccessible.c b/gtk/gtkaccessible.c index 79b56f6eeb..0c9a399ef0 100644 --- a/gtk/gtkaccessible.c +++ b/gtk/gtkaccessible.c @@ -179,7 +179,7 @@ out: } /** - * gtk_accessible_update_state_value: + * gtk_accessible_update_state_value: (rename-to gtk_accessible_update_state) * @self: a #GtkAccessible * @n_states: the number of accessible states to set * @states: (array length=n_states): an array of #GtkAccessibleState @@ -323,7 +323,7 @@ out: } /** - * gtk_accessible_update_property_value: + * gtk_accessible_update_property_value: (rename-to gtk_accessible_update_property) * @self: a #GtkAccessible * @n_properties: the number of accessible properties to set * @properties: (array length=n_properties): an array of #GtkAccessibleProperty @@ -458,7 +458,7 @@ out: } /** - * gtk_accessible_update_relation_value: + * gtk_accessible_update_relation_value: (rename-to gtk_accessible_update_relation) * @self: a #GtkAccessible * @n_relations: the number of accessible relations to set * @relations: (array length=n_relations): an array of #GtkAccessibleRelation From d58136e23d71236d332eac07986922c96139c744 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 25 Aug 2020 16:34:04 +0100 Subject: [PATCH 8/8] a11y: Simplify the ATContext::state-change signal We cannot pass all the data we pass to the virtual function, because the types are private, but the class and the signal are public API. The signal is just a notification, so we can decouple the virtual function (which stays the same, for internal types that implement the ATContext API contract) from the signal. --- gtk/gtkatcontext.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/gtk/gtkatcontext.c b/gtk/gtkatcontext.c index e1e2edd599..6686e3f9a9 100644 --- a/gtk/gtkatcontext.c +++ b/gtk/gtkatcontext.c @@ -175,12 +175,6 @@ gtk_at_context_class_init (GtkATContextClass *klass) /** * GtkATContext::state-change: * @self: the #GtkATContext - * @changed_states: flags for the changed states - * @changed_properties: flags for the changed properties - * @changed_relations: flags for the changed relations - * @states: the new states - * @properties: the new properties - * @relations: the new relations * * Emitted when the attributes of the accessible for the * #GtkATContext instance change. @@ -189,12 +183,10 @@ gtk_at_context_class_init (GtkATContextClass *klass) g_signal_new ("state-change", G_TYPE_FROM_CLASS (gobject_class), G_SIGNAL_RUN_FIRST, - G_STRUCT_OFFSET (GtkATContextClass, state_change), + 0, NULL, NULL, NULL, - G_TYPE_NONE, 6, - G_TYPE_UINT, G_TYPE_UINT, G_TYPE_UINT, - G_TYPE_POINTER, G_TYPE_POINTER, G_TYPE_POINTER); + G_TYPE_NONE, 0); g_object_class_install_properties (gobject_class, N_PROPS, obj_props); } @@ -438,9 +430,10 @@ gtk_at_context_update (GtkATContext *self) GtkAccessibleRelationChange changed_relations = gtk_accessible_attribute_set_get_changed (self->relations); - g_signal_emit (self, obj_signals[STATE_CHANGE], 0, - changed_states, changed_properties, changed_relations, - self->states, self->properties, self->relations); + GTK_AT_CONTEXT_GET_CLASS (self)->state_change (self, + changed_states, changed_properties, changed_relations, + self->states, self->properties, self->relations); + g_signal_emit (self, obj_signals[STATE_CHANGE], 0); self->updated_properties = 0; self->updated_relations = 0;