1224063 - Refactor System to use modern L10n API

Status: RESOLVED FIXED

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

Description

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

That mainly means we want to remove all `mozL10n.get` uses.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] zbraniecki:1224063-migrate-system-away-from-mozl10n-get-part1 > mozilla-b2g:master

Comment 2

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

Comment on attachment 8694109 [details] [review]
[gaia] zbraniecki:1224063-migrate-system-away-from-mozl10n-get-part1 > mozilla-b2g:master

Fernardo, can you review my patch?

Comment 3

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

Comment on attachment 8694109 [details] [review]
[gaia] zbraniecki:1224063-migrate-system-away-from-mozl10n-get-part1 > mozilla-b2g:master

Erienne, can you review this patch as well pls?

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8694109 [details] [review]
[gaia] zbraniecki:1224063-migrate-system-away-from-mozl10n-get-part1 > mozilla-b2g:master

r=me on the identity, payments and mobile ID parts.

Thanks!

Comment 5

[GitHub Autolander Bot]

Attachment: [gaia] zbraniecki:1224063-migrate-system-away-from-mozl10n-get-part2 > mozilla-b2g:master

Comment 6

[Etienne Segonzac (:etienne)]

Comment on attachment 8694109 [details] [review]
[gaia] zbraniecki:1224063-migrate-system-away-from-mozl10n-get-part1 > mozilla-b2g:master

Thanks! It was indeed a really manageable patch :)

r=me for everything except the migrator part which I know _nothing_ about, hoping that Sam will.

Comment 7

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

Comment on attachment 8694109 [details] [review]
[gaia] zbraniecki:1224063-migrate-system-away-from-mozl10n-get-part1 > mozilla-b2g:master

I dont know much about settings_migrator.js, but I do know we are using the Intl api for 12/14 hour concerns now so removing these tests seems like the right thing to do, so I'll add my r+ FWIW.

Comment 8

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

Thanks!

Commit of part1: https://github.com/mozilla-b2g/gaia/commit/89349eb99f91ba91973432f7a21058d8d2251075

Part2 is a bit more complex since I'm moving ValuePicker and ValueSelector and it requires a bit more serious refactor to make it work using Intl and L20n.

Comment 9

[GitHub Autolander Bot]

Attachment: [gaia] KevinGrandon:test_backout > mozilla-b2g:master

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8698221 [details] [review]
[gaia] KevinGrandon:test_backout > mozilla-b2g:master

Oops - just testing, ignore this for now. Seeing some weirdness with the tests, and wanted to see if this caused it.

Comment 11

[Nigel Babu [:nigelb]]

Backed out in https://github.com/mozilla-b2g/gaia/commit/c58c206b43dc04c9648b0e82d784e0f375b3d1ec for Gij(21) failures like this: https://treeherder.mozilla.org/logviewer.html#?job_id=3549013&repo=b2g-inbound

Comment 12

[GitHub Autolander Bot]

Attachment: [gaia] zbraniecki:1224063-migrate-system-away-from-mozl10n-get-part1 > mozilla-b2g:master

Comment 13

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

Ok, it seems that there already was an intermittent bug 1192843 and this patch made it more permanent.

The way I'm fixing it is two-fold -

a) I'm making sure that elements in marionette tests are visible to screen reader
b) I'm artificially keeping 'OK' string in alert dialog

The reason for b) is that it seems that in marionette context l10n.js' MutationObserver is not running and setting data-l10n-id on okButton does not translate it. Without manually set string in the template, marionette test fails because it cannot reach to the element without value.

In real world l10n.js works and the element is properly localized.

This should also fix bug 1192843.

Comment 14

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

Relanded part1: https://github.com/mozilla-b2g/gaia/commit/654089cda5844be22fc29473e78d3bfa32122fe8

Fixed the Gij intermittent test.

Comment 15

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

Comment on attachment 8699412 [details] [review]
[gaia] zbraniecki:1224063-migrate-system-away-from-mozl10n-get-part1 > mozilla-b2g:master

Ok, this is not going to be fun.

Tim, the second patch is probably the most entangled one. I'm refactoring pieces of very ancient code here - most of it dates back to 2012.

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.

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8699412 [details] [review]
[gaia] zbraniecki:1224063-migrate-system-away-from-mozl10n-get-part1 > mozilla-b2g:master

I assume you want me to review part 2? Part 1 has already merged.

I did not work on value selector directly so my review will not be very helpful, still I tried to spot a few stuff and leave some comment there. Otherwise this looks good.

Comment 17

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8697576 [details] [review]
[gaia] zbraniecki:1224063-migrate-system-away-from-mozl10n-get-part2 > mozilla-b2g:master

You should probably land this on under another bug #?

Comment 18

[[e-mail]]

Comment on attachment 8699412 [details] [review]
[gaia] zbraniecki:1224063-migrate-system-away-from-mozl10n-get-part1 > mozilla-b2g:master

Looks good, thanks!

Comment 19

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

Filed as bug 1235975

Comment 20

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

And this is complete now!