WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
NEW
139581
AX: [ATK] Use ATK_RELATION_MEMBER_OF to implement linkedUIElements for radio button group members.
https://bugs.webkit.org/show_bug.cgi?id=139581
Summary
AX: [ATK] Use ATK_RELATION_MEMBER_OF to implement linkedUIElements for radio ...
Andrzej Badowski
Reported
2014-12-12 03:04:36 PST
I would like to be discussed the use of the ATK_RELATION_MEMBER_OF in order to expose relationship AccessibilityRenderObject::linkedUIElements.
Attachments
proposed patch
(11.73 KB, patch)
2014-12-13 04:50 PST
,
Andrzej Badowski
no flags
Details
Formatted Diff
Diff
proposed patch 2
(11.92 KB, patch)
2014-12-29 04:17 PST
,
Andrzej Badowski
no flags
Details
Formatted Diff
Diff
proposed patch 2
(11.92 KB, patch)
2014-12-29 05:46 PST
,
Andrzej Badowski
no flags
Details
Formatted Diff
Diff
proposed patch 2
(11.92 KB, patch)
2014-12-30 02:02 PST
,
Andrzej Badowski
cfleizach
: review-
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Andrzej Badowski
Comment 1
2014-12-13 04:50:34 PST
Created
attachment 243255
[details]
proposed patch
Joanmarie Diggs
Comment 2
2014-12-15 01:19:39 PST
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
Comment 3
2014-12-16 01:01:13 PST
(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 4
2014-12-16 11:38:25 PST
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
Comment 5
2014-12-16 13:21:02 PST
(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
Comment 6
2014-12-16 13:21:28 PST
(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
Comment 7
2014-12-17 04:27:35 PST
(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
Comment 8
2014-12-17 05:59:03 PST
(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 9
2014-12-17 06:00:16 PST
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
Comment 10
2014-12-17 11:22:38 PST
<
rdar://problem/19281517
>
Andrzej Badowski
Comment 11
2014-12-29 04:17:50 PST
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
Comment 12
2014-12-29 05:46:23 PST
Created
attachment 243794
[details]
proposed patch 2
Andrzej Badowski
Comment 13
2014-12-30 02:02:25 PST
Created
attachment 243819
[details]
proposed patch 2 attaching another patch 2 due to efl-WK2 build errors not associated with the patch
Andrzej Badowski
Comment 14
2015-01-09 01:40:01 PST
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
Comment 15
2016-08-17 08:59:39 PDT
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
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug