[B2G][SMS] support 24 hour time format

VERIFIED FIXED in 2.1 S3 (29aug)

Status

VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: hochang, Assigned: azasypkin)

Tracking

unspecified
2.1 S3 (29aug)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(feature-b2g:2.1)

Details

(Whiteboard: [p=1])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
There will be a setting toggle to let user choose between 12/24 hour time format in v2.1. User story and settings spec: Bug 903683

For each app, the tasks to do after toggle is added would be:
1.Read/listen to the time format toggle value/change event.

2.Input the corresponding time format to existing mozL10n.DateTimeFormat() function. Currently the input time format should be a fixed 12 hour 
shortTimeFormat = %I:%M %p , line 201 of http://goo.gl/DR4CNe. There will be a 24 hour time format added.

3.Show the return of the function as it's doing now.
Stas, Gandalf, could this be somehow done in the l10n_date library instead?
Flags: needinfo?(stas)
Flags: needinfo?(gandalf)
We specifically wanted to avoid the dependency on mozSettings for l10n.js and l10n_data.js.  See bug 1013929 and https://groups.google.com/forum/#!msg/mozilla.dev.gaia/LSL5d3EOPAM/_Gtmc-B1DWgJ for examples.

Does this bug mean that any app which wants to show dates and times to the user will require the 'settings' permission and thus will need to be certified?

A few thoughts on the topic itself:  I think it would be good to start to bring mozL10n.DateTimeFormat closer to the standard Intl.DateTimeFormat.  Intl.DateTimeFormat takes an 'options' object as an optional second argument, which can have a boolean 'hour12' option set:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat#Parameters

We could use this to control the behavior of localeFormat.  I think this would require us to have translations for both 12-hour and 24-hour variants, in all locales. E.g. in English, instead of:

  shortTimeFormat = %I:%M %p

we'd have:

  shortTimeFormat12 = %I:%M %p
  shortTimeFormat24 = %H:%M

However, this doesn't work well with the current coding pattern which looks like the following and explicitly asks for the translation of "shortTimeFormat":

  var f = new navigator.mozL10n.DateTimeFormat(null, { hour12: false });
  var format = _('shortTimeFormat');
  element.textContent = f.localeFormat(d, format);

Not to mention that we'd like to move away from inserting translations as textContent or innerHTML and use Mutation Observers and setAttribute('data-l10n-{id,args}') instead.

We'll also need a way to react to the changes of the preference and retranslate and rerender the elements which have dates and times in them.

Gandalf, any thoughts on this from your side?
Flags: needinfo?(stas)
It looks like this will be a common problem for many apps.  I just realized bug 903683 was filed to track this in all of Gaia.  Let's discuss there?
Flags: needinfo?(gandalf)
yep, the meta bug is a better location, thanks
QA Whiteboard: [COM=Gaia::SMS]
Test steps:

1. Open settings app
2. change the Time format settings in Settings > Date & Time > Time Format panel

Expect:

The time format in sms app will be changed.


here's the patch steps to apply correct time format:

1. include navigator.mozHour12 API shim in gaia app's html

```
<script defer src="shared/js/date_time_helper.js"></script>
```

(currently we still need at least `settings: readonly` permission in manifest, which might changed in next release)


2. detect proper time format via mozHour12 API, and get correct time format string from new 'shortTimeFormat12'/'shortTimeFormat24'.

```
var timeFormat = window.navigator.mozHour12 ? _('shortTimeFormat12') : _('shortTimeFormat24');
```

3. call mozL10n DateTimeFormat to localize it


4. listen to 'timeformatchange' event and localize time strings when its triggered


The full usage is at [3](WIP) for reference.


[3] https://github.com/mozilla-b2g/gaia/pull/22757/files
QA Contact: echang
Blocks: 1055470
Whiteboard: [p=1]
Target Milestone: --- → 2.1 S3 (29aug)
Assignee: nobody → azasypkin
Status: NEW → ASSIGNED
Created attachment 8478109 [details] [review]
GitHub pull request URL

Hey Steve, could you please review this patch? I've found few minor localization issues while working on this, so this patch contains fixes for them too.
Attachment #8478109 - Flags: review?(schung)
(In reply to Oleg Zasypkin [:azasypkin] from comment #6)
> Created attachment 8478109 [details] [review]
> GitHub pull request URL
> 
> Hey Steve, could you please review this patch? I've found few minor
> localization issues while working on this, so this patch contains fixes for
> them too.

Thanks for the addtional bugs fixing in the patch! I let my questions and comments on github but most of them just some suggestions. Still keep the flag because I think you could reply/fix right away :)
(In reply to Steve Chung [:steveck] from comment #7)
> (In reply to Oleg Zasypkin [:azasypkin] from comment #6)
> > Created attachment 8478109 [details] [review]
> > GitHub pull request URL
> > 
> > Hey Steve, could you please review this patch? I've found few minor
> > localization issues while working on this, so this patch contains fixes for
> > them too.
> 
> Thanks for the addtional bugs fixing in the patch! I let my questions and
> comments on github but most of them just some suggestions. Still keep the
> flag because I think you could reply/fix right away :)

Updated PR, thanks! :)
Comment on attachment 8478109 [details] [review]
GitHub pull request URL

r=me, thanks for the great effort on huge one point! Seems we underestimated this issue, maybe it would be more precise to create another issue for localization helper ;)
Attachment #8478109 - Flags: review?(schung) → review+
(In reply to Steve Chung [:steveck] from comment #9)
> Comment on attachment 8478109 [details] [review]
> GitHub pull request URL
> 
> r=me, thanks for the great effort on huge one point! Seems we underestimated
> this issue, maybe it would be more precise to create another issue for
> localization helper ;)

Thanks!

Master: https://github.com/mozilla-b2g/gaia/commit/ae00a2bb5950c4a63e96b2ddb08928c24f9912ab
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Verified User story is fixed. 24 hour format string shown in messages.

Gaia      2be78d83a760fa3b9638fe51c266b442d14597f1
Gecko     https://hg.mozilla.org/mozilla-central/rev/1db35d2c9a2f
BuildID   20140831160203
Version   34.0a1
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230
Status: RESOLVED → VERIFIED
Duplicate of this bug: 959449
You need to log in before you can comment on or make changes to this bug.