804394 - [Clock] if more than one alarm is set and goes off and the screen lock is placed, only the top alarm is disabled.

Status: VERIFIED FIXED

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

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

## Environment :
Otoro phone, build 2012-10-22
Taken from default.xml in b2g-distro: 
* "platform_build" revision= 0d6d050bc37d3167cc82a1885fd7660456bb0f4e
* "gaia" revision= cf533e66c4c589478f5addc53cab2d9a4853402b 
* "releases-mozilla-aurora" revision= 471232068fc202ba54b12eaf5b7de70aef4a079e
* "gonk-misc" revision= da6e50508245d023166e0c04369200e5bb898220

  
## Repro :
1. set alarm for 1 min into the future
2. set alarm for 2 min into the future
3. leave the phone alone for 3 mins.
4. unlock the phone after clicking close on the alarm

## Expected :
1. both alarms are disabled.

## Actual :
1. only the topmost alarm is disabled.

## Note :

Comment 1

[[e-mail], [e-mail]]

I find out the first alarm's ringtone won't be turned off when the second alarm goes off. Then, there is no way to turn the first alarm ringtone off.
The issue should be bb+.

Comment 2

[Joe Cheng [:jcheng] (please needinfo)]

Triage: BB+, C3, C4 - severe usability issue for not able to dismiss alarm in such case

Comment 3

[Dylan Oliver [:doliver]]

Ian, can you provide an update on this bug?

Comment 4

[Anthony Ricaud (:rik)]

Ian: I'm looking into this. Let me know if you're working on this.

Comment 5

[Joe Cheng [:jcheng] (please needinfo)]

talked to Ian last week, he was working on it.

Comment 6

[[e-mail], [e-mail]]

Dylan and Anthony,
I'm looking the issue for a long time. And I already have a experimental patch in my branch. I also consider multiple alarms(more than 2 or 3..) case. It should be updated each alarm in the alarm list. Not refresh the alarm list every time. Thanks for caring. I will give patch soon.

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Ian Liu [:ianliu] from comment #6)
> Dylan and Anthony,
> I'm looking the issue for a long time. And I already have a experimental
> patch in my branch. I also consider multiple alarms(more than 2 or 3..)
> case. It should be updated each alarm in the alarm list. Not refresh the
> alarm list every time. Thanks for caring. I will give patch soon.

By "soon", what's specific the ETA?

Comment 8

[[e-mail], [e-mail]]

I'll give the patch by end of work day Tuesday at Taipei timezone.

Comment 9

[[e-mail], [e-mail]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7370

Pointer to Github pull-request

Comment 10

[[e-mail], [e-mail]]

Comment on attachment 699138 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7370

Tim,
Could you please help to review my pr?
I leave some comment on github.
Thank you.

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 699138 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7370

r=me with nit addressed.

I worry that landing this big chunk of change at this stage might break something, but I am confident that you have already test through it. I will notify QAs to test the Clock app throughly once this lands. 

The overall arch of the Clock app has become too complicated for everyone other than you to work on it. Either we need to put some comment into the code (djf style), or you would need to prepare some kind of documentation on "a alarm's life" so code tracing will not be this painful.

Good work btw.

Comment 12

[[e-mail], [e-mail]]

Tim,
Thanks for your review effort.
I fixed nit and add some comment in detail(flow map..).
And having some test from night to this morning.
Even if I test through it with many cases, it's necessary with QA's test completely.
If you don't have any concern of the comment, I'll merge it.

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Ian Liu [:ianliu] from comment #12)
> Tim,
> Thanks for your review effort.
> I fixed nit and add some comment in detail(flow map..).
> And having some test from night to this morning.
> Even if I test through it with many cases, it's necessary with QA's test
> completely.
> If you don't have any concern of the comment, I'll merge it.

Please merge, thanks!

Comment 14

[[e-mail], [e-mail]]

https://github.com/mozilla-b2g/gaia/pull/7370
Since the pr is merged, we can close the issue now.

Comment 15

[Al Tsai [:atsai]]

verify the fix with local build.