Migrate l10n_date.js to Intl/L20n

RESOLVED FIXED

Status

Firefox OS
Gaia::L10n
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: gandalf, Assigned: gandalf)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

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

3 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

3 years ago
Created attachment 8614602 [details] [review]
patch1
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Attachment #8614602 - Flags: review?(stas)
(Assignee)

Updated

3 years ago
Depends on: 1170973
(Assignee)

Comment 3

3 years ago
Gabriele, you may be interested in this for your work on Callscreen/Communications
(Assignee)

Updated

3 years ago
Depends on: 1171206
(Assignee)

Updated

3 years ago
Depends on: 1171856
(Assignee)

Updated

3 years ago
Depends on: 1172609
(Assignee)

Updated

3 years ago
Depends on: 1172621
(Assignee)

Updated

3 years ago
Depends on: 1172703, 1172699
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1015181
(Assignee)

Updated

2 years ago
See Also: → bug 1186262
(Assignee)

Comment 5

2 years ago
Poke for review :)
Flags: needinfo?(stas)
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

2 years ago
Commit: https://github.com/mozilla-b2g/gaia/commit/14e32276025b0310d3e89027320cf4b2a24cedfb
(Assignee)

Updated

2 years ago
Depends on: 1197019
(Assignee)

Updated

2 years ago
Depends on: 1197383
(Assignee)

Updated

2 years ago
Depends on: 1197454
(Assignee)

Comment 8

2 years ago
We're down to:

1) localeFormat: 85
2) fromNow: 6
(Assignee)

Updated

2 years ago
Depends on: 1203348
(Assignee)

Updated

2 years ago
Depends on: 1206306
(Assignee)

Updated

2 years ago
Depends on: 1206318
(Assignee)

Updated

2 years ago
Blocks: 1207044
(Assignee)

Updated

2 years ago
No longer blocks: 1207044
Depends on: 1207044
(Assignee)

Updated

2 years ago
Depends on: 1208889
(Assignee)

Updated

2 years ago
Depends on: 1209866
(Assignee)

Updated

2 years ago
Depends on: 1210252
(Assignee)

Updated

2 years ago
Depends on: 1211668
Created attachment 8674788 [details] [review]
[gaia] zbraniecki:1170963-remove-l10n_date.js > mozilla-b2g:master
(Assignee)

Comment 10

2 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 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 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 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 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

2 years ago
Flags: needinfo?(gandalf)
Attachment #8674788 - Flags: review?(borja.bugzilla) → review?(francisco)
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

2 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
(Assignee)

Updated

2 years ago
Blocks: 1212151
(Assignee)

Updated

2 years ago
Depends on: 1247360
(Assignee)

Updated

2 years ago
No longer depends on: 1209866
Created attachment 8722603 [details] [review]
[gaia] zbraniecki:1170963-remove-l10n_date > mozilla-b2g:master
(Assignee)

Comment 18

2 years ago
L10n Date removed: https://github.com/mozilla-b2g/gaia/commit/cc85139ab659fc2dbf0379052991a55c24861468
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.