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)
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
Comment 1•8 years ago
|
||
Assignee | ||
Comment 2•8 years ago
|
||
carrying over r+ from timdream and gasolin
Attachment #8703068 -
Flags: review+
Assignee | ||
Comment 3•8 years ago
|
||
Commit: https://github.com/mozilla-b2g/gaia/commit/65cbfa65eeec90494a48952b9b192c54ae950913
Assignee | ||
Updated•8 years ago
|
User Story: (updated)
Assignee | ||
Comment 4•8 years ago
|
||
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
Assignee | ||
Comment 5•8 years ago
|
||
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)
Assignee | ||
Comment 6•8 years ago
|
||
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 → ---
Comment 8•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8703067 -
Attachment is obsolete: true
Flags: needinfo?(gandalf)
Assignee | ||
Updated•8 years ago
|
Attachment #8703068 -
Attachment is obsolete: true
Assignee | ||
Comment 9•8 years ago
|
||
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 ago → 8 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?
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.
Comment 12•8 years ago
|
||
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)
Comment 15•8 years ago
|
||
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)
Comment 16•8 years ago
|
||
:gandalf, you want to keep an eye on bug 1246588 :)
Flags: needinfo?(apastor) → needinfo?(gandalf)
Assignee | ||
Comment 17•8 years ago
|
||
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)
Updated•8 years ago
|
Flags: needinfo?(wchen)
You need to log in
before you can comment on or make changes to this bug.
Description
•