Description: The user is prompted "Service currently unavailable" when sending a text to an invalid number. Repro Steps: 1) Updated to Buri Build ID: 20130916040205 2) Open the "Messaging" app. 3) Tap the "Compose New Message" icon. 4) In the "To" text field, enter in an invalid number. Such as (465665676567). 5) Type text into the "Message" text field and tap the "Send" button. Actual: The user is prompted with the error message "Service currently unavailable Message will automatically be sent once service is available." when sending a sms to an invalid number. Expected: The user is prompted to "Please check the number and try again." for example. Environmental Variables Gecko: http://hg.mozilla.org/mozilla-central/rev/c4bcef90cef9 Gaia: a0079597d510ce8ea0b9cbb02c506030510b9eeb Platform Version: 26.0a1 Notes: Repro frequency: (100%) Test Suite Name: (SMS) See attached: (screenshot)

Found a window, but we're seeing some problems from past builds that didn't exist before. Currently 6/24 Buri 1.2 was when it was working, then SMS was broken through to 7/3. 7/4 build is able to "send" a text and the issue repros. - 6/24 Buri 1.2 Working - Build ID: 20130624031223 Gecko: http://hg.mozilla.org/mozilla-central/rev/76820c6dff7b Gaia: 59c7f4a59157a0bc1c45d705294395f988c509b2 Platform Version: 24.0a1 - 7/4 Buri 1.2 Broken - Build ID: 20130704030830 Gecko: http://hg.mozilla.org/mozilla-central/rev/85d75ed04851 Gaia: 3f51aa0913ad86b50abd97d2b25e07b59677708c Platform Version: 25.0a1

This is not a regression, this is basically like this since the start. I proposed something in bug 823392 comment 13, and was accepted by Ayman in bug 823392 comment 14, but was somewhat rejected later in the discussion. There was a general agreement to not validate the number before sending though. My plan was: * send the recipient * if there is a general error (service currently unavailable), then we do a sanity check on the recipients and display a non-generic error. If we're ok with this plan, at least as a temporary solution that is better than a statu quo, then we can move forward. Ayman, what do you think ?

(In reply to Julien Wajsberg [:julienw] from comment #3) > This is not a regression, this is basically like this since the start. > > I proposed something in bug 823392 comment 13, and was accepted by Ayman in > bug 823392 comment 14, but was somewhat rejected later in the discussion. > There was a general agreement to not validate the number before sending > though. > > My plan was: > * send the recipient > * if there is a general error (service currently unavailable), then we do a > sanity check on the recipients and display a non-generic error. > > If we're ok with this plan, at least as a temporary solution that is better > than a statu quo, then we can move forward. Ayman, what do you think ? I think that is fine for a temporary solution and certainly improves the users understanding about what the actual problem is.

BTW it occurs to me that "Message will automatically be sent once service is available." is displayed in situations that it's not true.

Not a blocker for 1.2 after all, this is not a regression, and we don't want to add new strings at this point. Let's make it a blocker for 1.3 though, as the message is definitely not right.

(In reply to Julien Wajsberg [:julienw] from comment #6) > Not a blocker for 1.2 after all, this is not a regression, and we don't want > to add new strings at this point. > > Let's make it a blocker for 1.3 though, as the message is definitely not > right. No. comment 2 shows this working on a 6/24 build, which implies that this was working previously.

comment 2 is not clear to me. please test in 1.1.

Maybe in 1.1 we didn't show any error message at all, but rather we were only showing the error icon. Then is it really a regression ? Moreover, do we want new strings in 1.2 at this point ?

(In reply to Julien Wajsberg [:julienw] from comment #9) > Maybe in 1.1 we didn't show any error message at all, but rather we were > only showing the error icon. > > Then is it really a regression ? > > Moreover, do we want new strings in 1.2 at this point ? I need actual proof that this isn't a regression, otherwise, I tend to think the arguments presented here are not right.

Issue occurs on Buri and Leo 1.1. The "service unavailable" message still appears after sending an SMS to an invalid number. Not a regression it seems. - Buri 1.1 repros - Build ID: 20130925041202 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/18175c7fdaa6 Gaia: 6d9eec501a209bea945c6f841400ec0a75fac11d Platform Version: 18.1 - Leo 1.1 repros - Build ID: 20130925041202 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/18175c7fdaa6 Gaia: 6d9eec501a209bea945c6f841400ec0a75fac11d Platform Version: 18.1

Okay. That gives us concrete evidence this isn't a regression. Clearing flags.

hi julien, Please assign this issue to me and provide info to proced furthur.

My observations: Errors like 'NoSignalError', 'NotFoundError', 'UnknownError', 'InternalError','InvalidAddressError' were grouped to show common error message. My Idea is to have different error messages for each type. With log information an "UnknowError" is shown even for "InvalidAddressError" type.

Hi, first you need to make sure that "InvalidAddressError" is actually sent in that case. If that's what's happen, then I think this is good. Otherwise, please see my comment 3. Thanks !

hi, Errors like 'NoSignalError', 'NotFoundError', 'UnknownError', 'InternalError','InvalidAddressError' were grouped to show common error message. Now the general error message "Service currently unavailable" is used for error type 'NoSignalError' and new error messages are defined for each error type. When an address is like "&%&", gives an 'InvalidAddressError' and when an address is of random number or unknown name say (112345676543 or gfds) gives 'UnknownError' which are grouped to shown the error message "Message not sent successfully" as shown in attachemnet. When a file is removed while a mms is still in sending mode, gives 'NotFoundError', for this a seperate error message is used. Please review the strings used for this error messages.

Gene, here is the list of errors that we know in Gaia: NoSimCardError RadioDisabledError FdnCheckError NoSignalError NotFoundError InternalError UnknownError InvalidAddressError Could you please explicit the causes of all these errors ? This will be useful to display a correct error message to the user. Also, the current generic message is "Message will automatically be sent once service is available." which I think is wrong: we won't send automatically the message when the service is available. Can you tell us if this message is wrong as I think, or if _I_ am wrong here ? :)

Comment on attachment 821637 [details] pull request 13064 please request review again once you've updated your pull request thanks !

(In reply to Julien Wajsberg [:julienw] from comment #21) > Gene, here is the list of errors that we know in Gaia: > > NoSimCardError > RadioDisabledError > FdnCheckError > NoSignalError > NotFoundError > InternalError > UnknownError > InvalidAddressError > > Could you please explicit the causes of all these errors ? This will be > useful to display a correct error message to the user. Please see https://bugzilla.mozilla.org/show_bug.cgi?id=891855#c1. > > Also, the current generic message is "Message will automatically be sent > once service is available." which I think is wrong: we won't send > automatically the message when the service is available. Can you tell us if > this message is wrong as I think, or if _I_ am wrong here ? :) Yes! That message looks very weird. We won't do the resend unless users send them manually.

Thanks. FYI this message is here since 1.0 ;-) Thanks also for the pointer to bug 891855, I completely forgot about this, and it seems we didn't filed a bug to make it correct as bug 891855 didn't introduce the new string. Kodanda, please see then the following informations for the correct strings: case "InvalidAddressError": messageTitle = 'Invalid number'; messageBody = 'Make sure the mobile number you're sending to is valid.'; buttonLabel = 'OK'; break; case "NoSimCardError": messageTitle = 'Missing SIM card'; messageBody = 'Insert a valid SIM card to continue.'; buttonLabel = 'OK'; break; case "RadioDisabledError": messageTitle = 'Airplane mode activated'; messageBody = 'Turn off airplane mode to send messages.'; buttonLabel = 'OK'; break; case "NotFoundError": messageTitle = 'Message not found'; messageBody = 'The message you're looking for is no longer available.'; buttonLabel = 'OK'; break; case "NoSignalError": messageTitle = 'No network coverage'; messageBody = 'Make sure you have a mobile signal and try again.'; buttonLabel = 'OK'; break; case "UnknownError": case "InternalError": /* falls through */ default: messageTitle = 'Message not sent'; messageBody = 'There was a problem sending the message. Please try again.'; buttonLabel = 'OK'; + keep the FdnCheckError strings as it was before. Thanks to all !

hi julien, Please find the pull request 13064, modified as per the comment 24. Regards, Kodanda.

This pull request includes the changes in the test cases.

Comment on attachment 823878 [details] pull request 13064 Looks nice to me. However, I'm not neither UX nor UI reviewers. Please get Julien's review as well to land. Thanks! Btw, Bug 932201 is under way landing, which provides another new "NonActiveSimCardError". Do you want to solve that as well or fire another bug? That string stands for something like: "Please switch the default SIM to download MMS" Since you're handling sending failure, not retrieving failure. I'd suggest you can directly put that in the default case.

Hi Gene Lian, Please fire another bug, so that it would be easy to track. If possible please assign the new bug to me. Regards Ramu.

Gene, I'd like to know exactly when "InvalidAddressError" is sent. Is it when the network rejects the phone number ? Can we use this to mark a recipient as invalid ?

Comment on attachment 823878 [details] pull request 13064 Hi and thanks for your patch! You didn't follow all comments I left in my first review, therefore I've put it again as a comment. Please follow it, change the l10n key, and request review again. Thanks again!

Hey Kodandi, Steve needs this for bug 928330 and therefore he'll include your in-progress work in his patch there, so I'm duping this bug there. Thanks again for your work!