1024501 - Enable accessibility settings when the screen reader is enabled with quick toggle.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Yura Zenevich [:yzen] (please use "needinfo")]

Currently a11y settings are enabled when a check box is checked in the settings menu (they become visible in the top level settings). 

What we want is: when the user quick toggles the screen reader on the a11y settings should become visible in the top level settings (as if the dev settings check box is checked).

Comment 1

[Alexey Novak]

Is this a good first bug? I have not done any a11y contributions yet but I'm interested in this bug and eager to learn.
If no one is looking into it, I would love to take it.

Could you also suggest any files or any places in the code I should start looking first?

Thanks

Comment 2

[Yura Zenevich [:yzen] (please use "needinfo")]

Yes it is a good bug to learn things about how Settings, System work; also gaia-ui tests.

You should look into the Settings app and how UI corresponds to preferences being set. 

Also this https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/accessibility.js is where the volume button presses are handled.

In terms of gaia-ui tests this should be helpful:
https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/tests/accessibility/system/test_a11y_volume_buttons.py and there should be some functional settings tests where you can see how to check for settings being set and menu's being visible.

Comment 3

[Alexey Novak]

Thanks a lot for the links you provided. I will take a look and start working on the task.
Will keep you updated on my progress.

Comment 4

[Alexey Novak]

I have lots of questions.

1) It took me a while to find what is a proper key combination which is supposed to enable screen reader. First I started searching online (which was my mistake) and found different variations like 3 times up and then 3 times down or even 3 times power button press (like in this blog article http://yzen.github.io/2014/02/24/firefoxos-screen-reader-toggle.html)

Then I just looked at the code and finally found my answer in this Bug - https://bugzilla.mozilla.org/show_bug.cgi?id=957674  (it is (up then down) x 3 )

But still cannot activate screen reader in emulator since getting the following error in terminal:
|JavaScript error: http://system.gaiamobile.org:8080/js/accessibility.js, line 156: this.speechSynthesis is undefined| Any hints or advices with on how to deal with this?

2) I assume that the task of this bug is upon pressing key combination from (1) show/hide |Accessibility| panel inside |Settings App|, also flip the |Screen reader| toggle ON/OFF inside the Accessibility view. Is it correct?

3) Then I was looking into the code. After some poking around I found that the hide/show Accessibility option inside Settings app happens via generic function |onSettingsChange()| https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/modules/panel_utils.js#L345
First thought that https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/accessibility.js will call this function somehow in order to make panel visible does not sound right to me... on the other hand I cannot find another dedicated function/code which would hide/show |Accessibility| panel. Am I looking into the right direction?

4) In order to toggle |Screen reader| switch it seems that upon key combination in (1) I would need to call https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/accessibility.js#L36 is it a proper thought ?

5) I think label should say |Screen Reader| instead of |Screen reader| inside of the |Accessibility| view.

Comment 5

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Alexey Novak from comment #4)
> I have lots of questions.
> 
> 1) It took me a while to find what is a proper key combination which is
> supposed to enable screen reader. First I started searching online (which
> was my mistake) and found different variations like 3 times up and then 3
> times down or even 3 times power button press (like in this blog article
> http://yzen.github.io/2014/02/24/firefoxos-screen-reader-toggle.html)
> 
> Then I just looked at the code and finally found my answer in this Bug -
> https://bugzilla.mozilla.org/show_bug.cgi?id=957674  (it is (up then down) x
> 3 )
> 
> But still cannot activate screen reader in emulator since getting the
> following error in terminal:
> |JavaScript error: http://system.gaiamobile.org:8080/js/accessibility.js,
> line 156: this.speechSynthesis is undefined| Any hints or advices with on
> how to deal with this?

Yeah I think the phone build expects speechSynthesis to be present. Just to get further I suggest commenting lines 156 and 163 that call this.speechSynthesis.cancel(). We should open another bug that checks that it's present first.

> 
> 2) I assume that the task of this bug is upon pressing key combination from
> (1) show/hide |Accessibility| panel inside |Settings App|, also flip the
> |Screen reader| toggle ON/OFF inside the Accessibility view. Is it correct?

Toggle will be enabled automatically as it is bound to the settings, so no need for that. You only need to: after observing a change to 'accessibility.screanreader' setting change the setting responsible for screen reader settings visibility in the settings app.

> 
> 3) Then I was looking into the code. After some poking around I found that
> the hide/show Accessibility option inside Settings app happens via generic
> function |onSettingsChange()|
> https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/modules/
> panel_utils.js#L345
> First thought that
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/accessibility.
> js will call this function somehow in order to make panel visible does not
> sound right to me... on the other hand I cannot find another dedicated
> function/code which would hide/show |Accessibility| panel. Am I looking into
> the right direction?

So that code does not really matter for this bug. The visibility of the panel is controlled by the setting. It is mentioned here https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/tests/accessibility/settings/test_a11y_slider_visibility.py . They settings values are mapped based on the name of the toggle, checkbox etc: https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/elements/developer.html#L175

> 
> 4) In order to toggle |Screen reader| switch it seems that upon key
> combination in (1) I would need to call
> https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/
> accessibility.js#L36 is it a proper thought ?

No need for that. Will happen automatically.

> 
> 5) I think label should say |Screen Reader| instead of |Screen reader|
> inside of the |Accessibility| view.

Ya this looks inconsistent but it's not the scope of this bug. I am not 100% sure what the correct format should be.

Comment 6

[Alexey Novak]

I will try things you suggested.

What about the capitalization... when I had a chance to work on https://bugzilla.mozilla.org/show_bug.cgi?id=956645 for Dev Tools we used http://titlecapitalization.com/ in order to properly format any UI labels

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Alexey Novak from comment #6)
> I will try things you suggested.
> 
> What about the capitalization... when I had a chance to work on
> https://bugzilla.mozilla.org/show_bug.cgi?id=956645 for Dev Tools we used
> http://titlecapitalization.com/ in order to properly format any UI labels

feel free to open a bug for that.

Comment 8

[Eitan Isaacson [:eeejay]]

Re-aligning priorities with 2.1 accessibility goals.

Comment 9

[Alexey Novak]

Attachment: pull request for suggestions

Hey.

Here is a dummy pull request only to get your ideas or feedback. I spent more time with tests trying to comment/uncomment line by line and see what fails. Also tried to use scrolling in the gaia (which did not work for me whatever I tried), tried different code snippets from different test files....

But the bottom line (as far as I think is the problem) is line 46 in https://github.com/mozilla-b2g/gaia/pull/22192/files#diff-e8c1fd5051fd39ab432147cbb34cf71eR46

|accessibility.show-settings| has to be changed to True once volume button are pressed. It works if you launch FFOS but does not work in a test for some reason. I think it is the key to Accessibility panel visibility and the rest of checks. Tried many things including sleeps in case it is async problem... but nothing helps. If you have any ideas or suggestions that would be great.

There is not much I can do in those python tests :(

Comment 10

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Alexey Novak from comment #9)
> Created attachment 8462998 [details] [review]
> pull request for suggestions
> 
> Hey.
> 
> Here is a dummy pull request only to get your ideas or feedback. I spent
> more time with tests trying to comment/uncomment line by line and see what
> fails. Also tried to use scrolling in the gaia (which did not work for me
> whatever I tried), tried different code snippets from different test
> files....
> 
> But the bottom line (as far as I think is the problem) is line 46 in
> https://github.com/mozilla-b2g/gaia/pull/22192/files#diff-
> e8c1fd5051fd39ab432147cbb34cf71eR46
> 
> |accessibility.show-settings| has to be changed to True once volume button
> are pressed. It works if you launch FFOS but does not work in a test for
> some reason. I think it is the key to Accessibility panel visibility and the
> rest of checks. Tried many things including sleeps in case it is async
> problem... but nothing helps. If you have any ideas or suggestions that
> would be great.
> 
> There is not much I can do in those python tests :(

Hi Alex, so I managed to run the tests successfully. Here's what I've tried:

Think of this test as it being run in 2 contexts: System app (that handles volume stuff) and the Settings app. This means that when you launch the Settings app (which is for the most of your test), and you want simulate volume keys you need to do it in the context of a System app by first calling:

self.marionette.switch_to_frame()

Once you simulated your volume keys you need to switch back to the Settings app context by calling:

self.apps.switch_to_displayed_app()

Once you do the above steps your test sequence should run without any problems.

Comment 11

[Alexey Novak]

Attachment: Pull request

Thanks a lot for the feedback. This helps, although I have no idea what |self.marionette.switch_to_frame()| does exactly but it seems to be working. Please review and let me know if I need to change/improve anything.

Comment 12

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8463722 [details] [review]
Pull request

Thanks for the PR. I left some comments in Github. Also regarding the call to switch_to_frame: when you call it without any args, you go to the system frame. You should ask for a r? from a system peer or owner now.

Comment 13

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Yura Zenevich [:yzen] from comment #12)
> You should ask for a r? from a system peer or owner now.

Or rather once the comments are addressed.

Comment 14

[Alexey Novak]

Attachment: Pull Request

Updated code according to yzen comments

Comment 15

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8464048 [details] [review]
Pull Request

Not sure what does 'accessibility.show-settings' do but the code looks ok.

Comment 16

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #15)
> Comment on attachment 8464048 [details] [review]
> Pull Request
> 
> Not sure what does 'accessibility.show-settings' do but the code looks ok.

If it's set to true, Accessibility section will show up in the top level list in the settings app.

Comment 17

[Yura Zenevich [:yzen] (please use "needinfo")]

merged: https://github.com/mozilla-b2g/gaia/commit/577efee2558b0829c13919dbcb84fb1f581c2bb3

Thanks for the work!