1061417 - [SMS] We don't properly discard the draft in some occasions

Status: VERIFIED FIXED

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

Description

[ankit93040]

User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.143 Safari/537.36

Steps to reproduce:

Pre-condition - Delete all threads
1. Go to message-> New Message->To Field
2. Type 45698
3. Put the cursor on message field by touching it.
4. Press home button.
5. Go to message. 
6. Type anything in message filed.
7. Press back button
8. Press Discard
We don't see any message in the thread list
9. Long press the home key. Kill message application
10. Go to message


Actual results:

The draft exist


Expected results:

The draft shouldn't exist

Comment 1

[ankit93040]

Hi Steve

Need your urgent feedback on this.

Thanks!

Comment 2

[Steve Chung [:steveck]]

I could not reproduce it on master, is this stil v2.0 specific problem?

Comment 3

[ankit93040]

Attachment: CAM00058.mp4

Comment 4

[ankit93040]

The issue is reproducible on both master as well as v2.0

Comment 5

[ankit93040]

If you help me a little, then I ll be able to fix it.

Probably where to look into.

Comment 6

[Julien Wajsberg [:julienw]]

[Blocking Requested - why for this release]:

I know for a long time that we have an issue with correctly discarding drafts. It's not easy to reproduce in a QA-like way but it happens quite often when using the device on a day to day device, because we often use "home" while writing a message.

I think we should try to fix it for v2.0 (I think it already exists in v1.4 though).

Comment 7

[Steve Chung [:steveck]]

(In reply to ankit93040 from comment #5)
> If you help me a little, then I ll be able to fix it.
> 
> Probably where to look into.

Ok, now I can reproduce by your steps, the interesting things is if we type something in the message input in step 3, or kill the message app after step 4, then everything works fine. Will take more time on finding the root cause.

Comment 8

[Jason Smith [:jsmith]]

steps-wanted to find reduced STR

Comment 9

[Chris Kreinbring [:CKreinbring]]

Unable to repro the bug by reducing steps.  The only flexible part is the exact number to enter in step 2.

BuildID: 20140902095454
Gaia: 44bf2e3bc5ddea9db9a8c851bd353cb234aa883c
Gecko: a40250cb7d93
Platform Version: 35.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Comment 10

[bhavana bajaj [:bajaj]]

The STR seem reasonable to me for an end user to use when discarding message. Can we confirm this is a regression and test this on 1.3/1.4 ? Would help make a blocking decision.

Comment 11

[Chris Kreinbring [:CKreinbring]]

The bug repros on Flame 1.4.  We do not have builds for Flame 1.3.
Actual result: The deleted conversation reappears on the thread list after relaunching the Messages app.

BuildID: 20140909105753
Gaia: 2ee5b00bfbb8a67a967094804390b4afce8ecf54
Gecko: faa0a4505925
Platform Version: 30.0
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Comment 12

[Joshua Mitchell (Inactive)]

Chris - can you check just the latest base (Flame v123) in lieu of actual 1.3 builds. And after, mark the appropriate tracking flags.

Comment 13

[Wilfred Mathanaraj [:WDM]]

[Blocking Requested - why for this release]:

given this is been in the software for 1.4 I would like this to be fixed for 2.1 (given the 2.0 timelines)

Comment 14

[Wilfred Mathanaraj [:WDM]]

bad experience and should be fixed.

Comment 15

[Dietrich Ayala (:dietrich)]

Triage group is renominating for discussion. We didn't hold the last two releases for this bug, so why would we block on it now? Age of the bug doesn't change the impact on the user.

Which of the blocking criteria here does it meet? https://wiki.mozilla.org/B2G/Triage#Blocker_Triage_Guidelines

Comment 16

[Wayne Chang [:wchang]]

(In reply to Dietrich Ayala (:dietrich) from comment #15)
> Triage group is renominating for discussion. We didn't hold the last two
> releases for this bug, so why would we block on it now? Age of the bug
> doesn't change the impact on the user.
> 
> Which of the blocking criteria here does it meet?
> https://wiki.mozilla.org/B2G/Triage#Blocker_Triage_Guidelines

We didn't hold the last two releases for this bug maybe because it was not found then? The bug was reported about just a week ago.

My vote goes to 2.1 blocking given the broken feature/user experience and the timelines. Imagine the phone saving a text that you really wish it had discarded properly...

Comment 17

[Chris Kreinbring [:CKreinbring]]

The bug does not repro on the Flame 1.3 base image.
Actual result: When the Messages app is relaunched the deleted thread does not reappear.

BuildID: 20140627162151
Gaia: 5c43d012a9351e8aaeacda0b87b0886b7f67d33d
Gecko: 
Platform Version: 28.0
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:28.0) Gecko/28.0 Firefox/28.0

Comment 18

[Beatriz Rodríguez [:brg]]

(In reply to Wayne Chang [:wchang] from comment #16)
> (In reply to Dietrich Ayala (:dietrich) from comment #15)
> > Triage group is renominating for discussion. We didn't hold the last two
> > releases for this bug, so why would we block on it now? Age of the bug
> > doesn't change the impact on the user.
> > 
> > Which of the blocking criteria here does it meet?
> > https://wiki.mozilla.org/B2G/Triage#Blocker_Triage_Guidelines
> 
> We didn't hold the last two releases for this bug maybe because it was not
> found then? The bug was reported about just a week ago.
> 
> My vote goes to 2.1 blocking given the broken feature/user experience and
> the timelines. Imagine the phone saving a text that you really wish it had
> discarded properly...

Please block on 2.1. This is a future certification blocker. If the issue is discovered under 2.0 cert, we can try a waiver but... we will need it fixed in 2.1.

Comment 19

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: According to comment 18, this will be a cert blocker so I'm setting blocking.

Comment 20

[Julien Wajsberg [:julienw]]

stealing since Steve is in PTO this week

Comment 21

[Mozilla Shepherd (This account is used for automation and doesn't receive bugmail)]

Attachment: mozilla-b2g:master PR#24145

Comment 22

[Julien Wajsberg [:julienw]]

Comment on attachment 8490837 [details] [review]
mozilla-b2g:master PR#24145

This was a lot easier than I expected ;)

Comment 23

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8490837 [details] [review]
mozilla-b2g:master PR#24145

Looks good, but I've left just one suggestion to unify delete operation with add one to eliminate such issues in the future. If you think it makes sense, you can set r? again :)

Thanks!

Comment 24

[Julien Wajsberg [:julienw]]

master: 7e933e991856f3a720ca6345ac749554b4fef58e

Comment 25

[Julien Wajsberg [:julienw]]

Comment on attachment 8490837 [details] [review]
mozilla-b2g:master PR#24145


[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 879143 and dependencies
[User impact] if declined: sometimes we don't properly delete drafts
[Testing completed]: yes (unit tests and manually)
[Risk to taking this patch] (and alternatives if risky): very low
[String changes made]: none

The bug happens since we do drafts (so since version 1.4 if I'm not wrong), so I'd also like to uplift to v2.0 given the simplicity of the patch and that the bug's impact is quite annoying.

Comment 26

[bhavana bajaj [:bajaj]]

Comment on attachment 8490837 [details] [review]
mozilla-b2g:master PR#24145

Lets get this landed and tested on 2.1 for now. Barring no regressions are seen in the next few days we can conciser taking on 2.0.


QA can you please help verify this on 2.1 and do some exploratory testing around this area ? Thanks!

Comment 27

[ankit93040]

Hi Julienw

Just for the curiosity purpose first I took the reviewed patch from https://github.com/mozilla-b2g/gaia/pull/23901

then I applied this bug'a pull request & found the following two issues- 

1.	Save some contact -> Select Message icon from contact -> Select ‘x’ button -> ‘Save as Draft’
Result: The Draft message is not saved
2.	Enter some number in dialer -> Select Message icon -> Select ‘x’ button -> ‘Save as Draft’
Result : Draft is not saved

I applied it to MOZ v2.0.

Thanks

Comment 28

[Julien Wajsberg [:julienw]]

Ankit, are you seeing the same issues without the patch too? If you see these issues with/without the patch, then please file a separate bug, and also test on newer versions.

I honestly think this is unrelated to this patch, but let's see.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

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

Comment 30

[bhavana bajaj [:bajaj]]

Julien, can you confirm there were no fallouts to get this landed on 2.0, I am a bit confused by comment #28 but see no active dependencies, hence this comment.. Thanks!

Comment 31

[Julien Wajsberg [:julienw]]

I'm quite sure that what Ankit saw is bug 1058459 (an issue about synchronizing the different windows of an app). So not related to this bug, let's uplift this !

Comment 32

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/ddb7567c60b31dbcb27817f8c126bbba8dccb63b

Comment 33

[Kai-Zhen Li [:seinlin][:kli]]

v2.0m: https://github.com/mozilla-b2g/gaia/commit/ddb7567c60b31dbcb27817f8c126bbba8dccb63b

Comment 34

[Oliver Nelson [:oliverthor]]

VERIFIED FIX in Master 2.2, 2.1, and 2.0
Results: Draft is discarded when following STR from comment 1
****************************************************
Environment Variables:

Device: Flame 2.2 Master
BuildID: 20141011040204 (Full Flash)
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

Device: Flame 2.1
BuildID: 20141011000201 (Full Flash)
Gaia: f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1
Gecko: d813d79d3eae
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.03

Device: Flame 2.0
BuildID: 20141011000202 (Full Flash)
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 35

[KTucker [:KTucker][Inactive 3/4/2016]]

Adding verifyme because 2.0M needs to be verified. We do not have the appropriate device to check this.

Comment 36

[Coler]

Verify passed, this issue can't be repro on Woodduck 2.0.
Attached: Verify_Woodduck_SMS.mp4
Reproducing rate: 0/5
Gaia-Rev        ee5cf148b4c546beea9bfb799d2a3ee62074957d
Gecko-Rev       73601b71861cbc2f180c4d2653cec3e9fbb39db5
Build-ID        20141114050313
Version         32.0

Comment 37

[Coler]

Attachment: Verify_Woodduck_SMS.MP4