Closed Bug 1011654 Opened 10 years ago Closed 10 years ago

[B2G][SMS]Opening the messages app after sending a message from the Call log causes the sent notification sound to occur a second time

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::SMS
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:1.4+, b2g-v1.3 unaffected, b2g-v1.4 fixed, b2g-v2.0 fixed)

Status:
RESOLVED FIXED
Milestone:
2.0 S3 (6june)
Project Flags:
blocking-b2g 1.4+
Tracking Flags:
Tracking Status
b2g-v1.3 --- unaffected
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: astole, Assigned: azasypkin)

References

(
URL
)

Details

(Keywords: regression, Whiteboard: [p=1])

Attachments

(3 files)

logcat
243.62 KB, text/plain
Details
GitHub pull request URL (v1.4)
46 bytes, text/x-github-pull-request
steveck
: review+
Details | Review
GitHub pull request URL
46 bytes, text/x-github-pull-request
steveck
: review+
Details | Review
Attached file logcat — 
Opening the messages app after sending a message from the call log triggers the sent notification sound again if a message has been previously sent to the same recipient.  This also occurs if the second message is sent from contact details. On some occasions, the sound occured twice immediately after sending the message from the call log.

Repro Steps:
1) Update a Open_C to BuildID: 20140516040203
2) Have a number in the Call log or a Contact
3) Open messages app and send a message to the number from Step 2
4) Press the home button
5) Open Call log or Contacts and send a message to the number from Step 3
6) Press the home button
7) Open the messages app

Actual:
The sent message notification sound occurs when opening the messages app

Expected:
The sound notification does not occur when opening the messages app

2.0 Environmental Variables:
Device: Open_C 2.0 MOZ
BuildID: 20140516040203
Gaia: 7973e06dc278f67b4109ac3c33020ed086f0d042
Gecko: 58c5a3427997
Version: 32.0a1
Firmware Version: P821A10v1.0.0B06_LOG_DL

Repro frequency: 2/2, 100%
See attached: video, logcat
This issue does not occur on the latest 1.4

1.4 Environmental Variables:
Device: Open_C 1.4
BuildID: 20140516000201
Gaia: 32fca83da31b9a0f9a5a88f96c913a25accdc14b
Gecko: a1e455367fa6
Version: 30.0
Firmware Version: P821A10V1.0.0B06_LOG_DL

Adding qawanted to check 2.0 on Buri.
URL: http://youtu.be/0UgEUXQJdbA
status-b2g-v1.4: --- → unaffected
status-b2g-v2.0: --- → affected
Keywords: qawanted
QA Contact: jmercado
This issue does occur on Buri 2.0

2.0 Environmental Variables:
Device: Buri 2.0 MOZ
BuildID: 20140516052831
Gaia: 799b0f8bb71bc1b944f90c117ab5d6be4837ba1f
Gecko: b5b35ec88db8
Version: 32.0a1
Firmware Version: v1.2-device.cfg

An additional notification sound can be heard.
Keywords: regression
blocking-b2g: --- → 2.0?
Keywords: regressionwindow-wanted
Hey,

just want to make sure you hear the sound notification in step 3 and 5 too.
Flags: needinfo?(astole)
(In reply to Julien Wajsberg [:julienw] from comment #3)
> Hey,
> 
> just want to make sure you hear the sound notification in step 3 and 5 too.

The video does show that the sound is heard.
Flags: needinfo?(astole)
I was able to reproduce this issue on the 1.4 build in comment 1 on an OpenC with the actual results listed in comment 0 where the sound immediately plays twice.  Builds are not available far enough back to get a more precise window.

Mozilla Nightly Central Regression Window

Last Working environment variables
Device: Buri 1.3 MOZ
BuildID: 20131101040203
Gaia: ccdf357ea150fc7d8b8a4b74c7adf31e7a57e465
Gecko: abe6790a5dd8
Version: 28.0a1
Firmware Version: v1.2-device.cfg

First broken environment variables
Device: Buri 1.3 MOZ
BuildID: 20131102040200
Gaia: 50f0c914fa5cb89694c0cdd41b6e10b714787070
Gecko: 396e59370945
Version: 28.0a1
Firmware Version: v1.2-device.cfg


Last working gecko / First broken gaia - occurs - Indicates a Gaia issue
Gaia: 50f0c914fa5cb89694c0cdd41b6e10b714787070
Gecko: abe6790a5dd8


First broken gecko / Last working gaia - does not occur
Gaia: ccdf357ea150fc7d8b8a4b74c7adf31e7a57e465
Gecko: 396e59370945

Gaia Pushlog: https://github.com/mozilla-b2g/gaia/compare/ccdf357ea150fc7d8b8a4b74c7adf31e7a57e465...50f0c914fa5cb89694c0cdd41b6e10b714787070
blocking-b2g: 2.0? → 1.4?
status-b2g-v1.3: --- → unaffected
status-b2g-v1.4: unaffected → affected
There's three SMS bugs in the range - bug 815093, bug 927784, and unknown bug # (https://github.com/mozilla-b2g/gaia/commit/d82ecd50783cc79121b25f24f899b9df2f702c7c).

Julien - Which one looks like the cause of the regression here?
Flags: needinfo?(felash)
bug 815093 for sure: yet another inline activity change regression.

we need to move the part that plays the sound from ThreadUI.onMessageSent to the onsuccess send/sendMMS callback.
Flags: needinfo?(felash)
Blocks: 815093
blocking-b2g: 1.4? → 1.4+
Hey Oleg, any chance you can have a look at this?
Flags: needinfo?(azasypkin)
Sure, will look into it.
Assignee: nobody → azasypkin
Status: NEW → ASSIGNED
Flags: needinfo?(azasypkin)
Blocks: sms-sprint-1
Whiteboard: [p=1][not-part-of-initial-sprint]
Target Milestone: --- → 2.0 S2 (23may)
Attachment #8426456 - Flags: review?(felash)
Comment on attachment 8426456 [details] [review]
GitHub pull request URL (v1.4)

handing over the review to Steve as I'm fairly overloaded.
Attachment #8426456 - Flags: review?(felash) → review?(schung)
Blocks: sms-sprint-2
No longer blocks: sms-sprint-1
Whiteboard: [p=1][not-part-of-initial-sprint] → [p=1]
Target Milestone: 2.0 S2 (23may) → 2.0 S3 (6june)
Comment on attachment 8426456 [details] [review]
GitHub pull request URL (v1.4)

Some nits on github but overall is good, so r=me :)
BTW, will you also submit another patch for master later?
Attachment #8426456 - Flags: review?(schung) → review+
Please review this master patch too as I've rebased it on master and it differs from v1.4 version now.
Attachment #8429080 - Flags: review?(schung)
(In reply to Oleg Zasypkin [:azasypkin] from comment #13)
> Created attachment 8429080 [details] [review]
> GitHub pull request URL
> 
> Please review this master patch too as I've rebased it on master and it
> differs from v1.4 version now.

Hey Oleg, since the patch will change the parameter of resendMessage, would you mind adding a test for it? Thanks!
(In reply to Steve Chung [:steveck] from comment #14)
> (In reply to Oleg Zasypkin [:azasypkin] from comment #13)
> > Created attachment 8429080 [details] [review]
> > GitHub pull request URL
> > 
> > Please review this master patch too as I've rebased it on master and it
> > differs from v1.4 version now.
> 
> Hey Oleg, since the patch will change the parameter of resendMessage, would
> you mind adding a test for it? Thanks!

Sure! I just wanted to confirm first that you're OK with the method parameter changes, I'll add some tests for resendMessage.
(In reply to Steve Chung [:steveck] from comment #14)
> (In reply to Oleg Zasypkin [:azasypkin] from comment #13)
> > Created attachment 8429080 [details] [review]
> > GitHub pull request URL
> > 
> > Please review this master patch too as I've rebased it on master and it
> > differs from v1.4 version now.
> 
> Hey Oleg, since the patch will change the parameter of resendMessage, would
> you mind adding a test for it? Thanks!

Added more tests! Please, take a look :)

Thanks!
Flags: needinfo?(schung)
Only one comment about using the mock message ;)
Flags: needinfo?(schung)
(In reply to Steve Chung [:steveck] from comment #17)
> Only one comment about using the mock message ;)

Thanks for review! Added mock message as you've suggested and waiting for green Travis.
Comment on attachment 8429080 [details] [review]
GitHub pull request URL

Thanks for the fixing and r=me. I trigger the travis again since the test failed in other module. It should be fine to land the patch if the test still failed in unrelated test.
Attachment #8429080 - Flags: review?(schung) → review+
in master: 93402e84de5a9ae3824aead5fb03ce9be984f09e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-b2g-v2.0: affected → fixed
Resolution: --- → FIXED
in v1.4: 7bc1c15c67661a0b8e35f18f15a9d03d1d2cfcd5
status-b2g-v1.4: affected → fixed
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: