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

RESOLVED FIXED

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: yzen, Assigned: eeejay)

Tracking

({access})

unspecified
ARM
Gonk (Firefox OS)
access
Dependency tree / graph

Firefox Tracking Flags

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

Details

Attachments

(5 attachments)

(Reporter)

Description

3 years ago
We need to make sure that when the screen reader is enabled, the user can only select languages that the screen reader supports.
(Assignee)

Updated

3 years ago
Assignee: nobody → eitan
Created attachment 8667431 [details] [review]
[gaia] eeejay:bug-1209050 > mozilla-b2g:master
(Assignee)

Updated

3 years ago
Attachment #8667431 - Flags: review?(yzenevich)
Attachment #8667431 - Flags: review?(sfoster)
Attachment #8667431 - Flags: review?(gandalf)
(Assignee)

Comment 2

3 years ago
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)

Updated

3 years ago
Blocks: 1199493
(Reporter)

Comment 4

3 years ago
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)
(Assignee)

Comment 6

3 years ago
(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.
(Assignee)

Updated

3 years ago
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+
(Reporter)

Comment 8

3 years ago
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+
(Assignee)

Comment 10

3 years ago
(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+
(Assignee)

Comment 13

3 years ago
(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
Last Resolved: 3 years ago
Flags: needinfo?(eitan)
Resolution: --- → FIXED
Created attachment 8670966 [details] [review]
[gaia] eeejay:bug-1209050-v22r > mozilla-b2g:v2.2r
(Assignee)

Updated

3 years ago
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)
(Assignee)

Comment 16

3 years ago
Rebased, thanks!
Flags: needinfo?(eitan)
This appears to have broken Gu tests on 2.2r:
https://treeherder.mozilla.org/logviewer.html#?job_id=9707&repo=mozilla-b2g37_v2_2r
Flags: needinfo?(eitan)
Created attachment 8672049 [details] [review]
[gaia] eeejay:bug-1209050-v22r-fix-unit-test > mozilla-b2g:v2.2r
(Assignee)

Comment 21

3 years ago
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)
(Assignee)

Comment 22

3 years ago
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)

Updated

3 years ago
status-b2g-v2.2r: --- → fixed
Keywords: checkin-needed
Hrm, ICS, L, and Flame-KK builds are all still broken. :\
Flags: needinfo?(eitan)
(Assignee)

Comment 25

3 years ago
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.
(Reporter)

Comment 30

3 years ago
Created attachment 8679793 [details] [review]
2.2r pull request

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

Updated

3 years ago
Keywords: checkin-needed
(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)
(Reporter)

Comment 33

3 years ago
(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)
(Reporter)

Comment 34

3 years ago
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
Created attachment 8680053 [details] [review]
[gaia] yzen:bug-1209050-follow-up > mozilla-b2g:v2.2r
(Reporter)

Comment 39

3 years ago
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
(Reporter)

Comment 41

3 years ago
Ah, looks like it's finally green \o/
You need to log in before you can comment on or make changes to this bug.