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

VERIFIED FIXED in Firefox OS v2.0

Status

Firefox OS
Gaia::SMS
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: ankit93040, Assigned: julienw)

Tracking

(Blocks: 1 bug)

unspecified
2.1 S5 (26sep)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.1+, b2g-v1.4 affected, b2g-v2.0 verified, b2g-v2.0M verified, b2g-v2.1 verified, b2g-v2.2 verified)

Details

(Whiteboard: [g+][LibGLA,TD92485,QE1, B][p=1])

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
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
(Reporter)

Updated

3 years ago
Whiteboard: [
(Reporter)

Updated

3 years ago
Whiteboard: [ → [g+][LibGLA,TD92485,QE1, B]
(Reporter)

Updated

3 years ago
Component: General → Gaia::SMS
(Reporter)

Comment 1

3 years ago
Hi Steve

Need your urgent feedback on this.

Thanks!
Flags: needinfo?(schung)
I could not reproduce it on master, is this stil v2.0 specific problem?
Flags: needinfo?(schung) → needinfo?(ankit93040)
(Reporter)

Comment 3

3 years ago
Created attachment 8482527 [details]
CAM00058.mp4
Flags: needinfo?(ankit93040)
(Reporter)

Comment 4

3 years ago
The issue is reproducible on both master as well as v2.0
(Reporter)

Comment 5

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

Probably where to look into.
Flags: needinfo?(schung)
(Assignee)

Comment 6

3 years ago
[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).
blocking-b2g: --- → 2.0?
(Assignee)

Updated

3 years ago
Summary: [SMS]Cache problem → [SMS] We don't properly discard the draft in some occasions
(Assignee)

Updated

3 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(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.
Flags: needinfo?(schung)
steps-wanted to find reduced STR
Keywords: steps-wanted
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
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: steps-wanted
QA Contact: ckreinbring
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
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.

Updated

3 years ago
Keywords: qawanted
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
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
Chris - can you check just the latest base (Flame v123) in lieu of actual 1.3 builds. And after, mark the appropriate tracking flags.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell) → needinfo?(ckreinbring)
[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)
blocking-b2g: 2.0? → 2.1?
bad experience and should be fixed.
blocking-b2g: 2.1? → 2.1+
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
blocking-b2g: 2.1+ → 2.1?
(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...
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
Flags: needinfo?(ckreinbring)
status-b2g-v1.4: --- → affected
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → affected
(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.
triage: According to comment 18, this will be a cert blocker so I'm setting blocking.
blocking-b2g: 2.1? → 2.1+
Blocks: 1026943

Updated

3 years ago
Assignee: nobody → schung
Target Milestone: --- → 2.1 S5 (26sep)
(Assignee)

Updated

3 years ago
Blocks: 1067820
Whiteboard: [g+][LibGLA,TD92485,QE1, B] → [g+][LibGLA,TD92485,QE1, B][p=1]
(Assignee)

Comment 20

3 years ago
stealing since Steve is in PTO this week
Assignee: schung → felash
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
Created attachment 8490837 [details] [review]
mozilla-b2g:master PR#24145
(Assignee)

Comment 22

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

This was a lot easier than I expected ;)
Attachment #8490837 - Flags: review?(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!
Attachment #8490837 - Flags: review?(azasypkin) → review+
(Assignee)

Comment 24

3 years ago
master: 7e933e991856f3a720ca6345ac749554b4fef58e
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 25

3 years ago
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.
Attachment #8490837 - Flags: approval-gaia-v2.1?(bbajaj)
Attachment #8490837 - Flags: approval-gaia-v2.0?(bbajaj)
status-b2g-v2.2: affected → fixed
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!
Attachment #8490837 - Flags: approval-gaia-v2.1?(bbajaj) → approval-gaia-v2.1+

Updated

3 years ago
Keywords: verifyme
(Reporter)

Comment 27

3 years ago
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
Flags: needinfo?(felash)
(Assignee)

Comment 28

3 years ago
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.
Flags: needinfo?(felash)
v2.1: https://github.com/mozilla-b2g/gaia/commit/e0cfbebc6e96bf636cecf43b29cb9fe15e8d661a
status-b2g-v2.1: affected → fixed
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!
Flags: needinfo?(felash)
(Assignee)

Comment 31

3 years ago
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 !
Flags: needinfo?(felash)

Updated

3 years ago
Attachment #8490837 - Flags: approval-gaia-v2.0?(bbajaj) → approval-gaia-v2.0+
v2.0: https://github.com/mozilla-b2g/gaia/commit/ddb7567c60b31dbcb27817f8c126bbba8dccb63b
status-b2g-v1.4: affected → fixed
v2.0m: https://github.com/mozilla-b2g/gaia/commit/ddb7567c60b31dbcb27817f8c126bbba8dccb63b
status-b2g-v1.4: fixed → affected
status-b2g-v2.0: affected → fixed
status-b2g-v2.0M: --- → fixed
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.0: fixed → verified
status-b2g-v2.1: fixed → verified
status-b2g-v2.2: fixed → verified
Flags: needinfo?(ktucker)
Keywords: verifyme
Adding verifyme because 2.0M needs to be verified. We do not have the appropriate device to check this.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Keywords: verifyme

Comment 36

3 years ago
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
status-b2g-v2.0M: fixed → verified

Comment 37

3 years ago
Created attachment 8522761 [details]
Verify_Woodduck_SMS.MP4

Updated

3 years ago
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.