1071988 - [Clock] All alarms show military time when they ring

Status: VERIFIED FIXED

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

Description

[Rachel Pribble [:rpribble]]

Attachment: Logcat.txt

Description:
All alarms show military time when they sound, regardless of whether the alarm was originally created in military or 12-hour time.
   
Repro Steps:
1) Update a Flame device to BuildID: 20140922043003
2) Clock > create a timer in 12-hour time
3) Observe screen when alarm sounds
  
Actual:
All alarms display in military time.
  
Expected: 
Alarms display in the time they were originally created in.

Device: Flame 2.1 KK
BuildID: 20140923003005
Gaia: 3a2947df41a480de1457a6dcdbf46ad0af70d8e0
Gecko: df42b05782aa
Version: 34.0a2
Firmware: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 Master KK
BuildID: 20140922043003
Gaia: 3802009e1ab6c3ddfc3eb15522e3140a96b33336
Gecko: 5e704397529b
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
  
Repro frequency: 100%
See attached: Video, logcat

Comment 1

[Peter Bylenga [:PBylenga]]

qawanted for branch checks.

Comment 2

[Pi Wei Cheng [:piwei] (inactive)]

Attachment: Flame 2.0 (no repro)

This issue is a regression from Flame 2.0. Attaching a screenshot of alarm activating in 2.0 showing time + AM/PM.

Device: Flame
BuildID: 20140925082640
Gaia: 95a51689acd0488b3ba79abe2423cdcc132fff4a
Gecko: bd67c37ece85
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 3

[Joshua Mitchell (Inactive)]

[Blocking Requested - why for this release]: Regression, very bad UX for ignoring user input / preference

Comment 4

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Bug 1035757 seems to be the cause of this issue.

B2g-inbound Regression Window

Last working 
Environmental Variables:
Device: Flame 2.1
BuildID: 20140826131129
Gaia: a91e3021cd8f6f9d02aac7b6ae3c697261491a32
Gecko: 63304ac1d451
Version: 34.0a1 (2.1) 
Firmware Version: L1TC00011230
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First Broken 
Environmental Variables:
Device: Flame 2.1
BuildID: 20140826132727
Gaia: 14a2fef47142cd873df27a6c846fcd9f854a75e6
Gecko: 753a7143884f
Version: 34.0a1 (2.1) 
Firmware Version: L1TC00011230
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Last working gaia / First broken gecko - Issue does NOT occur
Gaia: a91e3021cd8f6f9d02aac7b6ae3c697261491a32
Gecko: 753a7143884f

First broken gaia / Last working gecko - Issue DOES occur
Gaia: 14a2fef47142cd873df27a6c846fcd9f854a75e6
Gecko: 63304ac1d451

Gaia Pushlog:  https://github.com/mozilla-b2g/gaia/compare/a91e3021cd8f6f9d02aac7b6ae3c697261491a32...14a2fef47142cd873df27a6c846fcd9f854a75e6

Comment 5

[Joshua Mitchell (Inactive)]

Broken by Bug 1035757? Can you take a look Marcus?

Comment 6

[Dylan Oliver [:doliver]]

triage: blocking for regression in new 2.1 feature, highly visible

Comment 7

[Dylan Oliver [:doliver]]

> Expected: 
> Alarms display in the time they were originally created in.

I disagree with this; we should honor whatever the current 12/24 setting is, not what it was when the alarm was created.

Comment 8

[Evan Tseng [:evanxd]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24540

Comment 9

[Evan Tseng [:evanxd]]

Comment on attachment 8497289 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24540

Hi Marcus,

Could you help to review the patch?

Thanks.

Comment 10

[Marcus Cavanaugh [:mcav]]

Comment on attachment 8497289 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24540

Ah, that sneaky date_time_helper. Thanks for taking this Evan, and for the marionette test!

Comment 11

[Evan Tseng [:evanxd]]

NP, Marcus. :)

Comment 12

[Evan Tseng [:evanxd]]

master: https://github.com/mozilla-b2g/gaia/commit/d107b9b80d3d8eb54c487d5b7fed3e10c3db3118

Comment 13

[Evan Tseng [:evanxd]]

Comment on attachment 8497289 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24540

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Bug 1035757
[User impact] if declined: The date time in alarm screen always shows 24 hour even it is on the 12 hour format.
[Testing completed]: We have marionette tests for the patch, and it work well[1] on try server.
[Risk to taking this patch] (and alternatives if risky): Low rick, we just add one line to include one missing lib.
[String changes made]: No.

[1] https://tbpl.mozilla.org/?rev=fa091a10f26478094869015e224aa477cc71cdf5&tree=Gaia-Try

Comment 14

[Kevin Grandon :kgrandon]

Comment on attachment 8497289 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24540

Not ready for uplift, comment coming soon.

Comment 15

[Kevin Grandon :kgrandon]

This was backed out for causing basically a perma failure ( > 50% failure rate on tbpl). This was not necessarily your fault as all failures were masked on gaia-try due to bug 1075787. Please see the recent dev-gaia thread for more info.

It was present in your try run here, but unfortunately invisible due to the bug: https://tbpl.mozilla.org/php/getParsedLog.php?id=49209277&tree=Gaia-Try

Backout commit: https://github.com/mozilla-b2g/gaia/commit/5a4da5a9ad9bda75f621a9430625b0cbe234a8ea

Comment 16

[Evan Tseng [:evanxd]]

Kevin, thanks for the help.
I'm fix this bug.

Comment 17

[Evan Tseng [:evanxd]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24646

Comment 18

[Evan Tseng [:evanxd]]

Comment on attachment 8498635 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24646

Hi Marcus,

Could you help review the patch again?

I fixed the intermittent failing issues of marionette tests.
The tests are passed continually for 10 time[1] on try server.
The failed Gij tasks are about framework timeout issue, and I think it is not related with the patch.

Sorry for the previous bad tests.

[1] https://tbpl.mozilla.org/?rev=e50ec97fdd5b80f67a4c25db0a29a3e94c69504e&tree=Gaia-Try

Comment 19

[Marcus Cavanaugh [:mcav]]

I have just one question on your PR, about the change in active_alarm.js. Other than that it looks okay, but I'd like to confirm that the change in that file is necessary.

Comment 20

[Evan Tseng [:evanxd]]

Marcus,

Sorry, it is a mistake. I already updated it[1].
Please help to review again.
Thanks.

And I will make sure the tests could be passed for 10 time at least on try server before landing the code. It's not easy to skip intermittent failures.

[1] https://github.com/mozilla-b2g/gaia/pull/24646/files?diff=unified#diff-b5f8952b284350359413e30d57a310b1L41

Comment 21

[Marcus Cavanaugh [:mcav]]

Comment on attachment 8498635 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24646

Okay, sounds good.

Comment 22

[Marcus Cavanaugh [:mcav]]

master:

https://github.com/mozilla-b2g/gaia/commit/a976e2cbd9e57d27b749a1e493acb941f6975dc8

Comment 23

[Evan Tseng [:evanxd]]

Comment on attachment 8498635 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24646

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Bug 1035757
[User impact] if declined: The date time in alarm screen always shows 24 hour even it is on the 12 hour format.
[Testing completed]: We have marionette tests for the patch, and it work well[1] on try server.
[Risk to taking this patch] (and alternatives if risky): Low rick, we just add one line to include one missing lib.
[String changes made]: No.

[1] https://tbpl.mozilla.org/?rev=127d09ad96518cd5251933a595568d9ffb99646d&tree=Gaia-Try

Comment 24

[Evan Tseng [:evanxd]]

Marcus, thanks for the landing.

Comment 25

[bhavana bajaj [:bajaj]]

Comment on attachment 8498635 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24646

Requesting QA verification once this lands..

Comment 26

[Ryan VanderMeulen [:RyanVM]]

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

Comment 27

[cnelson]

Issue is verified fixed on Flame 2.0, Flame 2.1, and Flame 2.2.

Comment 28

[cnelson]

Flame 2.1 KK (512mb) (Full Flash)

Environmental Variables:
Device: Flame 2.1
BuildID: 20141010000201
Gaia: bc8eb493311c58f1f311a56b8b645b52bfbd2f71
Gecko: 72c13d8631ff
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.2 Master KK (319mb) (Full Flash)

Device: Flame 2.2 Master
BuildID: 20141011040204
Gaia: 95f580a1522ffd0f09302372b78200dab9b6f322
Gecko: 3f6a51950eb5
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Flame 2.0 KK (319mb) (Full Flash)

Device: Flame 2.0
BuildID: 20141011000202
Gaia: 6effca669c5baaf6cd7a63c91b71a02c6bd953b3
Gecko: 54ec9cb26b59
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 29

[cnelson]

All alarms set are appearing in the time format that the device is set to.  The time appears correctly when the alarm is set, as well as when the alarm goes off.