Bug 139581 – AX: [ATK] Use ATK_RELATION_MEMBER_OF to implement linkedUIElements for radio button group members.

• Component: Accessibility
• Status: NEW

[Andrzej Badowski]

I would like to be discussed the use of the ATK_RELATION_MEMBER_OF in order to expose relationship AccessibilityRenderObject::linkedUIElements.

[Andrzej Badowski]

Created attachment 243255 [details]
proposed patch

[Joanmarie Diggs]

It looks to me like ATK_RELATION_MEMBER_OF is applicable for a group of radio buttons because those radio buttons are all a member of a group. However, I question (quite a bit) its applicability for an internal anchor connection.

[Andrzej Badowski]

(In reply to comment #2)
> It looks to me like ATK_RELATION_MEMBER_OF is applicable for a group of
> radio buttons because those radio buttons are all a member of a group.
> However, I question (quite a bit) its applicability for an internal anchor
> connection.

Thank you for your feedback. I understand your concerns about internal anchor link, I had it as well. but: 
1. There is no other relationship ATK more appropriate to do so. 
2. The linkedUIElements treats both objects together.

[chris fleizach]

Comment on attachment 243255 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=243255&action=review

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1046
> +    return roleValue() == RadioButtonRole || (isLink() && internalLinkElement());

how come isLink gets this attribute?
i don't think there are any tests which test the isLink part

[Joanmarie Diggs]

(In reply to comment #3)
> (In reply to comment #2)
> > It looks to me like ATK_RELATION_MEMBER_OF is applicable for a group of
> > radio buttons because those radio buttons are all a member of a group.
> > However, I question (quite a bit) its applicability for an internal anchor
> > connection.
> 
> Thank you for your feedback. I understand your concerns about internal
> anchor link, I had it as well. but: 
> 1. There is no other relationship ATK more appropriate to do so. 

Assuming there is a use case for that, then the correct solution would be to file a bug against ATK requesting a new AtkRelation type; not to lump non-members together with members. 

> 2. The linkedUIElements treats both objects together.

Just because a particular toolkit or engine does something does not make it appropriate for all platforms.

Taking the above into account, for GTK please do NOT include internal anchors in the member-of AtkRelation type.

[Joanmarie Diggs]

(In reply to comment #3)
> (In reply to comment #2)
> > It looks to me like ATK_RELATION_MEMBER_OF is applicable for a group of
> > radio buttons because those radio buttons are all a member of a group.
> > However, I question (quite a bit) its applicability for an internal anchor
> > connection.
> 
> Thank you for your feedback. I understand your concerns about internal
> anchor link, I had it as well. but: 
> 1. There is no other relationship ATK more appropriate to do so. 

Assuming there is a use case for that, then the correct solution would be to file a bug against ATK requesting a new AtkRelation type; not to lump non-members together with members. 

> 2. The linkedUIElements treats both objects together.

Just because a particular toolkit or engine does something does not make it appropriate for all platforms.

Taking the above into account, for GTK please do NOT include internal anchors in the member-of AtkRelation type.

[Alejandro Piñeiro]

(In reply to comment #3)
> (In reply to comment #2)
> > It looks to me like ATK_RELATION_MEMBER_OF is applicable for a group of
> > radio buttons because those radio buttons are all a member of a group.
> > However, I question (quite a bit) its applicability for an internal anchor
> > connection.
> 
> Thank you for your feedback. I understand your concerns about internal
> anchor link, I had it as well. but: 
> 1. There is no other relationship ATK more appropriate to do so. 

First, as Joanmarie said, if you think that an appropiate relationship is missing, then the way to go is asking for a new ATK relation. Relationships has meaning, that different ATs use in order to know how to present information to the users. ATK_RELATION_MEMBER has a meaning, and it is not to include anchors.

Second, I really don't think that a relationship is needed here. Relations, at least in ATK, are used to relate two (or more) different objects, because is a convenient way to get one based on the other. But for the case of links/anchors, that is not needed because ATK API for links already contemplate this:

https://developer.gnome.org/atk/unstable/AtkHyperlink.html#atk-hyperlink-get-object

So you can already get the anchors from an Hypertext document without a relationship.

> 2. The linkedUIElements treats both objects together.

I know that one of the objectives of the test is also detecting things that are still not implemented on the ports, but in this case, seems that is more like that the current test infrastructure is forcing a specific accessibility implementation, that is not one of the objectives. As I said, ATK hyperlink/hypertext API already provides to ATs the way to get the individual anchors, without the need of adding a relationship. Adding the relationship sounds like adding something that ATs doesn't need, so it is just complicating the implementation (imho).

[Andrzej Badowski]

(In reply to comment #7)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > It looks to me like ATK_RELATION_MEMBER_OF is applicable for a group of
> > > radio buttons because those radio buttons are all a member of a group.
> > > However, I question (quite a bit) its applicability for an internal anchor
> > > connection.
> > 
> > Thank you for your feedback. I understand your concerns about internal
> > anchor link, I had it as well. but: 
> > 1. There is no other relationship ATK more appropriate to do so. 
> 
> First, as Joanmarie said, if you think that an appropiate relationship is
> missing, then the way to go is asking for a new ATK relation. Relationships
> has meaning, that different ATs use in order to know how to present
> information to the users. ATK_RELATION_MEMBER has a meaning, and it is not
> to include anchors.
> 
> Second, I really don't think that a relationship is needed here. Relations,
> at least in ATK, are used to relate two (or more) different objects, because
> is a convenient way to get one based on the other. But for the case of
> links/anchors, that is not needed because ATK API for links already
> contemplate this:
> 
> https://developer.gnome.org/atk/unstable/AtkHyperlink.html#atk-hyperlink-get-
> object
> 
> So you can already get the anchors from an Hypertext document without a
> relationship.
> 
> > 2. The linkedUIElements treats both objects together.
> 
> I know that one of the objectives of the test is also detecting things that
> are still not implemented on the ports, but in this case, seems that is more
> like that the current test infrastructure is forcing a specific
> accessibility implementation, that is not one of the objectives. As I said,
> ATK hyperlink/hypertext API already provides to ATs the way to get the
> individual anchors, without the need of adding a relationship. Adding the
> relationship sounds like adding something that ATs doesn't need, so it is
> just complicating the implementation (imho).
Thank you for checking and tips on how to solve the problem.

[Andrzej Badowski]

Comment on attachment 243255 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=243255&action=review

>> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1046
>> +    return roleValue() == RadioButtonRole || (isLink() && internalLinkElement());
> 
> how come isLink gets this attribute?
> i don't think there are any tests which test the isLink part

You're right. I should prepare a suitable test.

[Radar WebKit Bug Importer]

<rdar://problem/19281517>

[Andrzej Badowski]

Created attachment 243792 [details]
proposed patch 2

This patch takes into account negative positions of debaters as to the ATK_RELATION_MEMBER_OF applicability for the internal link.
In the current version ATK_RELATION_MEMBER_OF is restricted to the radio button group.

[Andrzej Badowski]

Created attachment 243794 [details]
proposed patch 2

[Andrzej Badowski]

Created attachment 243819 [details]
proposed patch 2

attaching another patch 2 due to  efl-WK2 build errors not associated with the patch

[Andrzej Badowski]

The function mentioned by Alejandro (atk-hyperlink-get-object) returns an object that is a visual representation of internal link (text or picture) rather than the object pointed to by the link. Hence, I would like to temporarily suspend this type of relationship and reduce the patch to a group of radiobuttons.
Getting to the object pointed to by the link requires several steps and I'm going to deal with this in the future.
What do you think about this, Joanie ?

[chris fleizach]

this patch is almost two years old. I don't know why its languished so long, but marking as r- so it can at least be re-based