1209050 - [Accessibility] Filter language selection to only lanugages that screen reader supports.

Status: RESOLVED FIXED

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

Description

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

We need to make sure that when the screen reader is enabled, the user can only select languages that the screen reader supports.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] eeejay:bug-1209050 > mozilla-b2g:master

Comment 2

[Eitan Isaacson [:eeejay]]

yzen for settings, sfoster for FTU, and gandalf for shared js.

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8667431 [details] [review]
[gaia] eeejay:bug-1209050 > mozilla-b2g:master

I think :stas is a better person to look at this.

Comment 4

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

Holding off with r+ until the tests are added/pass

Comment 5

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8667431 [details] [review]
[gaia] eeejay:bug-1209050 > mozilla-b2g:master

See my PR https://github.com/eeejay/gaia/pull/3 for a refactoring suggestion. Just trying to stop the bleeding in the FTU while I make a plan for a larger refactoring. Otherwise with a unit test for the new observer, this should be fine.

Comment 6

[Eitan Isaacson [:eeejay]]

(In reply to Sam Foster [:sfoster] from comment #5)
> Comment on attachment 8667431 [details] [review]
> [gaia] eeejay:bug-1209050 > mozilla-b2g:master
> 
> See my PR https://github.com/eeejay/gaia/pull/3 for a refactoring
> suggestion. Just trying to stop the bleeding in the FTU while I make a plan
> for a larger refactoring. Otherwise with a unit test for the new observer,
> this should be fine.

Cool. Added your suggestions. Added an uninit which is needed for tests. And added tests.

Comment 7

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8667431 [details] [review]
[gaia] eeejay:bug-1209050 > mozilla-b2g:master

r+ for the FTU bits. Just a couple nits in the PR - the quotes one I think will take care of the jshint failure. But if you've not already, you should npm install to ensure the pre-commit hook is working for you as it should have caught this.

Comment 8

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

Comment on attachment 8667431 [details] [review]
[gaia] eeejay:bug-1209050 > mozilla-b2g:master

Settings part looks good. Thanks!

Comment 9

[Staś Małolepszy :stas]

Comment on attachment 8667431 [details] [review]
[gaia] eeejay:bug-1209050 > mozilla-b2g:master

r=me on the languages_list changes, thanks!

Comment 10

[Eitan Isaacson [:eeejay]]

(In reply to Sam Foster [:sfoster] from comment #7)
> Comment on attachment 8667431 [details] [review]
> [gaia] eeejay:bug-1209050 > mozilla-b2g:master
> 
> r+ for the FTU bits. Just a couple nits in the PR - the quotes one I think
> will take care of the jshint failure. But if you've not already, you should
> npm install to ensure the pre-commit hook is working for you as it should
> have caught this.

Looks like I fixed that locally but didn't stage, so the pre-commit hook didn't catch it. Oops!

Comment 11

[Staś Małolepszy :stas]

Hey :eeejay, I have another patch to languages_list in bug 1204152 and I was hoping we could coordinate the landings to avoid bitrot.  I'll be happy to wait for you to land your pr here.  Is there much work left on your side?  Thanks!

Comment 12

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

I believe we need it for RedTai/RedSquare

Comment 13

[Eitan Isaacson [:eeejay]]

(In reply to Staś Małolepszy :stas from comment #11)
> Hey :eeejay, I have another patch to languages_list in bug 1204152 and I was
> hoping we could coordinate the landings to avoid bitrot.  I'll be happy to
> wait for you to land your pr here.  Is there much work left on your side? 
> Thanks!

I believe it landed! I thought autolander would comment and close this bug..

https://github.com/mozilla-b2g/gaia/commit/1673ea1abcd93a3a7b40731f9113398743bc887b

Comment 14

[GitHub Autolander Bot]

Attachment: [gaia] eeejay:bug-1209050-v22r > mozilla-b2g:v2.2r

Comment 15

[Carsten Book [:Tomcat]]

Hey Eitan, could you take a look at this, it tells me:

This branch has conflicts that must be resolved

Thanks!

Comment 16

[Eitan Isaacson [:eeejay]]

Rebased, thanks!

Comment 17

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/5d504cbac30315c93a320f157a09e3d087363723

thanks eitan!

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This appears to have broken Gu tests on 2.2r:
https://treeherder.mozilla.org/logviewer.html#?job_id=9707&repo=mozilla-b2g37_v2_2r

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Also possible this broke some b2g builds: https://treeherder.mozilla.org/logviewer.html#?job_id=9700&repo=mozilla-b2g37_v2_2r

Comment 20

[GitHub Autolander Bot]

Attachment: [gaia] eeejay:bug-1209050-v22r-fix-unit-test > mozilla-b2g:v2.2r

Comment 21

[Eitan Isaacson [:eeejay]]

Comment on attachment 8672049 [details] [review]
[gaia] eeejay:bug-1209050-v22r-fix-unit-test > mozilla-b2g:v2.2r

Big oops. This pull requests simplifies the unit test and doesn't rely on an event that does not exist in this branch.

Comment 22

[Eitan Isaacson [:eeejay]]

Comment on attachment 8672049 [details] [review]
[gaia] eeejay:bug-1209050-v22r-fix-unit-test > mozilla-b2g:v2.2r

Actually, I'll forego review since this is a simple fix to a burning tree.

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Merged: https://github.com/mozilla-b2g/gaia/commit/602948527518f60160affb99457f789079249f85

Comment 24

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Hrm, ICS, L, and Flame-KK builds are all still broken. :\

Comment 25

[Eitan Isaacson [:eeejay]]

I give up.. tried getting a local failure with no luck. I'll be AFK for 2 weeks. Do you want to pull this and see if it remedies things? I don't know what is causing the failures.

Comment 26

[Carsten Book [:Tomcat]]

(In reply to Wes Kocher (:KWierso) from comment #23)
> Merged:
> https://github.com/mozilla-b2g/gaia/commit/
> 602948527518f60160affb99457f789079249f85

reverted this to see if this helps

Comment 27

[Carsten Book [:Tomcat]]

Wes, do you know how to revert the first checkin from comment #17, i get errors when i try to revert it ?

Comment 28

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I believe I backed out everything in
https://github.com/mozilla-b2g/gaia/commit/156a48db12b00d38d939317d09e12d6fb599375e
https://github.com/mozilla-b2g/gaia/commit/44e6ab1a9378a147036658473fb839951ed9ea51
https://github.com/mozilla-b2g/gaia/commit/c69ce313d130561d4f5766dad339d597c72e8d4d

Comment 29

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

The builds are green on the backout, for the record.

Comment 30

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

Attachment: 2.2r pull request

Treeherder unit tests seemed to pass for this PR. Can we try landing it again?

Comment 31

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/899ef5efc4f969821a861a8708e281309e0c1f09

Comment 32

[Carsten Book [:Tomcat]]

(In reply to Yura Zenevich [:yzen] from comment #30)
> Created attachment 8679793 [details] [review]
> 2.2r pull request
> 
> Treeherder unit tests seemed to pass for this PR. Can we try landing it
> again?

and this broke 2.2r again

https://treeherder.mozilla.org/logviewer.html#?job_id=11797&repo=mozilla-b2g37_v2_2r

yura can you fix this or revert ?

Comment 33

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

(In reply to Carsten Book [:Tomcat] from comment #32)
> (In reply to Yura Zenevich [:yzen] from comment #30)
> > Created attachment 8679793 [details] [review]
> > 2.2r pull request
> > 
> > Treeherder unit tests seemed to pass for this PR. Can we try landing it
> > again?
> 
> and this broke 2.2r again
> 
> https://treeherder.mozilla.org/logviewer.html#?job_id=11797&repo=mozilla-
> b2g37_v2_2r
> 
> yura can you fix this or revert ?

Is this really caused by the Gaia changes?

Comment 34

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

Perhaps we can re-trigger it?
I noticed the original broken build had similar build errors:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-b2g37_v2_2r&revision=6f41eb86b2bf

Comment 35

[Carsten Book [:Tomcat]]

(In reply to Yura Zenevich [:yzen] from comment #34)
> Perhaps we can re-trigger it?
> I noticed the original broken build had similar build errors:
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-
> b2g37_v2_2r&revision=6f41eb86b2bf

clobbered the tree and retriggered the builds now

Comment 36

[Carsten Book [:Tomcat]]

(In reply to Carsten Book [:Tomcat] from comment #35)
> (In reply to Yura Zenevich [:yzen] from comment #34)
> > Perhaps we can re-trigger it?
> > I noticed the original broken build had similar build errors:
> > https://treeherder.mozilla.org/#/jobs?repo=mozilla-
> > b2g37_v2_2r&revision=6f41eb86b2bf
> 
> clobbered the tree and retriggered the builds now

too bad no success builds still busted :(

Comment 37

[Carsten Book [:Tomcat]]

talked to yura on irc and there will be a follow up PR to fix the bustage

Comment 38

[GitHub Autolander Bot]

Attachment: [gaia] yzen:bug-1209050-follow-up > mozilla-b2g:v2.2r

Comment 39

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

I'm suspecting this is possibly due to some problematic JS. Added PR, Can we try this one?

Comment 40

[Carsten Book [:Tomcat]]

(In reply to Yura Zenevich [:yzen] from comment #39)
> I'm suspecting this is possibly due to some problematic JS. Added PR, Can we
> try this one?

landed as https://github.com/mozilla-b2g/gaia/commit/f79524d38ea8eb89a3207ef1b1c9efd8e44afe02

Comment 41

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

Ah, looks like it's finally green \o/