Closed Bug 1235975 Opened 8 years ago Closed 8 years ago

Refactor l10n/i18n related pieces of ValuePicker/ValueSelector/InputParser/DatePicker

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

(Keywords: qawanted)

User Story

The scope of this patch:

 - Make ValuePicker work with L10n IDs and raw strings
 - Remove the use of properietary toLocaleFormat
 - Remove the use of mozL10n.get
 - Remove the reliance on values being western arabic digits (`getSelectedDisplayedText` and so on)
 - Conform the InputParser to input type=time|date norms (HH:MM, not h:MM)
 - Moves the hardcoded "0" padding to Intl.NumberFormat

Fred: there's a small piece of the patch that touches Settings. Hopefully nothing controversial :)

Testing completed:

 - I tested Settings, Calendar event add/modify, Clock alarm set/modify, Contacts add/modify.
 - I updated tests
 - I tested against en-US, ar, fr, de, pl, it, and ru
 - I tested on two devices

It *seems* to me that this patch works well with all tested apps without any changes to the API use so I'm hopeful it will be a smooth landing, but since it's a very old code very deep in the System I'd like to land it early in the cycle.

Attachments

(1 file, 2 obsolete files)

Spin-off from bug 1224063
Assignee: nobody → gandalf
Blocks: 1224063
Attached file pull request (obsolete) —
carrying over r+ from timdream and gasolin
Attachment #8703068 - Flags: review+
User Story: (updated)
Naoki, heads up for QA - this may regress something. I did my best to test everything, but I doubt I really did test all scenarios.
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(nhirata.bugzilla)
Resolution: --- → FIXED
Also, :yzen, with this patch you may adjust the aria-valuetext for AM/PM - https://github.com/mozilla-b2g/gaia/blob/master/shared/locales/date/date.en-US.properties#L241

My naive thinking is that it would be better to be more descriptive, but I'll leave that to an expert :)
Flags: needinfo?(yzenevich)
Alberto, when you're merging systems, please, use this version of the picker/selector. :)
Flags: needinfo?(apastor)
I had to revert this in https://github.com/mozilla-b2g/gaia/commit/d7ca8344dfc8fe25eb4081b7404a568ffb8b9e62 because this made date_time_test.js fail very frequently (but not permanently):

https://treeherder.mozilla.org/logviewer.html#?job_id=3653842&repo=b2g-inbound
Status: RESOLVED → REOPENED
Flags: needinfo?(gandalf)
Resolution: FIXED → ---
Attachment #8703067 - Attachment is obsolete: true
Flags: needinfo?(gandalf)
Attachment #8703068 - Attachment is obsolete: true
Sorry for trouble Wes! Relanded with an updated test: https://github.com/mozilla-b2g/gaia/commit/2d6d3d1e6d71a5adf51cf25291b0d4aa05cf99d1

Basically, the old test compared full Date objects which meant that if there was a millisecond diff it would fail. Now we test only the significant bits.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Doesn't sound like this touches on time zone changes?  - we should probably check this though I think some things break regardless of this fix.

Other things to look at are dialer log, message logs, email, camera meta-data, video meta-data and Usage dates and databases after update to make sure things don't break...  William can you get someone to look at this on your monday?

Kevin, can you get some peeps to do exploratory testing on this on monday please?
Flags: needinfo?(wchen)
Flags: needinfo?(ktucker)
Keywords: qawanted
Oops.  probably helps to look at the patch first.  it's related to the picker itself.

William, Kevin, please take a look at any place that has a time picker.
Looks like it works. However the valuetext attribute should be applied on the same element as the one with role spinbutton attribute. Otherwise it is never picked up by any screen reader.
Flags: needinfo?(yzenevich)
Gandalf, please see comment 12.
Flags: needinfo?(gandalf)
Thanks!

Filed bug 1237139 for that.
Flags: needinfo?(gandalf)
I tested all the pickers in the FTU, Settings, Calendar, Contacts and Clock apps and did not see an issue. 

Device: Aries 2.6
Build ID: 20160111111358
Gaia: 260e51a4262f75341e037e583dfc8f6835b5ab31
Gecko: 5acc2a44834ce0614f98466475e674517daf0041
Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56
Version: 46.0a1 (2.6)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:46.0) Gecko/46.0 Firefox/46.0
Flags: needinfo?(ktucker)
:gandalf, you want to keep an eye on bug 1246588 :)
Flags: needinfo?(apastor) → needinfo?(gandalf)
Yeah! It should be mostly a straightforward copy&paste, I CC'ed myself there.
Flags: needinfo?(gandalf)
Clearing my name as this is a done deal.  We've verified it after the patch landed I believe.
Flags: needinfo?(nhirata.bugzilla)
Flags: needinfo?(wchen)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: