Closed Bug 1134097 Opened 9 years ago Closed 9 years ago

[Accessibility] Add support for aria-hint (explicit and implicit) in Gaia::System.

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S7 (6mar)
Tracking Status
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: yzen, Assigned: bugzilla, Mentored)

References

Details

(Keywords: access, late-l10n)

Attachments

(2 files)

We added support in gecko for hints:
https://github.com/mozilla/gecko-dev/blob/master/accessible/jsat/Presentation.jsm#L517

They are set via the aria-moz-hint attribute or calculated implicitly based on the role iff the accessible has a greater that 0 action cound:
https://github.com/mozilla/gecko-dev/blob/master/accessible/jsat/Utils.jsm#L839-L856

We now should be able to add support for the hints in Gaia in the following way:
* When the screen reader cursor changes to a new location, we will optionally get the hint string to be localized as part of the event data when handling a vc-change event:
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/accessibility.js#L295-L299
* We would then wait for a moment (3? 5? seconds) for another accessibility-output or input event:
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/accessibility.js#L350 and if we don't get any, we speak the localized hint out.
* If a new accessibility-output or input event arrives while we are speaking the hint, we will stop the speech immediately.
Assignee: nobody → bugzilla
Hey Yura,

I've attached this very rough outline as a file here to serve as a guideline for a few questions -- hopefully it makes sense.

1.  So I guess my first question is whether or not this is what you're looking for in terms of implementation...
 - I've got some kind of parent-scoped "event-tracking" mechanism that tells me how many accessibility events have fired.
 - I've got some kind of "isPlaying" mechanism to tell me if a hint is currently playing
 - I've got a setTimeout function that only fires if the eventIndex hasn't changed since its timer was started
 - I've got an IF() statement that stops the hint from playing if another event fires while it's playing
 - And I've got an announceHint() function that starts speaking the hint and fires a callback that both stops the speaker and updates the isPlaying tracker variable on completion of the speech.

If I'm missing something there, please let me know.

2.  As noted above by their "mechanism" delineation, I've made a few assumptions with the outline that I'm not sure are correct.  Are there implementations for the following that I should look to use?
 
 - The accessibility event tracking object
 - An "isSpeaking" or "isPlaying" status for the speaker and/or screen reader?

3.  One thing that I felt I may not have grasped entirely from the description was whether or not were needing to localize the hints in the same way that we use data-l10n-id for the ARIA Labels.  Is that something that needs to be taken into account?
Attachment #8566265 - Flags: feedback?(yzenevich)
Hi Ross, Some comments inline (will go through them more thoroughly tomorrow).
(In reply to Ross from comment #1)
> Created attachment 8566265 [details]
> Rough outline of functionality
> 
> Hey Yura,
> 
> I've attached this very rough outline as a file here to serve as a guideline
> for a few questions -- hopefully it makes sense.

A quick note on the implementation, you would want to set your timeout in the speak callback here: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/accessibility.js#L312 as it is only used when vc-changes (and that's where we can potentially get the hints). as for checking if we got an event, you can simple have a tracking property on our object that you set as follows:

this.hintTimer = setTimeout(...); then if you get an accessibility output/control you just call clearTimeout(this.hintTimer); to prevent it ever firing. More to follow.

> 
> 1.  So I guess my first question is whether or not this is what you're
> looking for in terms of implementation...
>  - I've got some kind of parent-scoped "event-tracking" mechanism that tells
> me how many accessibility events have fired.
>  - I've got some kind of "isPlaying" mechanism to tell me if a hint is
> currently playing
>  - I've got a setTimeout function that only fires if the eventIndex hasn't
> changed since its timer was started
>  - I've got an IF() statement that stops the hint from playing if another
> event fires while it's playing
>  - And I've got an announceHint() function that starts speaking the hint and
> fires a callback that both stops the speaker and updates the isPlaying
> tracker variable on completion of the speech.
> 
> If I'm missing something there, please let me know.
> 
> 2.  As noted above by their "mechanism" delineation, I've made a few
> assumptions with the outline that I'm not sure are correct.  Are there
> implementations for the following that I should look to use?
>  
>  - The accessibility event tracking object
>  - An "isSpeaking" or "isPlaying" status for the speaker and/or screen
> reader?

It would be nice to keep the state for all stuff being spoken, but isSpeaking is reserved for screen reader on/off announcements. So if we end up setting the isSpeaking state, we would need to rename the currently existing one to avoid confusion.

> 
> 3.  One thing that I felt I may not have grasped entirely from the
> description was whether or not were needing to localize the hints in the
> same way that we use data-l10n-id for the ARIA Labels.  Is that something
> that needs to be taken into account?

We would, but that would be a separate bug. At this point we will only care about implicit hints which will be calculated in Gecko for the following controls:

    'pushbutton',
    'checkbutton',
    'combobox',
    'key',
    'link',
    'menuitem',
    'check menu item',
    'radio menu item',
    'option',
    'radiobutton',
    'slider',
    'spinbutton',
    'pagetab',
    'entry',
    'outlineitem'

Which means we would have to add *-hint localization strings in https://github.com/mozilla-b2g/gaia/blob/master/apps/system/locales/accessibility.en-US.properties
Some more suggestions:
so in the callback that you would define instead of the null argument https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/accessibility.js#L312 you would define a function that would:
* check for the presence of options.hints 
* if there are some, call this.setHintsTimeout();
* your setHintsTimeout would do something similar to what you sketched in 'vc-change' handling case, except for you would save the timeout id as a property inside of the Accessibility object (you would also want to clearTimeout and delete the id in the setTimeout callback).
* in the beginning of the handleAccessFuOutput and handleAccessFuControl, we would check if the id still exists and if so, we would clearTimeout and delete the id.
* The last thing (afaik) we need to take care of is, whether the hint is spoken ATM if we are cancelling it, for that we would need to add another flag that is similar to isSpeaking, perhaps isHintSpeaking. That we would set to true right before we call this.speak for the hint, and set to false in its own callback.

Let me know if that was the sort of thing you were thinking? thanks
Attachment #8566265 - Flags: feedback?(yzenevich)
Hey Yura,

Thanks so much for the descriptive clarifications and/or progressions upon my sketch.  I was able to go through and implement your suggestions in the PR but there were a few more things I was hoping to get cleared up...

1) In order to speak the hints, I called the Accessibility.speak() method with options.hints as its first argument.  I was hoping that options.hints would be formatted the same as an "aData" (speech data array), but because I was unable to inspect the contents of either (more on that below), I'm unsure that that's the correct way to do things.

2) The hints themselves, from the way I understand it, are being generated based on the role of the element that fired the event.  So, a "slider" for instance, should have its hint stored as {string:"slider-hint"}, after get interactionHints() (https://github.com/mozilla/gecko-dev/blob/master/accessible/jsat/Utils.jsm#L839-L856) is called, right?  So, does that mean that we'll have to populate https://github.com/mozilla-b2g/gaia/blob/master/apps/system/locales/accessibility.en-US.properties with hint definitions for all of the actionCount > 0 roles?  If so, going back to "slider" as an example, would that just look like "slider-hint = I'm the slider's hint"?  And finally, what are hoping to provide to the user with these hints?  Should a slider's hint look more like "Swipe up or down on the slider to progress"?

3) Debugging:  So, as mentioned in the past, I'm having trouble with WebIDE giving me any real feedback while connected to my Flame phone and it's making things fairly difficult to debug and/or inspect.  As of right now, the following functionality is broken:

 - I can't see any CSS Rules (the pane is just blank)
 - console.log()(written from inside the APP's .js file) doesn't write anything to the console
 - There are no errors logged in the console
 - I'm unable to see any javascript files at all
 - Without the javascript files, I'm naturally unable to step through the code and/or inspect memory

Without the functionality above, I'm having to get a little more creative than I'd like to debug the code, so I'm just wondering if there's anything else I can do with WebIDE or otherwise to get a better debugging environment setup.  I've got the latest versions of everything (B2G/Gaia/FirefoxDevEd) so I'm not sure where else to go.
Flags: needinfo?(yzenevich)
Hi Ross,

(In reply to Ross from comment #5)
> Hey Yura,
> 
> Thanks so much for the descriptive clarifications and/or progressions upon
> my sketch.  I was able to go through and implement your suggestions in the
> PR but there were a few more things I was hoping to get cleared up...
> 
> 1) In order to speak the hints, I called the Accessibility.speak() method
> with options.hints as its first argument.  I was hoping that options.hints
> would be formatted the same as an "aData" (speech data array), but because I
> was unable to inspect the contents of either (more on that below), I'm
> unsure that that's the correct way to do things.

Yes they will look like the speech data array, some examples can be found in tests: https://github.com/mozilla/gecko-dev/blob/master/accessible/tests/mochitest/jsat/test_hints.html

> 
> 2) The hints themselves, from the way I understand it, are being generated
> based on the role of the element that fired the event.  So, a "slider" for
> instance, should have its hint stored as {string:"slider-hint"}, after get
> interactionHints()
> (https://github.com/mozilla/gecko-dev/blob/master/accessible/jsat/Utils.
> jsm#L839-L856) is called, right?  So, does that mean that we'll have to
> populate
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/locales/
> accessibility.en-US.properties with hint definitions for all of the
> actionCount > 0 roles?  

Correct.The list that I mentioned in comment 2 above is pretty much complete.

> If so, going back to "slider" as an example, would
> that just look like "slider-hint = I'm the slider's hint"?  

Yes

> And finally,
> what are hoping to provide to the user with these hints?  Should a slider's
> hint look more like "Swipe up or down on the slider to progress"?

Yes so this is something that needs to be descriptive (yet terse) enough and that explains how certain roles need o be interacted with. So for example: 
* for pushbutton, checkbutton, link and other similar ones we would say "Double tap to activate"
* for slider as you said we would have the swipe up swipe down comment.

By the way, I think we need to be a little smarter in terms of what we say. The hint (because it is delayed) should be accompanied by the element description itself to give it more context so for example:
instead of just saying "Double tap to acticate", it would be more helpful to repeat what we are focused on, for example: "Camera link, Double tap to activate". Which means in addition to hints you would save aData as well and then concat the hints to it. Play around with it, see what really makes sense usability wise.

> 3) Debugging:  So, as mentioned in the past, I'm having trouble with WebIDE
> giving me any real feedback while connected to my Flame phone and it's
> making things fairly difficult to debug and/or inspect.  As of right now,
> the following functionality is broken:
> 
>  - I can't see any CSS Rules (the pane is just blank)
>  - console.log()(written from inside the APP's .js file) doesn't write
> anything to the console
>  - There are no errors logged in the console
>  - I'm unable to see any javascript files at all
>  - Without the javascript files, I'm naturally unable to step through the
> code and/or inspect memory
> 
> Without the functionality above, I'm having to get a little more creative
> than I'd like to debug the code, so I'm just wondering if there's anything
> else I can do with WebIDE or otherwise to get a better debugging environment
> setup.  I've got the latest versions of everything (B2G/Gaia/FirefoxDevEd)
> so I'm not sure where else to go.

Hmm, I wonder if you have an out of date gecko/gonk layer (if it even doesn't work with latest Firefox Nightly). See if you can update your flame to the latest nightly: https://developer.mozilla.org/en-US/Firefox_OS/Phone_guide/Flame/Updating_your_Flame . Hopefully that can help when you flash your gaia branch on top and try debugging.
Flags: needinfo?(yzenevich)
Okay, after changing a few settings in "about:config" as recommended, re-installing my drivers, and unplugging/replugging my device a few times, I was finally able to get WebIDE to load the js files and allow me to do some debugging.  Still no CSS rules on the Inspector for some odd reason, but BIG improvement over this morning nonetheless! :)

Updated the PR with working functionality (the hints are actually working on the phone) and nits addressed.  Left a few questions inline on GitHub.  Once those (and any remaining issues) are taken care of I'll look toward the unit tests and try to wrap this up.

Thanks again for the assistance this morning!
Flags: needinfo?(yzenevich)
(In reply to Ross from comment #7)
> Still no CSS rules on the Inspector for some odd reason, but BIG
> improvement over this morning nonetheless! :)

I know this one! I raised this issue in bug 1132255, and was informed that you need to be using a recently nightly (2015-02-03 or later) Once I got back on the nightly track it resolved itself.
> I know this one! I raised this issue in bug 1132255, and was informed that
> you need to be using a recently nightly (2015-02-03 or later) Once I got
> back on the nightly track it resolved itself.

Hey Sam, yeah, I generally try to keep everything up to date.  I'm currently running Firefox Developer Edition 37.0a2 (2015-02-18) for Ubuntu and 37.0a2 (2013-02-23) on Windows and the CSS rules are still absent.  They used to work (and really well, at that), but mysteriously went away around 3.5 weeks ago...  Thanks for the heads-up tho.
> and 37.0a2 (2013-02-23) on Windows

Sorry, should read 2015-02-23
(In reply to Ross from comment #9)
> > I know this one! I raised this issue in bug 1132255, and was informed that
> > you need to be using a recently nightly (2015-02-03 or later) Once I got
> > back on the nightly track it resolved itself.

Yeah, it seems like it's safest to use firefox nightly for WebIDE

> 
> Hey Sam, yeah, I generally try to keep everything up to date.  I'm currently
> running Firefox Developer Edition 37.0a2 (2015-02-18) for Ubuntu and 37.0a2
> (2013-02-23) on Windows and the CSS rules are still absent.  They used to
> work (and really well, at that), but mysteriously went away around 3.5 weeks
> ago...  Thanks for the heads-up tho.
Flags: needinfo?(yzenevich)
Hi Ross, I added comments in Gihub, looks really good, so once you update the PR, you should be good to take care of the tests. Thanks!
Hey Yura,

A few more questions on the PR regarding the hint loop.
Flags: needinfo?(yzenevich)
Okay, got the Hint loop taken care of as well as the unit tests.  Take a look at the PR (I left some comments/questions there) when you get a chance!
Hi Ross, I left comment, sorry if i missed anything, but i think it should be in its final shape. mark me for a11y-review once you address the last nits. Thanks!
Flags: needinfo?(yzenevich)
Comment on attachment 8567522 [details] [review]
[gaia] FunkTron:Bug1134097 > mozilla-b2g:master

Looks great (with final test comments addressed) ! Marking Alive for review. Thanks Ross!
Attachment #8567522 - Flags: review?(alive)
Attachment #8567522 - Flags: a11y-review+
Comment on attachment 8567522 [details] [review]
[gaia] FunkTron:Bug1134097 > mozilla-b2g:master

Don't forget to squash commits
Attachment #8567522 - Flags: review?(alive) → review+
Keywords: checkin-needed
http://docs.taskcluster.net/tools/task-graph-inspector/#2tUfC9D2RaCZ4BmfN9m7_A

The pull request failed to pass integration tests. It could not be landed, please try again.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment on attachment 8567522 [details] [review]
[gaia] FunkTron:Bug1134097 > mozilla-b2g:master

[Approval Request Comment] Adds hints for screen reader users
[Bug caused by] (feature/regressing bug #): improvement, not a bug
[User impact] if declined: the users will not get hints about how to activate elements on the screen.
[Testing completed]: unit and on device
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: https://github.com/mozilla-b2g/gaia/pull/28350/files#diff-832ba08de6e4e23b203d15ffd21706aa
Attachment #8567522 - Flags: approval-gaia-v2.2?
Attachment #8567522 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
There is a typo:

+accessibility-key-hint = Prese and hold to activate
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: