GNOME Bugzilla – Bug 626660
Additional accessibility support required on the dash items at overview
Last modified: 2011-03-08 22:49:24 UTC
Created attachment 167649 [details] [review] A silly test showing how to set the name on the object. BACKGROUND: During GUADEC, Dan Winship and me were testing the focus navigation patches on bug 621671. Orca speeched out the focus change, and a word that we didn't recognize. After review the orca debug file, I found out that Orca speeched "panel". After some triagging and talking with Joanmarie Diggs, it seems that if there are no a proper name (atk_object_get_name), orca uses as fallback the role of the object. SOLUTIONS I have attached a silly example of how we can set name on a object created on the javascript code. If you run that, instead of "panel" you hear "This is a test". So one idea would going further in this idea, and just get the inner label name, and set this as the accessible name. But this is a really ad-hoc solution. Next improvement would be instead of set the name, get the label and set the ATK_RELATION_LABELLED_BY relation between the object (just a StClickable) and the label. But, what I would like to do is a more general solution, similar to the one implemented on GailWidget. On ref_relation_set, gailwidget tries to find the labels associated to the widget, and if he found any, he sets automatically the relation. In this case it uses gtk_widget_list_mnemonic_labels to get the label. As there isn't any similar (AFAIK) here, in this case would be search in the childs for a label. If I finally implement that in this way, probably this bug could be moved to Cally itself. [1] http://library.gnome.org/devel/atk/stable/AtkRelation.html#AtkRelationType
Comment on attachment 167649 [details] [review] A silly test showing how to set the name on the object. Marking test patch as needs-work to get off patches-to-review list.
After updated patch on bug 626658 I will work again with this bug. Another good new is that patches on bug 621671 and bug 618885 were updated. But I would like to comment here what I did some months ago. As I explain on the description I tried to implement a generic solution on StWidget. I used as inspiration find_label on GailWidget. This method searches if a GtkWidget has a label, and then it sets the relation ATK_RELATION_LABELLED_BY between the widget and his label. But as I said on description, Gail uses gtk_widget_list_mnemonic_labels to do that, and clutter/st (AFAIK) misses this functionality. So I tried to just get all the objects with ATK_ROLE_LABEL in a container and set the relation. But this seems a overkill, and several extra relations were set, so Orca speeched out too many focus changes when you change the focus to a different container. So right now, I think that the best idea would be set the atk relation by hand on the js code. Anyway take into account that some times the mnemonics are added on gtk applications just in order to get them working to the accessibility tool (like recently on OCRFeeder). Anyway, any feedback is welcome.
Created attachment 173516 [details] [review] a11y support an appWellIcon This patch adds by hand the relation ATK_LABELLED_BY/ATK_LABEL_FOR between the application icon and his label. Note: right now it access directly to the label that is a private data of a internal component.
Created attachment 173517 [details] [review] a11y support an placeDisplayItem
Created attachment 173518 [details] [review] a11y support on each Dash section
The three previous patches add by hand the accessibility information required, so the environment described on the solution are solved. When you focus on any item orca speechs out the proper name and the role. Basically the first two patches get the accessible object of the item being focus and the internal label, set by hand the relation ATK_RELATION_LABELLED_BY/ATK_RELATION_LABEL_FOR, and sets a proper role. The last one sets the relation for the different sections on the dash, so when you move to other section orca speechs out this change. In order to test that I used the old (GUADEC time) patches on bug 621671. In the same way, as it is planed a relayout of the overview the items I added accessibility support would probably be removed. But anyway, with the current information in hand, it would be required patches similar to those ones, so I upload this in order to discuss them (should we place the atk code directly on the init?, any problem with the variable names?, etc) As I said before, this seems somewhat artificial, and the best solution could be automatize this on St itself (like Gtk), but right now St lacks any way to do that, like gtk mnemonics. Anyway, as I said on comment 2, if St provides this mnemonics we would require to add that on the js code. Finally, as the original description were focused on the dash/overview, I will change a little the description and set the proper bug dependencies.
Comment on attachment 173517 [details] [review] a11y support an placeDisplayItem Obsolete due the integration of overview-relayout branch
Comment on attachment 173518 [details] [review] a11y support on each Dash section Obsolete due overview-relayout branch integration
Comment on attachment 173516 [details] [review] a11y support an appWellIcon >+ // accessibility support >+ let accessible = this.actor.get_accessible(); >+ let label_accessible = this._icon._name.get_accessible (); //FIXME: _name is private >+ >+ if ((accessible) && (label_accessible)){ >+ accessible.add_relationship (Atk.RelationType.LABELLED_BY, label_accessible); >+ label_accessible.add_relationship (Atk.RelationType.LABEL_FOR, accessible); >+ accessible.set_role (Atk.Role.PUSH_BUTTON); so, tying in to what I said in bug 636717 comment 4, I think the way this code *should* look is that the role could just be specified as a (StWidget) property at construct time: this.actor = new St.Clickable({ style_class: 'app-well-app', reactive: true, x_fill: true, y_fill: true, accessible_role: Atk.Role.PUSH_BUTTON }); (except that really you wouldn't even need to specify it because StClickable should default to ATK_ROLE_PUSH_BUTTON). and the relationship would be specified by something like: this._icon._name.is_label_for = this.actor; (except without the illegal private member access), where is_label_for would be an StLabel property, and setting it would add the correct relationship to both the labeler and the labelee.
(In reply to comment #9) > (From update of attachment 173516 [details] [review]) > >+ // accessibility support > >+ let accessible = this.actor.get_accessible(); > >+ let label_accessible = this._icon._name.get_accessible (); //FIXME: _name is private > >+ > >+ if ((accessible) && (label_accessible)){ > >+ accessible.add_relationship (Atk.RelationType.LABELLED_BY, label_accessible); > >+ label_accessible.add_relationship (Atk.RelationType.LABEL_FOR, accessible); > >+ accessible.set_role (Atk.Role.PUSH_BUTTON); > > so, tying in to what I said in bug 636717 comment 4, I think the way this code > *should* look is that the role could just be specified as a (StWidget) property > at construct time: > > this.actor = new St.Clickable({ style_class: 'app-well-app', > reactive: true, > x_fill: true, > y_fill: true, > accessible_role: Atk.Role.PUSH_BUTTON > }); > > (except that really you wouldn't even need to specify it because StClickable > should default to ATK_ROLE_PUSH_BUTTON). Yes, I already have StClickable on my TODO list, to at least, set the proper role, and probably to start to set the proper AtkActions on the widget. Right now CallyActor adds the actions "click", "release", "press" on any object. The reason is that when I started Cally some people were using any ClutterActor (specially ClutterTexture) as buttons (and still do that). So as technically you can interact with any ClutterActor, I decided to add those actions on any ClutterActor. Of course that mean that in several cases, you click on a actor and you didn't get any answer. But in the case of St, as there is enough context information, I think that it makes sense to have these actions just to clickable elements. However, independently of this action stuff, I think that it is worth to create the StClickableAccessible object to have, at least, the proper default role for this object. > and the relationship would be specified by something like: > > this._icon._name.is_label_for = this.actor; > > (except without the illegal private member access), where is_label_for would be > an StLabel property, and setting it would add the correct relationship to both > the labeler and the labelee. I guess that in this case you are proposing a new property StLabel:is_label_for. What about something more similar to what GtkWidget have: mnemonics. In this case it would be something like replicate these methods: gtk_widget_list_mnemonic_labels gtk_widget_add_mnemonic_label gtk_widget_remove_mnemonic_label on StWidget, and change that call for something like: this.actor.add_mnemonic_label (this._icon._name) (except without the illegal private member access)
(In reply to comment #10) > I guess that in this case you are proposing a new property > StLabel:is_label_for. > > What about something more similar to what GtkWidget have: mnemonics. In this > case it would be something like replicate these methods: > > gtk_widget_list_mnemonic_labels > gtk_widget_add_mnemonic_label > gtk_widget_remove_mnemonic_label Well, we don't currently have any concept on mnemonics on labels though, so the name would be wrong. But we could have the API be on StWidget rather than StLabel, whatever it ended up being called.
(In reply to comment #11) > (In reply to comment #10) > > I guess that in this case you are proposing a new property > > StLabel:is_label_for. > > > > What about something more similar to what GtkWidget have: mnemonics. In this > > case it would be something like replicate these methods: > > > > gtk_widget_list_mnemonic_labels > > gtk_widget_add_mnemonic_label > > gtk_widget_remove_mnemonic_label > > Well, we don't currently have any concept on mnemonics on labels though, so the > name would be wrong. But we could have the API be on StWidget rather than > StLabel, whatever it ended up being called. Yes, when I was comparing that to the gtk case, I was thinking on add the API to st_widget, something like (avoiding mnemonics, as for your comment this is not the way to go): GList* st_widget_list_<word>_labels (StWidget *widget) void st_widget_add_<word>_label (StWidget *widget, StWidget *label) void st_widget_remove_<word>_label (StWidget *widget, StWidget *label) Where <word> would be something like "naming", "mnemonic" (if in the end it the concept is imported to gnome-shell) or just void.
Something similar to what we were discussing on this bug implemented on bug 644253. As with other accessibility related gnome shell bugs, the meaning of this bug has become somewhat vague, so as we have something more specific on that bug, and others are coming, I will close this bug as OBSOLETE, due all the discussions related.