Merge lp:~rodrigo-moya/unity/fix-731651 into lp:unity

[Rodrigo Moya]

[Alejandro Piñeiro (apinheiro)]

20 + g_signal_emit_by_name (ATK_OBJECT (pia), "children-changed",
40 + g_signal_emit_by_name (ATK_OBJECT (pia), "children-changed",

That signal supports two details [1] "add" and "remove", so it would be good (and easy) to use them here.

60 + if (GTK_IS_LABEL (entry->label))
61 + atk_object_set_name (accessible, gtk_label_get_text (GTK_LABEL (entry->label)));
62 + else
63 + atk_object_set_name (accessible, entry->accessible_desc);
64 + atk_object_set_description (accessible, entry->accessible_desc);

As we chatted on IRC, with this code in some cases you will have a description equal to the name. And in the same way, atk_object_get_name is reimplemented on gtkimage (the other case of entry). So I think that this should be:

    if (is_label) => name ==> gtk_label_get_text
    if (is_image) => name ==> atk_object_get_name (from gail image)
    and use entry->accessible_desc just for the description in both cases (after all _desc seems to suggest a description, not a name)

167 atk_object_set_name (accessible, piea->priv->entry->accessible_desc);
168 }
169 }
170 +
171 + atk_object_set_description (accessible, piea->priv->entry->accessible_desc);

The previous comment also applies here.

[1] http://library.gnome.org/devel/atk/stable/AtkObject.html#AtkObject-children-changed

review: Needs Fixing

[Rodrigo Moya (rodrigo-moya)]

> 20 + g_signal_emit_by_name (ATK_OBJECT (pia), "children-changed",
> 40 + g_signal_emit_by_name (ATK_OBJECT (pia), "children-changed",
>
> That signal supports two details [1] "add" and "remove", so it would be good
> (and easy) to use them here.
>
this is fixed in my other branch: https://code.launchpad.net/~rodrigo-moya/unity/fix-732049/+merge/53232

> 60 + if (GTK_IS_LABEL (entry->label))
> 61 + atk_object_set_name (accessible, gtk_label_get_text (GTK_LABEL
> (entry->label)));
> 62 + else
> 63 + atk_object_set_name (accessible, entry->accessible_desc);
> 64 + atk_object_set_description (accessible, entry->accessible_desc);
>
> As we chatted on IRC, with this code in some cases you will have a description
> equal to the name. And in the same way, atk_object_get_name is reimplemented
> on gtkimage (the other case of entry). So I think that this should be:
>
> if (is_label) => name ==> gtk_label_get_text
> if (is_image) => name ==> atk_object_get_name (from gail image)
> and use entry->accessible_desc just for the description in both cases
> (after all _desc seems to suggest a description, not a name)
>
> 167 atk_object_set_name (accessible, piea->priv->entry->accessible_desc);
> 168 }
> 169 }
> 170 +
> 171 + atk_object_set_description (accessible,
> piea->priv->entry->accessible_desc);
>
> The previous comment also applies here.
>
fixed

[Alejandro Piñeiro (apinheiro)]

Seems ok to me

review: Approve

[Rodrigo Moya (rodrigo-moya)]

Neil gave his +1 on IRC