[GTK] accessibility/selection-states.html is failing

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Zan Dobersek]

accessibility/selection-states.html is failing on all GTK platforms.
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=accessibility%2Fselection-states.html

[Joanmarie Diggs]

Another just implement methods in DRT:
* AccessibilityUIElement::isSelectable()
* AccessibilityUIElement::isSelected()
* AccessibilityUIElement::isMultiSelectable()

Piñeiro: Yours. :)

BTW, there's a checkElementState() convenience method.

[Alejandro Piñeiro]

(In reply to comment #1)
> Another just implement methods in DRT:

Unfourtunately, this seems that it is not the case. After adding the implementation for those methods the test fail. This is the expected outcome:

PASS accessibilityController.focusedElement.isMultiSelectable is true
PASS accessibilityController.focusedElement.childAtIndex(0).isSelectable is true
PASS accessibilityController.focusedElement.childAtIndex(0).isSelected is true
PASS accessibilityController.focusedElement.childAtIndex(1).isSelectable is true
PASS accessibilityController.focusedElement.childAtIndex(1).isSelected is false
PASS accessibilityController.focusedElement.childAtIndex(2).isSelectable is false
PASS accessibilityController.focusedElement.childAtIndex(2).isSelected is false

And this is the actual outcome:

PASS accessibilityController.focusedElement.isMultiSelectable is true
PASS accessibilityController.focusedElement.childAtIndex(0).isSelectable is true
PASS accessibilityController.focusedElement.childAtIndex(0).isSelected is true
PASS accessibilityController.focusedElement.childAtIndex(1).isSelectable is true
PASS accessibilityController.focusedElement.childAtIndex(1).isSelected is false
FAIL accessibilityController.focusedElement.childAtIndex(2).isSelectable should be false. Was true.
PASS accessibilityController.focusedElement.childAtIndex(2).isSelected is false

FWIW, without implementing those methods, the outcome is the following:

FAIL accessibilityController.focusedElement.isMultiSelectable should be true. Was false.
FAIL accessibilityController.focusedElement.childAtIndex(0).isSelectable should be true. Was false.
PASS accessibilityController.focusedElement.childAtIndex(0).isSelected is true
FAIL accessibilityController.focusedElement.childAtIndex(1).isSelectable should be true. Was false.
PASS accessibilityController.focusedElement.childAtIndex(1).isSelected is false
PASS accessibilityController.focusedElement.childAtIndex(2).isSelectable is false
PASS accessibilityController.focusedElement.childAtIndex(2).isSelected is false

So with the first step of implemented the methods I improved the previous 4/7 pass percentage to 6/7. Next step is investigate why I'm getting that wrong isSelected==TRUE state.

> * AccessibilityUIElement::isSelectable()
> * AccessibilityUIElement::isSelected()

BTW: this one was implemented while solving bug 50814. I just implemented isSelectable and isMultiselectable.

> * AccessibilityUIElement::isMultiSelectable()
> 
> Piñeiro: Yours. :)

Using WebkitGTK hackfest to catch it.

> BTW, there's a checkElementState() convenience method.

Saw that, using it. Thanks.

[Alejandro Piñeiro]

(In reply to comment #2)

> So with the first step of implemented the methods I improved the previous 4/7 pass percentage to 6/7. Next step is investigate why I'm getting that wrong isSelected==TRUE state.

Thats a typo, the fail is getting a wrong isSelectable==TRUE state. Anyway ...

After a little research, the one adding this state is not the webkit ATK related code, but ATK itself. The basic ref_state_set at atkobject.c adds some states. One is focused if the object is the current focused object (something that probably is broken) and the other is SELECTABLE/SELECTED states:

 * For any object checks if the parent is an ATK_SELECTION
 * If it is an ATK_SELECTION it add the state ATK_STATE_SELECTABLE *always*
 * Additionally it asks through the atk_selection interface which is the element selected in order to add ATK_STATE_SELECTED if applies

Basically when this was implemented, it assumed that every children of an object that implements ATK_SELECTION would be selectable. Obviously this is not true here, and I really doubt that in any GTK flexible list-container (like gtktreeview) you couldn't do the same. In general I think that so general assumption is wrong, and would depend on the case. One could be tempted to suggest to check if the object is enabled or not, but this is also a state that is checked on the children.

I will create a bug on ATK in relation with that problem, but before removing that code I would need to talk with toolkit maintainers, like GTK+. Removing the code would mean that all the SELECTABLE state management needs to be done by the toolkits, and I suspect that right now this is not the case.

[Alejandro Piñeiro]

(In reply to comment #3)

> I will create a bug on ATK in relation with that problem, but before removing that code I would need to talk with toolkit maintainers, like GTK+. Removing the code would mean that all the SELECTABLE state management needs to be done by the toolkits, and I suspect that right now this is not the case.

ATK bug created: https://bugzilla.gnome.org/show_bug.cgi?id=689907

[Alejandro Piñeiro]

As this bug depends on two things, I have just created a new bug. Bug 104481 is about the missed methods implemented at the DRT (see comment 1). The SELECTABLE thing is being managed at an ATK bug (see comment 4). So in this way we could get the WebKit part solved at bug 104481, and one the bots starts to use a ATK version that solves the other problem, we could close this bug.

[Joanmarie Diggs]

Here is my understanding (correct me if I am wrong):

* At this point the "fix" for this bug is to unskip the test.

* Unskipping the test is desired because a11y regressions can
  occur from all sorts of things that seem on the surface not
  related to a11y.

* Unskipping the test will, however, require bumping the ATK
  dependency to >= 2.7.3. (i.e. 2.8) because that is where the
  bug was.

Bumping dependencies is no fun, but in this case I think it would be worth it. After all, it's not like ATK is huge, hard to get, or hard to build. :)

Related to this: I noticed that ATK is in the "optional" moduleset, whereas at-spi2-core and at-spi2-atk are part of "webkitgtk-testing-dependencies." Shouldn't ATK also be part of the webkitgtk-testing-dependencies?

Martin, please advise. Thanks!

[Martin Robinson]

(In reply to comment #6)

> Related to this: I noticed that ATK is in the "optional" moduleset, whereas at-spi2-core and at-spi2-atk are part of "webkitgtk-testing-dependencies." Shouldn't ATK also be part of the webkitgtk-testing-
> Martin, please advise. Thanks!

Please see https://bugs.webkit.org/show_bug.cgi?id=113282#c23 I suppose since tests do not pass without the new version it must go in the jhbuild. Sadly, Carlos' work would have been a lot easier if we had known this a few days ago. I should have brought you in on the conversation. Consider this an apology to you and Carlos.

[Joanmarie Diggs]

Thanks Martin.

My questions still remain, however. What Carlos committed is for at-spi2-core and at-spi2-atk (the latter being the accessibility bridge). What needs to be bumped in terms of this bug is ATK itself. ATK itself is "optional" at the moment. And the "optional" version is not high enough.

[Martin Robinson]

Since ATK is now required to be at a specific version to pass tests, it should be moved from optional to the main module set.

[Zan Dobersek]

Doing the ATK bump in bug #113883 but also bumping at-spi2-core and at-spi2-atk as the 2.5.3 versions used might be causing crashes in libatk-bridge.so as currently evident on the 64-bit release builder (which uses ATK 2.8.0, the version to which the bump will be made).
http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release/r147556%20(36388)/results.html

[Zan Dobersek]

Removed expectations in r147727, the test now passes.
http://trac.webkit.org/changeset/147727