1205874 - [Messages] After starting a new conversation from previously saved draft, that conversation will be counted twice when using Select All in Inbox

Status: VERIFIED FIXED

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

Description

[Max Ivanov[:MaxIvanov]]

Attachment: logs.txt

Description:
After sending a message with a vCard, messages thread counter works wrong user is notified incorrectly. 

Repro Steps:
1) Update a Flame to 20150917030226
2) Open Message app and delete all messages from history  
3) Send message with vCard attached
4) Tap on the "back" button in top left corner
5) Tap on the "three dots" menu button
6) Tap on "Select Threads"
7) Tap on the "Select all"
8) Observe action bar

Actual:
User is notified that 2 messages is selected

Expected:
Since there is only one thread user should be notified about 1 selected thread

Environmental Variables:
Device: Flame 2.5
Build ID: 20150917030226
Gaia: db6664f0e07e9966283d30cfc7006151fe7103ff
Gecko: e7d613b3bcfe1e865378bfac37de64560d1234ec
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 43.0a1 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Repro frequency: (100%)
See attached: (video clip, logcat)
https://youtu.be/TZI_SJ3sO8Q

Comment 1

[Max Ivanov[:MaxIvanov]]

This issue DOES occur on Flame 2.5 and Aries 2.5 and does NOT on Flame 2.2 
Result: A user is notified incorrectly

Device: Flame 2.5Kk Full flash (319mb)
Build ID: 20150917030226
Gaia: db6664f0e07e9966283d30cfc7006151fe7103ff
Gecko: e7d613b3bcfe1e865378bfac37de64560d1234ec
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 43.0a1 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0
-------------------------------------------------------------------

Device: Aries 2.5
Build ID: 20150917133514
Gaia: aede8622d780ec71f766a3ecccbff74c04aaba4e
Gecko: 8d4c37a86b576db4ef150dff715a89ff77e84bf5
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Comment 2

[Julien Wajsberg [:julienw]]

Wow this is indeed a good bug.
I'm not so sure this is related to sending a vcard though, can we try to find a reduced STR ?

Comment 3

[Mahendra Potharaju [:mahe]]

Blocking for 2.5 with a P3.

Comment 4

[Martin Shuman [:Marty]]

This issue occurs whenever a new thread is started with a sent MMS message with any attachment. The new thread appears to be double counted. It does not seem to be counting previously deleted threads.

Notes:
-The MMS must have an attachment.  Just having a Subject is not enough
-This issue will occur with any new thread that starts with a sent MMS message, even if other threads are present.
-This issue will not occur if an MMS message is sent in an existing SMS thread.
-This issue will not occur with new threads started by receiving an MMS, only when sending an MMS.
-Closing and reopening the Messages app will cause this issue to stop occurring until the next new thread is started with an MMS message.

Repro Steps:
1) Update a Flame to 20150918030223
2) Open the messages app and compose and send a new MMS message with an attachment.
3) Navigate back to the thread view and tap the ellipsis (...) and tap 'Select Threads'
4) Tap 'Select All'

I'm going to move on to find the regression window for this issue.

Comment 5

[Martin Shuman [:Marty]]

This issue appears to be caused by:
Bug 1144132 - [Activity] Properly adjust the priority of activity openers to prevent them from being killed by the LMK when they are hidden

B2g-inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 2.5
BuildID: 20150622170244
Gaia: c4d3b8fd78bcd7eff1b127060ec6490a891b7a35
Gecko: 2ab4034b2cc1
Version: 41.0a1 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

First Broken 
Environmental Variables:
Device: Flame 2.5
BuildID: 20150623190325
Gaia: eb0d4aefa62b20420d6fa0642515a110daca5d97
Gecko: 5a3ee6baf143
Version: 41.0a1 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Last Working gaia / First Broken gecko - Issue does NOT reproduce
Gaia: c4d3b8fd78bcd7eff1b127060ec6490a891b7a35
Gecko: 5a3ee6baf143

First Broken gaia / Last Working gecko - Issue DOES reproduce
Gaia: eb0d4aefa62b20420d6fa0642515a110daca5d97
Gecko: 2ab4034b2cc1

Gaia Pushlog:
https://github.com/mozilla-b2g/gaia/compare/c4d3b8fd78bcd7eff1b127060ec6490a891b7a35...eb0d4aefa62b20420d6fa0642515a110daca5d97

Comment 6

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

Julien, Alive is no longer working on this product.  Do you know who should take a look at this now?

Comment 7

[Julien Wajsberg [:julienw]]

This is very surprising that Alive's patch is causing this.

I think it's best if the SMS team investigates this for now. Because it's a blocker it will stay on our radar so no need to NI anybody right now, we'll follow up.

Comment 8

[Julien Wajsberg [:julienw]]

One change in Alive's patch could affect us: the call to "setVisible".

And actually I see there is a test change in SMS so there was likely an impact. Still I fail to see how this could provoke _this_ bug :)

Comment 9

[(inactive after 6/18) Alive Kuo [:alive]]

It is very likely you are doing something when document is hidden, for example, saving the draft?
And it was always not hidden before my change, so it won't trigger this when invoking the contact activity.

The test change should be not related.

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

BTW, I'd dropped a mail to dev-gaia to notify the change about document.hidden when using activity :)

Comment 11

[Julien Wajsberg [:julienw]]

Yeah, I remember this; you're right, your test change is simply that we're saving the draft when we're hidden.

Anyway, we'll investigate, thanks for your message :)

Comment 12

[Oleg Zasypkin [:azasypkin]]

So as mentioned in comment 9, bug 1144132 just revealed bug we have for about 6 months already. Reduced STR will be:

1) Open New Message view;
2) Enter some content (doesn't matter SMS or MMS) and press Home button (to save a draft); 
3) Return back to Messages app and send message;
4) Tap on the "back" button in top left corner
5) Tap on the "three dots" menu button
6) Tap on "Select Threads"
7) Tap on the "Select all"
8) Observe action bar

The issue is that in the patch for bug 1084298 we started to register newly saved Draft in Threads collection that is used as the model for our selection logic, but once message is sent we don't clean Threads collection properly that leads to incorrect number of _all_ selected threads.

I'll take a look at the issue closely tomorrow.

Comment 13

[GitHub Autolander Bot]

Attachment: [gaia] azasypkin:bug-1205874-delete-draft-from-threads > mozilla-b2g:master

Comment 14

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8666004 [details] [review]
[gaia] azasypkin:bug-1205874-delete-draft-from-threads > mozilla-b2g:master

Hey Steve,

Here is the simplest and safest fix I can think of :) How does it look?

I just call "deleteThread" instead of "removeThread" as we do for other cases when we remove thread-less draft.

Thanks!

Comment 15

[Steve Chung [:steveck]]

Comment on attachment 8666004 [details] [review]
[gaia] azasypkin:bug-1205874-delete-draft-from-threads > mozilla-b2g:master

Thanks for finding the real root cause. I'm fine with reusing the deletThread since we also apply this for batch thread deletion and we can improve this in another bug.

Comment 16

[Oleg Zasypkin [:azasypkin]]

(In reply to Steve Chung [:steveck] from comment #15)
> Comment on attachment 8666004 [details] [review]
> [gaia] azasypkin:bug-1205874-delete-draft-from-threads > mozilla-b2g:master
> 
> Thanks for finding the real root cause. I'm fine with reusing the
> deletThread since we also apply this for batch thread deletion and we can
> improve this in another bug.

Thanks for review! Nit fixed, Treeherder is green, so landed.

Master: https://github.com/mozilla-b2g/gaia/commit/794a3ae447417edc1513d0fd6362f9e2bbaf0d94

Comment 17

[Jenny (Leave from Mozilla)]

Attachment: AriesKK_v2.5.3gp

This bug has been verified as "pass" on the latest build of Flame KK 2.5 and Aires KK 2.5 by the STR in comment 0 & comment 4 & comment 12.

Actual results: Since there is only one thread user can be notified about 1 selected thread.
See attachment: AriesKK_v2.5.3gp
Reproduce rate: 0/10

Device: Flame KK 2.5 (Pass)
Build ID               20151009150221
Gaia Revision          1609aecaba381c14eff95d5084e59280f53b7d8b
Gaia Date              2015-10-09 07:16:19
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/c00e93135684441d065791f4f98a338b872d0420
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151009.183801
Firmware Date          Fri Oct  9 18:38:15 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK 2.5 (Pass)
Build ID               20151010002512
Gaia Revision          74b0d4b17f39d238a7997800bd9363d3c60f20c3
Gaia Date              2015-10-09 19:27:39
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/d1bb0de19476541cd517ab14017e7fedbd9f13e3
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151009.234251
Firmware Date          Fri Oct  9 23:42:59 UTC 2015
Bootloader             s1