494346 - STATE_FOCUSABLE not exposed on focusable video sliders

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Marco Zehe (:MarcoZ)]

Found after bug 490287 landed. NVDA does not properly go into focus mode because the sliders, although focusable, don't expose the focusable state.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[alexander :surkov (:asurkov)]

1) Bug doesn't make sense if controls aren't focusable
2) I introduced action "activate" on slider accessible
3) I do know what I fixed in bug 492518 and what my reviewers reviewed but that fix makes nothing :) so I fixed it here.

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 379143 [details] [diff] [review]
patch

Looks okay. r=me

> NS_IMETHODIMP
> nsXULSliderAccessible::GetValue(nsAString& aValue)
> {
>   return GetSliderAttr(nsAccessibilityAtoms::curpos, aValue);
>+}
>+
>+NS_IMETHODIMP
>+nsXULSliderAccessible::GetNumActions(PRUint8 *aCount)
>+{
>+  NS_ENSURE_ARG_POINTER(aCount);
>+
>+  *aCount = 1;
>+  return NS_OK;
>+}

Maybe file a follow up to add more actions.

Comment 4

[Marco Zehe (:MarcoZ)]

Comment on attachment 379143 [details] [diff] [review]
patch

r=me. This causes all the sliders to behave in a predictable manner when pressing ENTER on them from NVDA's virtual buffer.
I have to press ENTER on the "unknown object" accessible which is the anonymous xul:label which we marked up as presentation to get focus to the scrubber. BUT we can deal with that separately.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 379143 [details] [diff] [review]
patch

a191=beltzner; please land on mozilla-central first, and if all the boxes stay green, you can land on mozilla-1.9.1

Comment 6

[Marco Zehe (:MarcoZ)]

Pushed on Alexander's behalf in changeset:
http://hg.mozilla.org/mozilla-central/rev/6e3296a9980b

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #3)

> 
> Maybe file a follow up to add more actions.

David, if it will be you then I would be happy ;) What kind of actions do we expect? We need to check this with ATs.

Comment 8

[alexander :surkov (:asurkov)]

Attachment: patch 1.9.1

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 379143 [details] [diff] [review]
patch

We no longer want this for trunk as the revised patch on bug 486899 was causing problems with the keyboard shortcuts, and this patch caused problems with the previous fix for 486899.

We may take it again at some point, but for now, it'll get backed out of the tree.

Comment 10

[David Bolter [:davidb] (NeedInfo me for attention)]

This was pushed to 1.9.1 before discovery of keyboard conflict, and backed out via bsmedberg.
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f2b6a8093866

Comment 11

[alexander :surkov (:asurkov)]

It was backed out from trunk as well but there is no link on hg revision and there is no link on failing tests. So what's wrong with this bug because it can't affect on bug 486899 since it's pure a11y fix?

Comment 12

[alexander :surkov (:asurkov)]

Comment on attachment 379178 [details] [diff] [review]
patch 1.9.1

reaskin approaval on 1.9.1 - it's really good fix for xul:scale

1. It exposes focusable and focused states on slider accessible
2. It fires focus events on slider accessible
3. It exposes accessible action on slider accessilbe

So definitely we improve a lot the slider accessible even if this fix is not useful FOR video/audio controls because XUL sliders aren't focusable after bug 486899.

Comment 13

[alexander :surkov (:asurkov)]

(In reply to comment #11)
> It was backed out from trunk as well

Sorry it is still there (I looked at 1.9.1 tree). So marking fixed again.

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 379178 [details] [diff] [review]
patch 1.9.1

David: can you help clarify if the concerns in comment 9 and comment 10 have been addressed?

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 379178 [details] [diff] [review]
patch 1.9.1

Need an answer to Beltzner's question in comment 14. If you still want to land it on the 1.9.1 branch please re-request approval (for 1.9.1.4)

Comment 16

[David Bolter [:davidb] (NeedInfo me for attention)]

Sorry for the delay, comment 14 got lost during a 5 week holiday. We'll need to user test this patch on a 1.9.1 tree and re-request if/when we confirm no keyboard conflicts.

Alexander: can you check this? And before requesting 1.9.1 let's IRC a bit.

Comment 17

[David Bolter [:davidb] (NeedInfo me for attention)]

Let's test both default action and accSelect. I'm curious if the latter works.