1080295 - [Clock][Alarm] Creating multiple alarms causes one alarm to be undeletable

Status: RESOLVED FIXED

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

Description

[Corinne Innes [:CorinneI]]

Description:
When user creates multiple alarms, and then tries to delete all of the alarms, one alarm will become undeletable. The now immortal alarm can also be cloned from tapping the "Alarm On/Of" checkmark (Instead of by the "Add Alarm" icon at the top right of the page). 


Repro Steps:
1) Update a Flame device to BuildID: 20141008000201
2) Open Clock app from the Home screen 
3) Open Alarm page from Clock app
4) Create an alarm (Time, lable, repeat etc... shouldn't matter)
5) Select "Done"
6) Repeat steps 4 and 5 until you have a list of 10 or alarms set
7) Delete all alarms
8) If bug does NOT occur then repeat steps 4-8
  
Actual:
One alarm becomes undeletable
  
Expected: 
All alarms should be able to be deleted.
  
Environmental Variables:
Device: Flame 2.1 kk (319mb)(Full Flash)
BuildID: 20141008000201
Gaia: d71f8804d7229f4b354259d5d8543c25b4796064
Gecko: 7fa82c9acdf2
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
  
Notes:
  
Repro frequency: 50% to 75%
Link to failed test case: https://moztrap.mozilla.org/manage/case/1783/
See attached: Youtube video clip
Youtube Video Link: https://www.youtube.com/watch?v=UVG5VCJTITY

Comment 1

[Corinne Innes [:CorinneI]]

@qawanted: Could not get this to repro on 2.0 or 2.2, please check this when you can.

Comment 2

[Cody Roesch [:croesch]]

Tested with Shallow Flash on 319mb using Engineering builds

This bug repro's on Flame KK builds: Flame 2.2 KK, Flame 2.1 KK, Flame 2.0 KK, Flame 2.0 KK Base

Actual Results: Able to create a situation where alarms in the Clock App refuse to get deleted. When creating the alarms, I just keep quickly tapping Add Alarm and done really fast to create the alarms in quick succession and make a bunch of alarms. It seems like they all get broken this way and the user can't delete any or very few of them.

Repro Rate: 6/6

Environmental Variables:
Device: Flame Master KK
BuildID: 20141008170004
Gaia: 7b92615bdc97e5c675cd385ec68bc5e47e0c5288
Gecko: f0bb13ef0ee4
Version: 35.0a1 (Master) 
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
-----------------------------------------------------------------
Environmental Variables:
Device: Flame 2.1 KK
BuildID: 20141008192207
Gaia: 7e2ef41d3ac98757acaf490b5413fb42061ad3e6
Gecko: 75ebb70f8b38
Version: 34.0a2
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
-----------------------------------------------------------------
Environmental Variables:
Device: Flame 2.0 KK
BuildID: 20141008192303
Gaia: c1f60895e5bcc6a951f3667c2a1e1bd39d393420
Gecko: 4540201b5da0
Version: 32.0 (2.0) 
Firmware Version: L1TC10011800
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
-----------------------------------------------------------------
Environmental Variables:
Device: Flame 2.0 Base KK
BuildID: 20140904160718
Gaia: 506da297098326c671523707caae6eaba7e718da
Gecko: 
Gonk: 
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 3

[Corinne Innes [:CorinneI]]

Attachment: 10/9 logcat

Logcat documented for most recent occurence of this bug.

Comment 4

[Joshua Mitchell (Inactive)]

NI to the QA-Lead for Productivity: Clock - Edward - can you help make a blocking decision here?

Comment 5

[Edward Chen[:edchen]]

I can reproduce this issue, but in my option that its not blocking bug in v2.1. Since when you deleted alarms which is not quite quickly, this bug is hard reproduction. I suggest we can fix this bug in v2.2.

Comment 6

[Dylan Oliver [:doliver]]

triage: blocking for regression, will fix in 2.2

Comment 8

[Marcus Cavanaugh [:mcav]]

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

This is a glitch caused by the poor design of the AlarmEditPanel, which uses a `this.alarm` singleton variable to refer to the currently-editing alarm. When handling the "save alarm promise callback", we erroneously refer to `this.alarm` even though that may point to a different alarm (when the user has quickly saved an alarm and re-opened the AlarmEditPanel).

I have tested this manually and confirmed that this fixes the bug. I have added a comment in bug 1033213 (the bug tracking why the AlarmEditPanel tests are currently disabled) to add an automated test for this case, as I don't want to inadvertently introduce an intermittent by trying to replicate the currently-disabled unit test machinery for such a simple fix.

Comment 9

[Miller Medeiros [:millermedeiros]]

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

LGTM! changes are minimal and seems to fix the problem.

Comment 10

[Marcus Cavanaugh [:mcav]]

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

Comment 11

[Marcus Cavanaugh [:mcav]]

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

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): none, always present
[User impact] if declined: Users may see undeletable alarms.
[Testing completed]: Manual verification.
[Risk to taking this patch] (and alternatives if risky): Low risk, minimal change, clear cause and effect.
[String changes made]: none

Comment 12

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/032affa5bcfcaf8daa28b1ed2279a14e15d9bebf

Comment 13

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

This issue is NOT fixed on Flame 2.2 and NOT fixed on Flame 3.0 master.

Following STR + repro tips on comment 2 (adding alarms quickly), I was able to run into one or multiple alarms which are not deletable. Repro rate: 1/1 on 2.2, 1/1 on 3.0 master (1 attempt = doing the whole STR once)

Device: Flame 2.2 (shallow flash, 319MB mem)
BuildID: 20150122141946
Gaia: 237008137f6d72b9cad25ff4faff14ff2c40ac63
Gecko: 1090c8c5429f
Version: 37.0a2 (2.2)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Device: Flame 3.0 Master (shallow flash, 319MB mem)
BuildID: 20150122144346
Gaia: cba2f0bf49b882e0044c3cc583de8fcf83d2ffa4
Gecko: 43ad59d08bc4
Version: 38.0a1 (3.0 Master)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Comment 14

[Marcus Cavanaugh [:mcav]]

I've investigated this further. The patch correctly fixes the bug when you interpret the STR in a certain way (i.e. tap "done", tap "new alarm", repeatedly, but not necessarily purposefully double-tapping the "done" button). However, a very similar issue occurs when you double-tap the "Done" button on the edit screen, and that's what you're seeing in Comment 13; it doesn't occur merely from creating multiple alarms quickly in succession.

In other words, the STR for Comment 13 would be simply: double-tap the "Done" button when creating a new alarm.

Since the STR for this bug and that bug are similar, I've opened Bug 1124965 to track the related issue, and have requested blocking-b2g:2.2? to provide the appropriate uplift.