Closed
Bug 1170963
Opened 11 years ago
Closed 10 years ago
Migrate l10n_date.js to Intl/L20n
Categories
(Firefox OS Graveyard :: Gaia::L10n, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: zbraniecki, Assigned: zbraniecki)
References
Details
Attachments
(3 external files)
The current model of l10n_date is relying on mozL10n.get, so we need to update that.
We also now have Intl, which should offload some of the tasks.
l10n_date provides:
1) localeDateString
2) localeTimeString
3) localeString
4) localeFormat
5) fromNow
6) relativeParts
1) localeDateString
function localeDateString(d) {
var options = {
month: 'numeric',
day: 'numeric',
year: 'numeric',
};
var formatter = new Intl.DateTimeFormat(undefined, options);
return formatter.format(d);
}
2) localeTimeString
function localeTimeString(d) {
var options = {
hour12: false,
hour: 'numeric',
minute: 'numeric',
second: 'numeric',
};
var formatter = new Intl.DateTimeFormat(undefined, options);
return formatter.format(d);
}
3) localeString
function localeString(d) {
var options = {
hour12: false,
weekday: 'short',
month: 'short',
day: 'numeric',
hour: 'numeric',
minute: 'numeric',
second: 'numeric',
year: 'numeric',
};
var formatter = new Intl.DateTimeFormat(undefined, options);
return formatter.format(d);
}
4) localeFormat
This is the most tricky. Intl's format works differently from localeFormat.
Instead of formatting a string like "Today is %I:%M %p", we ask just for the date, and we describe it using options.
I believe that it's better and it will also allow us to remove a lot of entities, but the new use raw Intl and define options only.
5) fromNow
will have to be asynchronous because it loads entities
6) relativeParts
this one may stay
| Assignee | ||
Comment 1•11 years ago
|
||
Uses:
1) localeDateString: 0
2) localeTimeString: 0
3) localeString: 0
4) localeFormat: 146
5) fromNow: 9
6) relativeParts: 1
My take:
Remove localeDateString, localeTimeString and localeString now.
Next, start porting uses of localeFormat to Intl.
p.s. Intl performance characteristic is different than l10n_date. new Intl.DateTimeFormat is costly, format is cheap. in l10n_date new object is cheap, format is costly. So as we are porting, we should make sure to minimize the new Intl.DateTimeFormat creation wherver possible.
| Assignee | ||
Comment 2•11 years ago
|
||
| Assignee | ||
Comment 3•11 years ago
|
||
Gabriele, you may be interested in this for your work on Callscreen/Communications
| Assignee | ||
Updated•11 years ago
|
Comment 6•11 years ago
|
||
Comment on attachment 8614602 [details] [review]
patch1
Sorry, I thought this was a different, more involved patch and I was afraid to look at it ;) r=me.
Flags: needinfo?(stas)
Attachment #8614602 -
Flags: review?(stas) → review+
| Assignee | ||
Comment 7•11 years ago
|
||
| Assignee | ||
Comment 8•11 years ago
|
||
We're down to:
1) localeFormat: 85
2) fromNow: 6
| Assignee | ||
Updated•11 years ago
|
Comment 9•11 years ago
|
||
| Assignee | ||
Comment 10•11 years ago
|
||
Comment on attachment 8674788 [details] [review]
[gaia] zbraniecki:1170963-remove-l10n_date.js > mozilla-b2g:master
Final patch to remove l10n_date.js and last lingering calls to it.
Stas - I'm asking for an overall review of the change
The changes in apps are minimal and trivial, but I'd like owners/peers to rubber stamp them so they know we're removing this file from their apps:
Borja - can you review the contacts part?
Gabriele - can you review the dialer/emergency call part?
Salva - can you review the cost control part?
Sam - can you review the FTU part?
Attachment #8674788 -
Flags: review?(stas)
Attachment #8674788 -
Flags: review?(sfoster)
Attachment #8674788 -
Flags: review?(salva)
Attachment #8674788 -
Flags: review?(gsvelto)
Attachment #8674788 -
Flags: review?(borja.bugzilla)
Comment 11•11 years ago
|
||
Comment on attachment 8674788 [details] [review]
[gaia] zbraniecki:1170963-remove-l10n_date.js > mozilla-b2g:master
I see `weekStartsOnMonday` is deprecated, [1]. Thanks for the work!
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1127770#c3
Flags: needinfo?(gandalf)
Attachment #8674788 -
Flags: review?(salva) → review+
Comment 12•11 years ago
|
||
Comment on attachment 8674788 [details] [review]
[gaia] zbraniecki:1170963-remove-l10n_date.js > mozilla-b2g:master
Good to see another legacy dependency go away!
Attachment #8674788 -
Flags: review?(gsvelto) → review+
Comment 13•11 years ago
|
||
Comment on attachment 8674788 [details] [review]
[gaia] zbraniecki:1170963-remove-l10n_date.js > mozilla-b2g:master
Yes, yes please. Nice work migrating all the apps to (moz)Intl beforehand; it makes this patch easy to understand! r=me.
Attachment #8674788 -
Flags: review?(stas) → review+
Comment 14•11 years ago
|
||
Comment on attachment 8674788 [details] [review]
[gaia] zbraniecki:1170963-remove-l10n_date.js > mozilla-b2g:master
Thanks for the heads-up. Btw do we still need the /shared/js/date_time_helper.js? It appears to provide a window.navigator.mozHour12 shim.
Attachment #8674788 -
Flags: review?(sfoster) → review+
| Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(gandalf)
Attachment #8674788 -
Flags: review?(borja.bugzilla) → review?(francisco)
Comment 15•10 years ago
|
||
Comment on attachment 8674788 [details] [review]
[gaia] zbraniecki:1170963-remove-l10n_date.js > mozilla-b2g:master
LGTM, tested on the phone and working perfectly.
Attachment #8674788 -
Flags: review?(francisco) → review+
| Assignee | ||
Comment 16•10 years ago
|
||
Thanks everyone!
Commit: https://github.com/zbraniecki/gaia/commit/95638548549acb98a2ab82d3242771f6f7d0e8d4
I'm leaving this bug open until we remove l10n_date from tv_apps - bug 1209866
Comment 17•10 years ago
|
||
| Assignee | ||
Comment 18•10 years ago
|
||
L10n Date removed: https://github.com/mozilla-b2g/gaia/commit/cc85139ab659fc2dbf0379052991a55c24861468
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•