Usage message with sound

VERIFIED FIXED in Firefox OS v2.0

Status

Firefox OS
Gaia::Cost Control
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: lolimartinezcr, Assigned: mai)

Tracking

unspecified
2.0 S6 (18july)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.0+, b2g-v2.0 verified, b2g-v2.1 verified)

Details

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
salva
: review+
Details | Review | Splinter Review
(Reporter)

Description

4 years ago
Flame
Platform versión: 32.0a2
Build ID: 20140630073320
Git commit: c0c8ad18

Reproducible: Not 100%

Pre-requisites: 
Prepaid with billing support SIM card

STR:
1. Tap usage application.
2. Tap "update" button

Actual result:
When user receive sms with balance it is with *sound*.

Expected reuslt
When user receive sms with balance it is without *sound*.
Loli, when you write "Reproducible: Not 100%", does it mean "near 100%" or less than this?
Flags: needinfo?(lolimartinezcr)
Also, it would be nice to know if the SMS app is launched in background or not. Can you please try both these cases?
(Reporter)

Comment 3

4 years ago
(In reply to Julien Wajsberg [:julienw] (away June 21 to 30) from comment #1)
> Loli, when you write "Reproducible: Not 100%", does it mean "near 100%" or
> less than this?

Sorry, when i write "Not 100%" it mean less tan this. Not always is reproducible. Marina have seen this bug with me.
Flags: needinfo?(lolimartinezcr)
(Reporter)

Comment 4

4 years ago
(In reply to Loli from comment #3)
> (In reply to Julien Wajsberg [:julienw] (away June 21 to 30) from comment #1)
> > Loli, when you write "Reproducible: Not 100%", does it mean "near 100%" or
> > less than this?
> 
> Sorry, when i write "Not 100%" it mean less tan this. Not always is
> reproducible. Marina have seen this bug with me.

Also, SMS app is launched in background
> Sorry, when i write "Not 100%" it mean less tan this. Not always is reproducible. Marina have seen this bug with me.

I meant: does it happen 25%, 50%, 75%, 99%?

When you have the sound, you still don't have a notification, right ?
Then the only code that does this is [1] but we should not run it if the number is made silent by Cost Control.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/activity_handler.js#L387-L388
(Reporter)

Comment 6

4 years ago
(In reply to Julien Wajsberg [:julienw] (away June 21 to 30) from comment #5)
> > Sorry, when i write "Not 100%" it mean less tan this. Not always is reproducible. Marina have seen this bug with me.
> 
> I meant: does it happen 25%, 50%, 75%, 99%?
> 
> When you have the sound, you still don't have a notification, right ?
> Then the only code that does this is [1] but we should not run it if the
> number is made silent by Cost Control.
> 
> [1]
> https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/activity_handler.
> js#L387-L388

does it happen 25%

When i have the sound when I have received a notification in utility try.
Ok, so you have a real notification in that case ?

(sorry, I try to be as complete as possible)
(Reporter)

Comment 8

4 years ago
Also Tested and it bug is reproducible:
Hamachi
2.1
Gecko-deb9478
Gaia-c086196
blocking-b2g: --- → 2.0?
Loli - Can you provide a video so that we can understand the sound used here?
Flags: needinfo?(lolimartinezcr)
Keywords: qawanted
(Assignee)

Comment 10

4 years ago
I cannot reproduce it with:
Device: flame
OS: 2.1
Platform versión: 33.0a1
Build ID: 20140703063604
Git commit: f456c886
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1032713
(Assignee)

Comment 12

4 years ago
Hi, 
At last, I've can reproduce the bug. This bug is not related with the silent sms feature. I found that we're receive two balance response messages. Then the first is silent but the second it isn't. This behavior is not correct.

I think the origin of the problem is related with the automatic balance request feature of costcontrol, This feature consist on the costcontrol app performs an automatic balance request in background when the time elapsed since the last request is greater than that reflected in the minimun_delay config var. I have to check why the update button is not locked when the background request is working. 

Regards
Flags: needinfo?(lolimartinezcr)
triage: 2.0+ per comment 12.
blocking-b2g: 2.0? → 2.0+
Another solution is to keep track of the various requests and to not "unsilent" the phone number until all requests were answered.
Assignee: nobody → mri
Target Milestone: --- → 2.0 S6 (18july)
(Assignee)

Comment 15

4 years ago
Created attachment 8452953 [details] [review]
patch v1.0

Hi Salva,
would you mind reviewing this patch?

On this patch, I added a flag to not allow send a balance request if another is being sended. And updating the property lastBalanceRequest when the balance timeout alarms is not added (because the request is sended correctly)

Regards
Attachment #8452953 - Flags: review?(salva)
Comment on attachment 8452953 [details] [review]
patch v1.0

The bug is related with a mistake I made consisting on combining the already-existing in-progress lock with the alarm ID used for the timeout. Let's use the already-existing lock instead. It is the setting called `waitingForBalance` (which now stores the alarm id for timeout too).
Attachment #8452953 - Flags: review?(salva)
(Assignee)

Comment 17

4 years ago
Comment on attachment 8452953 [details] [review]
patch v1.0

Updated the PR.
Would you mind reviewing the patch?
Regards
Attachment #8452953 - Flags: review?(salva)
Comment on attachment 8452953 [details] [review]
patch v1.0

I'm not sure about the solution here. Could you review my comments on GitHub?
Attachment #8452953 - Flags: review?(salva)
(Assignee)

Comment 19

4 years ago
Comment on attachment 8452953 [details] [review]
patch v1.0

Salva, would you review the patch again?
Regards
Attachment #8452953 - Flags: review?(salva)
Comment on attachment 8452953 [details] [review]
patch v1.0

As agreed offline, please comment the problems here and open a follow-up to discuss further solutions.

Change the name of the flag by `isSendingBalanceRequest`. Thank you for the patch, Mai!
Attachment #8452953 - Flags: review?(salva) → review+
(Assignee)

Comment 21

4 years ago
Hi,
this patch provides a solution for the most common of the cases. When the user ask manually for a balance request when anothe request is sent in background.

Although the app has a lock to prevent this kind of situations, the non-transactional nature of AsyncStorage produces that the lock does not work fine when two balance request are asked at the same time. Besides the costcontrol app has the handicap of its architecture, the app hast two UI (app and widget) that complicates find a easy solution like a queue or a request counter (as Julien suggestion on comment 14), because this solution would also have the same problem of the current lock implemented at  asyncStorage level.

Currently on gaia dev list is being arguing how to proceed with this kind of problems, you can follow the progress through the following link:
https://groups.google.com/d/msg/mozilla.dev.gaia/g5_eOE7YHWw/s8dH40HC8lsJ (Non-transactional nature of AsyncStorage)

This bug still could be reproduced (although the probability is low), if the user interacts with the balance section of usage app at the same time that slide down the widget bar, and the minimun delay has been passed (currently the minimun delay is 3 hours in all the config of the countries). This kind of scenario only could be reproduced with the interaction of the user, and when both ui are involved.

Regards
(Assignee)

Comment 22

4 years ago
Master: af72c579ace4cbe1cdd7fd6e50abcb0a56cb0ae2
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
status-b2g-v2.1: --- → fixed
issue is resolved, do not believe QA-wanted tag is still needed.
Keywords: qawanted
(Reporter)

Comment 25

4 years ago
tested and working
Flame
2.0
Gecko-5287d8d
Gaia-8cb1a94

2.1
Gecko 64bhb3bd
Gaia 89fd655
Status: RESOLVED → VERIFIED
status-b2g-v2.0: fixed → verified
status-b2g-v2.1: fixed → verified
You need to log in before you can comment on or make changes to this bug.