Closed Bug 1209050 Opened 9 years ago Closed 9 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(feature-b2g:2.2r+, b2g-v2.2r fixed)

RESOLVED FIXED
feature-b2g 2.2r+
Tracking Status
b2g-v2.2r --- fixed

People

(Reporter: yzen, Assigned: eeejay)

References

Details

(Keywords: access)

Attachments

(5 files)

We need to make sure that when the screen reader is enabled, the user can only select languages that the screen reader supports.
Assignee: nobody → eitan
Attachment #8667431 - Flags: review?(yzenevich)
Attachment #8667431 - Flags: review?(sfoster)
Attachment #8667431 - Flags: review?(gandalf)
yzen for settings, sfoster for FTU, and gandalf for shared js.
Comment on attachment 8667431 [details] [review]
[gaia] eeejay:bug-1209050 > mozilla-b2g:master

I think :stas is a better person to look at this.
Attachment #8667431 - Flags: review?(gandalf) → review?(stas)
Holding off with r+ until the tests are added/pass
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.
Attachment #8667431 - Flags: review?(sfoster)
(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.
Attachment #8667431 - Flags: review?(sfoster)
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.
Attachment #8667431 - Flags: review?(sfoster) → review+
Comment on attachment 8667431 [details] [review]
[gaia] eeejay:bug-1209050 > mozilla-b2g:master

Settings part looks good. Thanks!
Attachment #8667431 - Flags: review?(yzenevich) → review+
Comment on attachment 8667431 [details] [review]
[gaia] eeejay:bug-1209050 > mozilla-b2g:master

r=me on the languages_list changes, thanks!
Attachment #8667431 - Flags: review?(stas) → review+
(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!
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!
Flags: needinfo?(eitan)
I believe we need it for RedTai/RedSquare
feature-b2g: --- → 2.2r+
(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
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(eitan)
Resolution: --- → FIXED
Keywords: checkin-needed
Hey Eitan, could you take a look at this, it tells me:

This branch has conflicts that must be resolved

Thanks!
Flags: needinfo?(eitan)
Rebased, thanks!
Flags: needinfo?(eitan)
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.
Flags: needinfo?(eitan)
Attachment #8672049 - Flags: review?(wkocher)
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.
Attachment #8672049 - Flags: review?(wkocher)
Hrm, ICS, L, and Flame-KK builds are all still broken. :\
Flags: needinfo?(eitan)
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.
Flags: needinfo?(eitan) → needinfo?(wkocher)
(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
Wes, do you know how to revert the first checkin from comment #17, i get errors when i try to revert it ?
The builds are green on the backout, for the record.
Attached file 2.2r pull request
Treeherder unit tests seemed to pass for this PR. Can we try landing it again?
(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 ?
Flags: needinfo?(yzenevich)
(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?
Flags: needinfo?(yzenevich) → needinfo?(cbook)
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
(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
Flags: needinfo?(cbook)
(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 :(
Flags: needinfo?(yzenevich)
talked to yura on irc and there will be a follow up PR to fix the bustage
I'm suspecting this is possibly due to some problematic JS. Added PR, Can we try this one?
Flags: needinfo?(yzenevich)
Keywords: checkin-needed
(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
Keywords: checkin-needed
Ah, looks like it's finally green \o/
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: