Closed Bug 880257 Opened 9 years ago Closed 9 years ago

[SMS][MMS] Implement a "Device without SIM" message when attempting to send messages without a SIM card

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed)

VERIFIED FIXED
1.1 QE3 (26jun)
blocking-b2g leo+
Tracking Status
b2g18 --- fixed
b2g-v1.1hd --- fixed

People

(Reporter: isabelrios, Assigned: evanxd)

References

Details

(Keywords: late-l10n, Whiteboard: MMS_TEF,[TD-42357], leorun4, [POVB], [u=commsapps-user c=messaging p=0],[LeoVB+])

Attachments

(5 files)

Bug seen on Unagi v1-train
Gecko-65bbcee
Gaia-13c6246

PROCEDURE
1. With the device without SIM Card, try to send an MMS

EXPECTED
There should be an error message or any kind of indication to let user know that the MMS can not be sent.

ACTUAL
The device keeps trying to send the MMS, the spinner is shown and no error notification appears.
This sounds similar to a bug I think ctai is working on - adding needinfo to confirm.

ctai - if this is a dupe, could you close it appropriately please?
Flags: needinfo?(ctai)
Yes, pretty sure this is a dupe and we marked it blocking minus.

Please look at existing bugs when filing new ones.
No, I am not working on this.
Flags: needinfo?(ctai)
For more clarify, after 877064 land, the user will see the spin icon for about 15 minutes(because of retry). Then the error icon will show up. I will work on 880561. If this bug land, it will not go to retry and return the error quickly with error code. Gaia developer can decide to notify the user no sim card/flight mode or not.
Please add a prefix "bug" for the bug number so the Bugzilla will show a link for it. For example, bug 880561.
Adding attachment to show what is happening now. 
For SMS there is an error, for MMS, the spinner and no error is shown.
Blocking for now, MMS work.
blocking-b2g: leo? → leo+
Bug 876799 is focused on the same issue but it's nominate as non-blocking issue... If this is really a blocking issue, we can close this one as duplicate and change Bug 876799 to blocking because it already has a patch reviewing.
Hi,

Bug 876799 is focused on having some alert a part from the current failure icon when sending a SMS without SIM card (similar behavior than airplane mode). In this case, the issue is more focused on showing at least the failure icon when sending a MMS without a SIM card inserted, currently under this scenario the spinner icon is continuously shown, failure icon never appears. Because of this, we still consider this issue as blocking one since the same behavior under no SIM scenario needs to be implemented for SMS and MMS. 

Showing a specific alert for this scenario would be a "nice to have", we could track it for both, SMS and MMS, in Bug 876799.
I agree with Noemi, what is important is that whenever an error is permanent (no SIM Card, Flight Mode, etc...) the message is directly marked as not possible to be sent (e.g. with the exclamation mark) and not showing the "trying to send" spinner we are currently showing for MMS under those conditions.
No longer depends on: 880561
See Also: → 880561
Assignee: nobody → mike
I believe this to be a bug in Gecko. The attached patch adds some simple debugging statements to demonstrate that the expected 'failed' event is not fired when sending MMS messages from a device with no SIM card installed.

In my own testing, I found additional information that might be useful:

If the message recipient is an invalid number (i.e. the string "a"), then the 'failed' event is fired as expected, and the UI updates accordingly. This bug only seems to occur when sending MMS messages to numeric recipients.
Flags: needinfo?(ctai)
Mike, please see bug 880561 for more information. I don't know why Maria the dependency, but I think this bug should depend on bug 880561. You can also give some suggestions on bug 880561. Thanks.
Flags: needinfo?(ctai)
This one should be a dupe of bug 880561. I believe we can close this one.
After talk with Gene, I prefer left this bug for Gaia developers do the error handling in Gaia side. Let bug 880561 be gecko part bug. So do not close this one.
(In reply to Gene Lian [:gene] from comment #14)
> This one should be a dupe of bug 880561. I believe we can close this one.
Depends on: 880561
OS: Windows 7 → Gonk (Firefox OS)
Hardware: x86_64 → ARM
See Also: 880561
Mike! I've seen that in Gecko they are working hard (Thanks Vicamo and Gene! ;) ) for getting new error status. Please Could you add me as reviewer of this patch for checking that the behavior is the expected by UX Team? THanks!
Hey Borja,

The patch I've attached simply adds a debugging statement--it does not implement any functional change.

There seems to be some confusion about what this Bug is actually intended to track.

(In reply to Maria Angeles Oteo (:oteo) from comment #11)
> I agree with Noemi, what is important is that whenever an error is permanent
> (no SIM Card, Flight Mode, etc...) the message is directly marked as not
> possible to be sent (e.g. with the exclamation mark) and not showing the
> "trying to send" spinner we are currently showing for MMS under those
> conditions.

According to this interpretation, there is no change to Gaia necessary. As soon as bug 880561 is resolved, messages sent from a device without a SIM card will display as "failed" immediately. (This was my understanding, and it is why the attached patch file simply adds a debugging statement.)

Completely separate is the new functionality of a specialized error message such as, "Unable to send message because no SIM card is present." I am re-naming the bug to make this more obvious.

Lukas: Since there has been some confusion regarding the intention of this bug, I want to confirm that the new functionality is indeed what you had in mind when you marked this bug as "Leo+".
Flags: needinfo?(lsblakk)
Summary: [MMS] No error shown when trying to send a MMS (device without SIM) → [MMS] Implement a "Device without SIM" message when attempting to send messages without a SIM card
Basically here we are going to handle both errors ('no sim' and 'flight mode') in the same method in Gaia code, so we need a little bit of coding in Gaia for having these 'prompts' working as expected in SMS and MMS. That's why we need this bug in Gaia as leo+. Thanks a lot Mike! Gracias;)
Flags: needinfo?(lsblakk)
It looks like similar functionality is almost complete in Bug 876799 (which Noemi previous recommended)

(In reply to Noemí Freire (:noemi) from comment #10)
> Showing a specific alert for this scenario would be a "nice to have", we
> could track it for both, SMS and MMS, in Bug 876799.

That bug implements the most of the behavior without requiring a patch to Gecko. (It does this by querying `navigator.mozMobileConnection.connState` whenever a message fails to send.)

This bug is *not* a duplicate, however, because bug 876799 does not specify a specialized "flight mode" message. We'll need the patch in bug 880561 to land before we can implement that.

Again, I have to wonder if this bug is truly "Leo+". It extends functionality that was explicitly denied that distinction. I was hoping to get Lukas's feedback on that.
Depends on: 876799
(In reply to Mike Pennisi [:jugglinmike] from comment #19)
> Again, I have to wonder if this bug is truly "Leo+". It extends
> functionality that was explicitly denied that distinction. I was hoping to
> get Lukas's feedback on that.
As Borja said, due to the work being done in gecko side (Bug 880561)there will be new error status for covering "no sim" and "flight mode" scenarios. Currently, a specific message under "flight mode" is shown for SMS but not in case of MMS so in order to make messaging app consistent the same behavior should be implemented across SMS/MMS. Because of this reason, we need gaia side as leo+.
Since "no sim" status is going to be also specifically reported (failure icon would be immediately shown), it would be nice to also show a specific message covering this scenario for SMS/MMS. 
Maybe it is a good idea to rename the summary of this Bug in order to reflect this.
(In reply to Borja Salguero [:borjasalguero] from comment #16)
> Mike! I've seen that in Gecko they are working hard (Thanks Vicamo and Gene!
> ;) ) for getting new error status.

That bug is actually taken by Chia-hung. I just supported the review. Thanks Chia-hung!
Duplicate of this bug: 876799
@Mike: We have discovered that Evan was making a patch related with this bug. I would like to sync. with Evan in order to move the assignment if we agree (because he was working on this), Wdyt? 

On the other hand Gecko's patch is on the right way, so it should be landed soon :).
Thanks!
Flags: needinfo?(mike)
I suppose this makes sense, Boja. In comment 19, I made this bug dependent on bug 876799. I was planning on re-factoring Evan's work as soon as it landed, but it might be more efficient for Evan to update his work to use the new Gecko functionality.
Flags: needinfo?(mike)
Whiteboard: MMS_TEF → MMS_TEF,[TD-42357]
Target Milestone: --- → 1.1 QE3 (24jun)
Priority: -- → P2
Hi Mike,

I think I could continue to update my patch when Bug 880561 is landed.
So could I take this bug?

After we have new error messages for flight mode and no sim card,
we don't need the ifRilDisabled function to check the networking in https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/thread_ui.js#L1372.

And I don't need to check the SIM card state with navigator.mozMobileConnection.cardState in my patch https://github.com/mozilla-b2g/gaia/pull/10130/files#L0R1270.

Thanks.
Hey Evan,

Yes, it sounds like we're all on the same page now :) This approach works for me, so I am re-assigning the bug to you.
Assignee: mike → evanxd
Please add late-l10n to the bug keywords if this will require new strings.
Flags: needinfo?(evanxd)
Flags: needinfo?(evanxd)
Keywords: late-l10n
Hi all,

I found out that when device sent SMS successfully or unsuccessfully, the request.onsuccess or request.onerror was never triggered in https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/message_manager.js#L405.

I think it might be a gecko issue.
So We could get error message from the request.onerror function to fix this bug.
And Vicamo and I are checking this.
Whiteboard: MMS_TEF,[TD-42357] → MMS_TEF,[TD-42357], TaipeiWW
(In reply to Evan Tseng [:evanxd] from comment #28)
> Hi all,
> 
> I found out that when device sent SMS successfully or unsuccessfully, the
> request.onsuccess or request.onerror was never triggered

We didn't solve this issue for SMS, for MMS instead. I believe the MMS part is working.
(In reply to Gene Lian [:gene] from comment #29)
> (In reply to Evan Tseng [:evanxd] from comment #28)
> > Hi all,
> > 
> > I found out that when device sent SMS successfully or unsuccessfully, the
> > request.onsuccess or request.onerror was never triggered
> 
> We didn't solve this issue for SMS, for MMS instead. I believe the MMS part
> is working.

To clarify: what I'm saying is for the Gecko part. Gaia part also needs to catch the error result to have some corresponding behaviours. The Gaia part is not yet done.
I just add some log and confirm that the problem should be you add the log in wrong place....
Successful:
06-21 11:33:39.701 I/Gecko   (  327): ###################################### forms.js loaded
06-21 11:33:39.821 E/GeckoConsole(  434): Content JS LOG at app://sms.gaiamobile.org/js/message_manager.js:430 in onSuccess: @@ Successfully send MMS.
Error:
06-21 11:36:30.968 E/GeckoConsole(  434): Content JS LOG at app://sms.gaiamobile.org/js/message_manager.js:435 in onError: Error Sending: undefined

The undefined is caused by you guys use wrong implement on the function |onError|.
Please see https://developer.mozilla.org/en-US/docs/Web/API/DOMRequest.
Both onsuccess and onerror should be a function take no argument. |event| you used in your code is an empty object. You guys should use this.error.name.
(In reply to Chia-hung Tai [:ctai :ctai_mozilla :cht] from comment #31)
> The undefined is caused by you guys use wrong implement on the function
> |onError|.
> Please see https://developer.mozilla.org/en-US/docs/Web/API/DOMRequest.
> Both onsuccess and onerror should be a function take no argument.

Actually the .onsuccess and .onerror can eat an event argument. Both the following two ways are valid usages:

- request.onerror = function (event) { alert(event.target.error.name); };
- request.onerror = function () { alert(request.error.name); };

Anyway, |event.error| is not allowed.
Depends on: 885652
Hi Borja,

Please help me review the patch.
Thanks. :)
Attachment #765888 - Flags: review?(fbsc)
I'd suggest let's solve Bug 885652 first and properly handle the SMS part as well in the Gaia part.
+1. Gene let us know when Gecko part will be ready and we will move forward with Gaia one. Thanks!
Comment on attachment 765888 [details]
Point to the GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10536

Evan, as Gene commented in the bug, we are going to wait until having Gecko part working (bug 885652) for taking a look to the Gaia part ok? Thanks!
Attachment #765888 - Flags: review?(fbsc)
Hi Borja,

OK, let us wait for that Bug 885652 is landed, and I can update the patch.

The we could review the patch.

Thanks. :)
Hi Leo,

Because of Comment 36, could we change the Target Milestone as QE4?

Thanks. :)
Flags: needinfo?(leo.bugzilla.gaia)
Comment on attachment 765888 [details]
Point to the GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10536

Taking a look while waiting Gene's patch :)
Attachment #765888 - Flags: review?(fbsc)
Comment on attachment 765888 [details]
Point to the GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10536

I would like to have Francisco for taking a look to the Custom Dialog thing. Thanks!
Attachment #765888 - Flags: review?(francisco.jordano)
Hi Evan,

I found the root cause why the onsuccess/onerror of SMS is not returned. It's due the wrong Gaia usage of calling SMS .send(...) API:

  request = this._mozMobileMessage.send(recipients, content);
  request.onsuccess = function onSuccess(event) {...};
  request.onerror = function onError(event) {...};

If the |recipients| is an array, the returned value is not a DOM request! Instead, It's an *array* of DOM requests. Anyway, we still need bug 885652 to support returning the error codes for the cases of the SIM card is not installed and the radio is not ready.
Summary: [MMS] Implement a "Device without SIM" message when attempting to send messages without a SIM card → [SMS][MMS] Implement a "Device without SIM" message when attempting to send messages without a SIM card
Comment on attachment 765888 [details]
Point to the GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10536

Please see comment #41. We also need to support the SMS part in a more correct way (i.e. onsuccess/onerror) instead of depending on the "sms-sent" or "sms-failed" events.
Attachment #765888 - Flags: review-
Bug 885652 is under the way to land to central. Need Dietrich's help to land it to b2g18.
(In reply to Gene Lian [:gene] from comment #43)
> Bug 885652 is under the way to land to central. Need Dietrich's help to land
> it to b2g18.

Given Bug 885652 recently landed, what are the next steps for this bug to get resolved ?
Seems like we should always force .send(recips, msg) to be an array, and then use forEach on the result to append the success/errors?
(In reply to Corey Frang [:gnarf] [:gnarf37] from comment #45)
> Seems like we should always force .send(recips, msg) to be an array, and
> then use forEach on the result to append the success/errors?

If the |recips| is a string, then the .send(...) simply returns a dom request. It only returns an array of dom requests when the |recips| is an array of strings.
Right - In order to make it as simple as possible in gaia, it would be best to force |recips| to be an array, so we know the return value will always be an array of dom request.
Flags: needinfo?(leo.bugzilla.gaia)
Hi all,

Because we'll remove CustomDialog form shared lib(see here: https://github.com/mozilla-b2g/gaia/pull/10536/files#L3R1397), the patch for this bug is depended on Bug 880624.

We need to review and land the patch after Bug 880624 is fixed.
Depends on: 880624
Whiteboard: MMS_TEF,[TD-42357], TaipeiWW → MMS_TEF,[TD-42357]
This bug must land before the end of June to be localized. Can we land this now that bug 885652 and 880624 are fixed?
Flags: needinfo?(francisco.jordano)
Alex, as it stands, Bug 880624 shouldn't have landed and there is enough time left in the month to back up and get it done right instead of rushing ahead.
Comment on attachment 765888 [details]
Point to the GitHub pull request: https://github.com/mozilla-b2g/gaia/pull/10536

Thanks Evan! Great job with this complicated issue, and now it's working perfectly! Nice to meet you in Taipei, see u soon in next work week!! ;)
Attachment #765888 - Flags: review?(francisco.jordano)
Attachment #765888 - Flags: review?(fbsc)
Attachment #765888 - Flags: review-
Attachment #765888 - Flags: review+
Hi Borja,

Thanks. :)
Thanks Evan and Borja for all the supports!
Uplifted fa3a72ca5ee6638a99357abf5f5ced3acc3f6cdd to:
v1-train: 139c8484e38b48ad4d499565044b39e66b965fd7
This issue tested on Leo 1.1 (Test Run Firefox OS 1.1.0 Full – Leo Run 4)
Gecko a378807ff04076c20f08b0102286b9eb2d08d60a
Gaia 1436e2778b90bd74635b0b94d1cf8ccb0d71b60c
Platform Version: 18.1

STEPS
1. With the device without SIM Card, try to send an SMS to any number

EXPECTED
There should be an error message or any  alert for sim unavailable
ACTUAL
The device doesn' send the SMS, but no alert for sim unavailable appears

Link to failed test case: 
https://moztrap.mozilla.org/manage/case/6860/
https://moztrap.mozilla.org/manage/case/5012/
Whiteboard: MMS_TEF,[TD-42357] → MMS_TEF,[TD-42357], leorun4
This is due to https://bugzilla.mozilla.org/show_bug.cgi?id=888147, Could you check if your build contains this fix? Thanks!
Borja, the build used in this Testrun4 is previous to the fixed day, So it is expected it will work ok till now.
Thanks.
Sure, ok for land on my side, actually Borja already did the review :)
Flags: needinfo?(francisco.jordano)
Tested on Unagi device.
Build user.manifest.V1-train.Rel0.4.Sprint12.B-290.Gecko-e78450a.Gaia-7c40bda (2013-07-08)
Commercial RIL 152

STEPS
1. With the device without SIM Card, try to send an SMS to any number

EXPECTED
There should be an error message or any  alert for sim unavailable
ACTUAL
The device doesn't send the SMS, but no alert for sim unavailable appears
(See attachment)
Beatriz, we have seen that the uplift was not done properly. Im gonna make it right now and you could test it tomorrow morning in the next build. Thanks!
Now it's uplifted.

Uplifted 897a54ce1896014fab232b20db0610b49d053356 to:
v1-train: e251ee6bdab13d8620afa8f9c2d5f14e5e6a4f99

Please take care when uplifting, because in this case this functionality was broken due to the uplifting process in v1-train. Beatriz thanks a lot for identifying this error. Im gonna add a needinfo for checking that this is working as expected in tomorrow's build. Thanks!
Flags: needinfo?(bov)
Tested on Unagi device.
Build user.manifest.V1-train.Rel0.4.Sprint12.B-292.Gecko-e78450a.Gaia-e251ee6(2013-07-09)
Commercial RIL 152

STEPS
1. With the device without SIM Card, try to send an SMS to any number

EXPECTED
There should be an error message or any  alert for sim unavailable
ACTUAL
It work well.
The device doesn't send the SMS and an alert screen is shown with the message "Service currently unavailable. Message will automatically be sent once service is available"
Status: RESOLVED → VERIFIED
Flags: needinfo?(bov)
In case sending MMS, the message is much more informative.

Tested on Unagi device.
Build user.manifest.V1-train.Rel0.4.Sprint12.B-292.Gecko-e78450a.Gaia-e251ee6(2013-07-09)
Commercial RIL 152

STEPS
1. With the device without SIM Card, try to send an MMS to any number

EXPECTED
There should be an error message or any  alert for sim unavailable
ACTUAL
It work well.
The device doesn't send the MMS and an alert screen is shown with the message "Sim card error. No Sim card"

STEPS
1. With the device without SIM Card, try to send an SMS to any number

EXPECTED
There should be an error message or any  alert for sim unavailable
ACTUAL
It work well.
The device doesn't send the SMS and an alert screen is shown with the message "Service currently unavailable. Message will automatically be sent once service is available"

The alert message in SMS should be the same as in MMS
Status: VERIFIED → RESOLVED
Closed: 9 years ago9 years ago
Flags: needinfo?(fbsc)
Hi Beatriz,
This is because we are retrieving different erros for both scenarios, and probably it should be the same. We are gonna ask Gecko Team for checking this. Chia-Hung we need your help here ! :)
Flags: needinfo?(fbsc) → needinfo?(glaxy.tai)
Regarding Comment 65, just adding that reference RIL messages are fine in all scenarios, the issue appears with commercial RIL and just in case of SMS (MMS messages are the expected ones):

a)SMS + No SIM Card
Currently:
Service currently unavailable
Message will automatically be sent once service is available
Expected:
SIM card error
No SIM card

b)SMS + Airplane mode
Currently: 
Service currently unavailable
Message will automatically be sent once service is available
Expected:
Airplane mode activated
To send a message, first disable airplane mode

Asking carol for help here. Thanks.
Status: RESOLVED → REOPENED
Flags: needinfo?(cyang)
Resolution: FIXED → ---
Whiteboard: MMS_TEF,[TD-42357], leorun4 → MMS_TEF,[TD-42357], leorun4, [POVB]
Is there a specific reason for using "Sim" instead of "SIM"? As far as I can tell this string is the only one using that form.
I attach the current alert screen when trying to send a MMS witouh SIM card
There's a change we're making that will be coming in soon which should fix this issue in commercial RIL.
Flags: needinfo?(cyang)
Hi Carol,

So I think this issue is not related with Gaia layer.

Did we already file a new bug for the issue?
Then we could set block flag with that bug, or we could just close this bug?

Thanks. :)
(In reply to Evan Tseng [:evanxd] from comment #72)
> Hi Carol,
> 
> So I think this issue is not related with Gaia layer.
> 
> Did we already file a new bug for the issue?
> Then we could set block flag with that bug, or we could just close this bug?
> 
> Thanks. :)

Anshul is making the fix. I'll have him comment whether another bug already exists?
Flags: needinfo?(anshulj)
This issue in commercial RIL is resolved in AU_LINUX_GECKO_ICS_STRAWBERRY.01.01.00.019.160
Flags: needinfo?(anshulj)
Whiteboard: MMS_TEF,[TD-42357], leorun4, [POVB] → MMS_TEF,[TD-42357], leorun4, [POVB], [u=commsapps-user c=messaging p=0]
Hi Anshul,

Could you leave the bug number here?
Thanks. :)
Flags: needinfo?(anshulj)
Hi all,

Because of https://bugzilla.mozilla.org/show_bug.cgi?id=880257#c74,
I think we could close this bug.

Thanks. :)
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
(In reply to Anshul from comment #74)
> This issue in commercial RIL is resolved in
> AU_LINUX_GECKO_ICS_STRAWBERRY.01.01.00.019.160

I've tested with the Unagi device. 
Build: Gecko-a1568a0.Gaia-fb9362d
Build ID: 20130716190405

Commercial RIL: Prebuilt_CDR008_AU_LINUX_GECKO_ICS_STRAWBERRY.01.01.00.019.158.tar.gz
11-Jul-2013 04:13  4.8M
We take the latest Commercial RIL from the server:
https://grease.qualcomm.com/binaries/outgoing/CDR008/ics_strawberry/

The result with this build is the same as in https://bugzilla.mozilla.org/show_bug.cgi?id=880257#c67
as the build doesn't include the Commercial RIl that resolved the bug as Anshul mentioned.

We don't know if this Commercial RIL (AU160) is not already available in the server or we are using a wrong server.
Could someone help us?
Checked with Unagi device v1-train 07/17 build:
Gecko-2d17cfb
Gaia-773a197

It is still reproducible as the comm ril is not the .160 yet but the .158. Will verify the bug when that ril is available with the build.
Thanks

(Commenting as this bug is in the leorun4 resol-fixed list as blocked due to testing needed)
Hi Isabel, looks like AU164 just became available overnight. Can you please test with that AU?
Flags: needinfo?(isabelrios)
Hi Carol, 
Thanks, yes it is available in the v1-train 07/17 build.
The problem described in comment 67 seems to be solved.

Verified with unagi device v1-train 07/17 build: 
Gecko-2d17cfb
Gaia-c506c50
Comm. ril AU 164
Status: RESOLVED → VERIFIED
Flags: needinfo?(isabelrios)
Whiteboard: MMS_TEF,[TD-42357], leorun4, [POVB], [u=commsapps-user c=messaging p=0] → MMS_TEF,[TD-42357], leorun4, [POVB], [u=commsapps-user c=messaging p=0],[LeoVB+]
v1.1.0hd: e251ee6bdab13d8620afa8f9c2d5f14e5e6a4f99
Flags: needinfo?(anshulj)
Flags: needinfo?(glaxy.tai)
You need to log in before you can comment on or make changes to this bug.