1035763 - [B2G][Dialer] support 24 hour time format

Status: VERIFIED FIXED

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

Description

[howie [:howie]]

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.

Comment 1

[[e-mail]]

Test steps:

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

Expect:

The time format in dialer 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

Comment 2

[Doug Sherk (:drs) (inactive)]

Places we display this that we know of:
* Call log
* Call screen while in lock screen

Comment 3

[Doug Sherk (:drs) (inactive)]

I'm going to double our estimate here due to lack of familiarity with it. Sorry for the spam as we changed this a few times during sprint planning.

Comment 4

[Johan Lorenzo [:jlorenzo]]

Removing the in-moztrap flag. This is already handled in bug 903683 comment 90.

Comment 5

[Tamara Hills [:thills]]

Attachment: 1035763.diff

Here is a WIP.  Need feedback on call_log.js... there are some notes in the code there explaining.

Thanks,

-tamara

Comment 6

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8477115 [details] [diff] [review]
1035763.diff

Review of attachment 8477115 [details] [diff] [review]:
-----------------------------------------------------------------

I think this is overall a good approach. I gave comments as though this is a review, but I'm setting the flag as though this is feedback.

I think Anthony is going to give feedback as well. One thing I wonder about is if the call/contact information page we're introducing in bug 1047353 will need to be tied into this as well. Perhaps Anthony can shed some light on that. I don't think it would be harmful to just write bug 1047353's mozHour12 support into the patch there instead of setting up weird dependencies.

::: apps/callscreen/index.html
@@ +42,4 @@
>      <script defer type="application/javascript" src="/shared/js/dialer/font_size_manager.js"></script>
>      <script defer type="application/javascript" src="/shared/js/dialer/voicemail.js"></script>
>      <script defer type="application/javascript" src="/shared/js/bluetooth_helper.js"></script>
> +    <script defer type="application/javascript" src="/shared/js/date_time_helper.js"></script>

I would group this with contact_photo_helper.js, font_size_utils.js, etc.

::: apps/callscreen/js/call_screen.js
@@ -447,4 @@
>    showClock: function cs_showClock(now) {
>      LazyL10n.get(function localized(_) {
>        var f = new navigator.mozL10n.DateTimeFormat();
> -      var timeFormat = _('shortTimeFormat').replace('%p', '<span>%p</span>');

Removing this span is liable to cause a regression, but after poking around, I couldn't see anywhere that it's used. Anthony doesn't know either. We should test every callscreen state to verify.

::: apps/communications/dialer/index.html
@@ +45,4 @@
>      <!-- for perf-measurement related utilities -->
>      <script defer src='/shared/js/performance_testing_helper.js'></script>
>  
> +    <script defer src='/shared/js/date_time_helper.js'></script>

This isn't a perf-measurement related utility, so it should be moved. I would group it with lazy_loader.js, font_size_utils.js, etc.

::: apps/communications/dialer/js/call_log.js
@@ +138,5 @@
>        self._dbupgrading = false;
>        self.render();
>      };
> +
> +    window.addEventListener('timeformatchange', self.updateCallTimes);

This should go inside the LazyL10n.get block above, after all the other event listeners are bound.

Also, this should be `self.updateCallTimes.bind(self)`.

@@ +143,5 @@
> +  },
> +
> +  updateCallTimes: function cl_updateCallTimes() {
> +    /* ---- NEED FEEDBACK-----
> +     * OPTION 1: This just updates the presentation of the time stamp.

I think it's fine to go with option 1 here.

@@ +148,5 @@
> +     * OPTION 2: Is to call the render function again and reload everything.
> +     *  -- probably slower, but maybe better maintenance wise.  Not able to get
> +     *  -- option 2 working as of yet.
> +     */
> +    var items = CallLog.callLogContainer.getElementsByClassName('log-item');

You can use `this` instead of `CallLog` here.

Also, everywhere else, we use `querySelectorAll('.log-item')`. I'd rather be consistent.

@@ +149,5 @@
> +     *  -- probably slower, but maybe better maintenance wise.  Not able to get
> +     *  -- option 2 working as of yet.
> +     */
> +    var items = CallLog.callLogContainer.getElementsByClassName('log-item');
> +    for(var i = 0; i < items.length; i++) {

This is feedback so I'm being a bit nit-picky, but there should be a space between `for` and `(`.

@@ +150,5 @@
> +     *  -- option 2 working as of yet.
> +     */
> +    var items = CallLog.callLogContainer.getElementsByClassName('log-item');
> +    for(var i = 0; i < items.length; i++) {
> +      var timestamp = items[i].getAttribute('data-timestamp');

Might want to save `items[i]` into a variable like `item` and use that instead.

@@ +152,5 @@
> +    var items = CallLog.callLogContainer.getElementsByClassName('log-item');
> +    for(var i = 0; i < items.length; i++) {
> +      var timestamp = items[i].getAttribute('data-timestamp');
> +      var newTimestamp = Utils.prettyDate(parseInt(timestamp)) + ' ';
> +      var callTime = items[i].getElementsByClassName('call-time');

You can use `item.querySelector('.call-time')` instead. This is preferable since it's more clear on the initial glance that we expect there to only be one element matching this.

@@ +153,5 @@
> +    for(var i = 0; i < items.length; i++) {
> +      var timestamp = items[i].getAttribute('data-timestamp');
> +      var newTimestamp = Utils.prettyDate(parseInt(timestamp)) + ' ';
> +      var callTime = items[i].getElementsByClassName('call-time');
> +      callTime[0].textContent = newTimestamp;

This inner block should be refactored out into another function and shared with createGroup().

::: shared/js/dialer/utils.js
@@ +3,1 @@
>  var Utils = {

You should remove this from xfail.list and fix the linting errors here in a separate patch.

@@ +8,2 @@
>      var dtf = new navigator.mozL10n.DateTimeFormat();
> +    return dtf.localeFormat(new Date(time), timeFormat);

This needs a new test for each of the time formats.

@@ +10,3 @@
>    },
>  
>    headerDate: function ut_headerDate(time) {

This function needs to be refactored to be clearer. In particular, the return statement is fairly unclear. It also needs some tests.

@@ +10,5 @@
>    },
>  
>    headerDate: function ut_headerDate(time) {
>      var _ = navigator.mozL10n.get;
> +    var timeFormat = window.navigator.mozHour12 ? _('shortTimeFormat12') :

This variable isn't used.

@@ +16,3 @@
>      var dtf = new navigator.mozL10n.DateTimeFormat();
>      var today = _('today');
>      var yesterday = _('yesterday');

We shouldn't pre-translate these. We can translate them as-needed.

@@ +21,2 @@
>      if (isNaN(day_diff))
>        return '(incorrect date)';

This should be localized. Please add it to date.en-US.properties and use that.

@@ +21,3 @@
>      if (isNaN(day_diff))
>        return '(incorrect date)';
>      if (day_diff < 0 || diff < 0) {

I don't think this block is needed at all. Future dates should fall through to the return statement at the end.

::: shared/locales/date/date.en-US.properties
@@ +202,5 @@
>  shortTimeFormat12 = %I:%M %p
>  shortTimeFormat24 = %H:%M
>  shortDateTimeFormat = %x %I:%M %p
> +shortDateTimeFormat12 = %x %I:%M %p
> +shortDateTimeFormat24 = %x %H:%M

These shouldn't be needed.

Comment 7

[Doug Sherk (:drs) (inactive)]

Also, please post patches with 8 lines of context. If you're exporting with git, you can use the -U8 flag in `git diff` or `git show`.

Comment 8

[Anthony Ricaud (:rik)]

Comment on attachment 8477115 [details] [diff] [review]
1035763.diff

Review of attachment 8477115 [details] [diff] [review]:
-----------------------------------------------------------------

::: apps/communications/dialer/index.html
@@ +45,4 @@
>      <!-- for perf-measurement related utilities -->
>      <script defer src='/shared/js/performance_testing_helper.js'></script>
>  
> +    <script defer src='/shared/js/date_time_helper.js'></script>

This actually needs to be lazy loaded. It isn't used on the first screen.

::: apps/communications/dialer/js/call_log.js
@@ +151,5 @@
> +     */
> +    var items = CallLog.callLogContainer.getElementsByClassName('log-item');
> +    for(var i = 0; i < items.length; i++) {
> +      var timestamp = items[i].getAttribute('data-timestamp');
> +      var newTimestamp = Utils.prettyDate(parseInt(timestamp)) + ' ';

parseInt should always be used with 10 as the second parameter to make sure we use the decimal base.

And maybe we should move the parseInt inside Utils.prettyDate?

Comment 9

[Tamara Hills [:thills]]

Attachment: updated after latest feedback

Hi Doug,

I'm sending this for feedback while I work on the tests.  Couple notes:

1.  No tests yet.  working on that now.
2.  No linting as of yet.  I will do that in a separate commit as you mentioned.
3.  I tested the callscreen states (except CDMA) and did not see anything amiss.

Comment 10

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8479162 [details] [diff] [review]
updated after latest feedback

Review of attachment 8479162 [details] [diff] [review]:
-----------------------------------------------------------------

My comments are pretty nit-picky now; this generally looks good.

> 1.  No tests yet.  working on that now.

What's your plan for testing? Please discuss with me on IRC what tests we should have.

> 3.  I tested the callscreen states (except CDMA) and did not see anything
> amiss.

To be sure of this, you can refer to: https://wiki.mozilla.org/FirefoxOS/Comms/Dialer/Call_screen_states

::: apps/communications/dialer/index.html
@@ -44,5 @@
>  
>      <!-- for perf-measurement related utilities -->
>      <script defer src='/shared/js/performance_testing_helper.js'></script>
>  
> -    <script defer src='/shared/js/date_time_helper.js'></script>

It doesn't look like you added this back somewhere else.

::: apps/communications/dialer/js/call_log.js
@@ +120,5 @@
>  
>          self.becameVisible();
> +
> +        window.addEventListener('timeformatchange',
> +                                self.updateCallTimes.bind(self));

A bit nit-picky, but I'd group this with the other addEventListener() calls.

@@ +145,3 @@
>    },
>  
>    updateCallTimes: function cl_updateCallTimes() {

This function should be tagged with an underscore to indicate that it's private.

@@ +145,4 @@
>    },
>  
>    updateCallTimes: function cl_updateCallTimes() {
> +    var items = this.callLogContainer.querySelectorAll('.log-item');

s/items/logItemElts/

@@ +149,2 @@
>      for (var i = 0; i < items.length; i++) {
> +      var item = items[i];

s/item/logItemElt/

@@ +149,4 @@
>      for (var i = 0; i < items.length; i++) {
> +      var item = items[i];
> +      var timestamp = item.getAttribute('data-timestamp');
> +      var newTimestamp = Utils.prettyDate(parseInt(timestamp, 10)) + ' ';

s/newTimestamp/formattedTime/

@@ +149,5 @@
>      for (var i = 0; i < items.length; i++) {
> +      var item = items[i];
> +      var timestamp = item.getAttribute('data-timestamp');
> +      var newTimestamp = Utils.prettyDate(parseInt(timestamp, 10)) + ' ';
> +      var callTime = item.querySelector('.call-time');

s/callTime/callTimeElt/

::: shared/js/dialer/utils.js
@@ +9,5 @@
>      return dtf.localeFormat(new Date(time), timeFormat);
>    },
>  
>    headerDate: function ut_headerDate(time) {
> +    var formattedTime;

You can stick this right before we first use it, above isNaN().

@@ +17,4 @@
>      var diff = (Date.now() - time) / 1000;
>      var day_diff = Math.floor(diff / 86400);
> +    if (isNaN(day_diff)) {
> +      formattedTime = _('incorrect date');

l10n keys shouldn't have spaces in them. Also, I would suggest naming this 'malformedDate' instead.

@@ +19,5 @@
> +    if (isNaN(day_diff)) {
> +      formattedTime = _('incorrect date');
> +    } else if (day_diff === 0) {
> +      formattedTime = _('today');
> +    } else if (day_diff == 1) {

===

Comment 11

[Tamara Hills [:thills]]

Attachment: utils test draft

Draft of tests.

Comment 12

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8479907 [details] [diff] [review]
utils test draft

Review of attachment 8479907 [details] [diff] [review]:
-----------------------------------------------------------------

This covers every situation that I can see, good job! I have a few mostly nit comments. We'll discuss next steps for testing on IRC.

::: apps/communications/dialer/test/unit/utils_test.js
@@ +11,5 @@
>    var number = '555-555-555-555';
>  
>    suiteSetup(function() {
>      realL10n = navigator.mozL10n;
>      navigator.mozL10n = {

This makes me a bit wary. Can you poke around and see if we can load a mozL10n mock instead, and append DateTimeFormat if we need it?

I would suggest looking at shared/test/unit/mocks/mock_l10n.js

@@ +15,5 @@
>      navigator.mozL10n = {
>        get: function get(key) {
>          return 'prefix-' + key;
> +      },
> +      DateTimeFormat: function () {

No space between `function` and `(`.

@@ +16,5 @@
>        get: function get(key) {
>          return 'prefix-' + key;
> +      },
> +      DateTimeFormat: function () {
> +        this.localeFormat = function (date, format) {

No space between `function` and `(`.

@@ +67,5 @@
>        });
>      });
> +
> +    test('#headerDate should identify yesterday as a time range', function() {
> +      var ts = Math.round(new Date().getTime() / 1000);

In general, I'd prefer just writing out "timestamp" but I don't feel strongly about this.

@@ +86,5 @@
> +      window.navigator.mozHour12 = false;
> +      assert.equal(subject.headerDate(ts3DaysAgo), '%A');
> +
> +      window.navigator.mozHour12 = true;
> +      assert.equal(subject.headerDate(ts3DaysAgo), '%A');

There's no need to test this against mozHour12 since the result is the same either way. If we were going to test this, we would make it a separate test, and call it something like "#headerDate is invariant on 12/24 hour time". I don't think that's necessary, though.

@@ +90,5 @@
> +      assert.equal(subject.headerDate(ts3DaysAgo), '%A');
> +    });
> +
> +    test('#headerDate should identify incorrect date', function() {
> +      assert.equal(subject.headerDate('not-a-date'), 'prefix-incorrect date');

As I've mentioned before, we can't use spaces in l10n keys. I would suggest naming this "malformedDate" instead of "incorrect date" as the latter implies that the user did something wrong.

Comment 13

[Tamara Hills [:thills]]

Attachment: WIP tests for 1035763

changes since last time:
1.  addressed the moz10n.js issue from last feedback and replaced it with the suggested one.  Made changes to other tests to make that work.
2.  Changes to callscreen_test
3.  Changed the span in callscreen to add that span back in there because without it there, the callscreen display shows AM/PM (which was not there before so I presume it was not desired to be there)
4.  Some small changes in call_log_tests.js.  Still working through this.

Comment 14

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8480636 [details] [diff] [review]
WIP tests for 1035763

Review of attachment 8480636 [details] [diff] [review]:
-----------------------------------------------------------------

We're going in the right direction, but there are a few problems here now. I'd like to see a new version of just this patch before we move forward. We can also talk on IRC about the problems with testing you've had.

In utils_test.js, I mention this several times:
"We should also check that `MockL10n.get()` was called with the correct args."

Here's the pattern I mean:

```
this.sinon.stub(MockL10n, 'get');
/* do something that should translate */
sinon.assert.calledWith(MockL10n.get, 'yesterday');
assert.equal(subject.headerDate(tsYesterday), 'yesterday');
```

::: apps/callscreen/js/call_screen.js
@@ +450,5 @@
>        var timeFormat = window.navigator.mozHour12 ? _('shortTimeFormat12') :
>                                                      _('shortTimeFormat24');
> +      // Review Note to drs: I added this back in because it removes the AM/PM
> +      // from the callscreen display.  That was not there initially (with 12 hour
> +      // presumably for a reason, so added back the line in to remove it.

Ok, please file a followup for us to do this by giving this span a '.hidden' class or something like that. This was really confusing when reading it.

::: apps/callscreen/test/unit/call_screen_test.js
@@ +864,5 @@
>    suite('showClock in screen locked status', function() {
>      var formatArgs = [],
>          currentDate,
> +        fakeClockTime12 = '12:02 <span>PM</span>',
> +        fakeClockTime24 = '13:02',

I'd suggest making the minutes different just to make it clear when glancing at this that they're not the same time in different formats.

@@ +870,5 @@
>  
>      setup(function() {
>        this.sinon.stub(navigator.mozL10n, 'DateTimeFormat', function() {
>          this.localeFormat = function(date, format) {
> +          console.log(format);

Make sure to remove this before we go into review.

@@ +893,5 @@
>        // The date parameter here should be equal to clock setup date.
>        assert.equal(formatArgs.length, 2);
>        assert.equal(formatArgs[0][0], currentDate);
>        assert.equal(formatArgs[1][0], currentDate);
> +      assert.equal(clockTime, fakeClockTime12);

I think we should break up this test into 1) a test for the current date, 2) a test for 12 hr time.

::: apps/communications/dialer/test/unit/call_log_test.js
@@ +431,5 @@
> +  suite('timeformatchange', function() {
> +    setup(function() {
> +    });
> +    test('should trigger update of the timestamps to new 12/24 timeformat',
> +      function() {

Line up `function() {` with `test(...`, and then indent everything in here back by 2 spaces.

@@ +436,5 @@
> +        CallLog.createGroup(incomingGroup);
> +        // Review note to drs:  I spent too much time trying to figure out why
> +        // the spy would not work with CallLog._updateCallTimes.  I think that's
> +        // probably the ideal solution, but for some reason, I was unable to get
> +        // the spy working for this, so I substitued for less than ideal approach here

We can talk on IRC about this. I'm not sure what the problem is. We should definitely get this working.

@@ +440,5 @@
> +        // the spy working for this, so I substitued for less than ideal approach here
> +        var timeSpy = this.sinon.spy(CallLog.callLogContainer, 'querySelectorAll');
> +        window.dispatchEvent(new CustomEvent('timeformatchange'));
> +        sinon.assert.calledWith(timeSpy, '.log-item');
> +        timeSpy.restore();

You don't need to call `(stub|spy).restore()` when using `this.sinon.***` as it's sandboxed to this test. This is only needed when using `sinon.***`.

@@ +445,5 @@
> +    });
> +
> +    test('update times to new 12/24 timeformat',
> +      function() {
> +        CallLog.createGroup(incomingGroup);

We should add a second group and test that the 2 log items, each one associated with a group, get updated.

@@ +451,5 @@
> +        //validate that the correct time format was called.  My thought was to
> +        //create a group using one of the pre-defined ones above, then validate
> +        //that the callTime is correct... so looking through the checkGroupDOM
> +        //and other functions to see if I can use them (or maybe modify them
> +        //to see if I can validate the correct format was called in there.

I don't think we actually need a second test here. What you're describing in this comment is what we should do in the first test. We should be checking the following there, in this order:
1) Each item has the time, but not in the desired format, before we begin.
2) Dispatching the 'timeformatchange' causes them to be updated. We will get this for free by adding checks after dispatching it.
3) The `Utils.prettyDate()` function is called with the correct args and the correct number of times.
4) Each item has the time in the desired format.

::: apps/communications/dialer/test/unit/utils_test.js
@@ +1,1 @@
> +/* global MockContacts, Utils MockL10n*/

s/Utils MockL10n/Utils, MockL10n /

@@ +3,5 @@
>  'use strict';
>  
>  require('/shared/test/unit/mocks/dialer/mock_contacts.js');
>  require('/shared/js/dialer/utils.js');
> +require('/shared/test/unit/mocks/mock_l10n.js');

I'm happy we were able to do this!

@@ +33,1 @@
>                       number, additionalInfo);

We should also check that `MockL10n.get()` was called with the correct args.

I think this can fit on one line now too. Though maybe not once you change it.

@@ +44,1 @@
>            MockContacts.mCarrier, additionalInfo);

We should also check that `MockL10n.get()` was called with the correct args.

@@ +63,5 @@
>  
>      test('#headerDate should identify yesterday as a time range', function() {
>        var timestamp = Math.round(new Date().getTime() / 1000);
>        var tsYesterday = (timestamp - (25 * 3600)) * 1000;
> +      assert.equal(subject.headerDate(tsYesterday), 'yesterday');

We should also check that `MockL10n.get()` was called with the correct args.

@@ +68,5 @@
>      });
>  
>      test('#headerDate should identify today as a time range', function() {
>        var tsToday = Math.round(new Date().getTime());
> +      assert.equal(subject.headerDate(tsToday), 'today');

We should also check that `MockL10n.get()` was called with the correct args.

@@ +75,5 @@
>      test('#headerDate should format 3 days ago as a weekday name',
>           function() {
>        var ts = Math.round(new Date().getTime() / 1000);
>        var ts3DaysAgo = (ts - (73 * 3600)) * 1000;
> +      assert.include(subject.headerDate(ts3DaysAgo), '%A');

We should also check that `MockL10n.get()` was called with the correct args.

@@ +80,4 @@
>      });
>  
>      test('#headerDate should identify incorrect date', function() {
> +      assert.equal(subject.headerDate('not-a-date'), 'malformedDate');

We should also check that `MockL10n.get()` was called with the correct args.

@@ +87,5 @@
>           function() {
>        var tsToday = Math.round(new Date().getTime());
>  
>        window.navigator.mozHour12 = false;
> +      assert.include(subject.prettyDate(tsToday), 'shortTimeFormat24');

Why not `assert.equal()`?

@@ +92,3 @@
>  
>        window.navigator.mozHour12 = true;
> +      assert.include(subject.prettyDate(tsToday), 'shortTimeFormat12');

Why not `assert.equal()`?

Comment 15

[Tamara Hills [:thills]]

Attachment: Pull Request Updated

Comment 16

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8480983 [details] [review]
Pull Request Updated

I left some comments on the PR.

This is really getting there. Most of the problems I found were relatively minor, except for a missing test and another test not checking things correctly.

Comment 17

[Doug Sherk (:drs) (inactive)]

Also, please rebase before the next review round. Just run:
git pull --rebase upstream master

You'll encounter merge conflicts, so let me know if you need help resolving them.

Comment 18

[Tamara Hills [:thills]]

Attachment: Pull request updated v2

Updates after last review + merge

Comment 19

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8481367 [details] [review]
Pull request updated v2

We're almost there. There are a few minor issues, and the only major thing remaining is a missing test. See the PR for comments.

Comment 20

[Doug Sherk (:drs) (inactive)]

There's actually one more potentially very serious issue I see with the initialTime variable in the 'timeformatchange' suite. Let's make sure we tackle that.

Comment 21

[Tamara Hills [:thills]]

Attachment: Pull request updated v3

Updated pull request

Comment 22

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8481503 [details] [review]
Pull request updated v3

Nice work! We'll land this once it gets a green try run.

Comment 23

[Doug Sherk (:drs) (inactive)]

https://github.com/mozilla-b2g/gaia/commit/1aabe50c948ac29ee4d5aa88dbdad52a5cdd6219

Comment 24

[Eric Chang [:ericcc] [:echang]]

Verified User story is fixed. ICE can be accessed lockscreen.

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

Comment 25

[Eric Chang [:ericcc] [:echang]]

Verified User story is fixed. 24-hour format can be seen from call log.

Comment 26

[Loli (:lolimartinezcr)]

Verified and working
Flame 
2.1
Gecko-039bd5d
Gaia-95e9b09