Closed Bug 888903 Opened 8 years ago Closed 8 years ago

[MMS] Sending a new MMS returns the user to 'new message' screen

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:leo+)

RESOLVED WORKSFORME
blocking-b2g leo+

People

(Reporter: maat, Assigned: borjasalguero)

References

Details

(Whiteboard: MMS_TEF, [u=commsapps-user c=messaging p=2])

Attachments

(2 files)

Sending a new MMS returns the user to 'new message' screen

**PATH**
1) open new message
2) add valid number from contact list
3) add invalid number
4) add a wall paper (to make it an MMS)
5) add some text to the message field
6) press send

**EXPECTED**
MMS is sent to valid recipients

**ACTUAL**
all content from new message dialogue is removed and new message dialogue is presented as empty

**BUILD**
build-20130701101518
Gecko-f9597b2
Gaia-c7472ac
blocking-b2g: --- → leo?
Whiteboard: MMS_TEF
Sounds like a Gaia issue. The Gecko doesn't check invalid numbers. That is, the MMS with invalid numbers can still be sent to the MMSC and can be delivered to those valid numbers.
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Assignee: nobody → mshiao
just as a further to this bug... all the MMS's i have attempted to send in Comment 0 were not displayed in the message inbox until i restarted the phone... and now i can see them:

so as an extension to Comment 0:

**PATH**
1) open new message
2) add valid number from contact list
3) add invalid number
4) add a wall paper (to make it an MMS)
5) add some text to the message field
6) press send

**EXPECTED**
MMS is sent to valid recipients

**ACTUAL**
all content from new message dialogue is removed and new message dialogue is presented as empty

7) press back button and view message app in box

**ACTUAL**
message app inbox does not contain the message constructed between step 1) and step 6) above

8) turn the phone off and then on again
9) open the message app 

**ACTUAL**
message app inbox now contains the message constructed between step 1) and step 6) above


**BUILD**
build-20130701101518
Gecko-f9597b2
Gaia-c7472ac
Attaching my phones log to the bug. 
NeedInfo me if i can be of any more assistance.
Hi Ayman,
I think that this could be related with https://bugzilla.mozilla.org/show_bug.cgi?id=888147, because in all your cases you have an 'invalid' phone number. In this case we will go directly to 'error' state without having 'sending', so that's why I could reproduce this issue during Taipei's WW. I would like to try with tomorrow's build due to the patch was uplifted some hours ago to B2G18. Assigning to me for checking this.
Assignee: mshiao → fbsc
Depends on: 888147
(In reply to Borja Salguero [:borjasalguero] from comment #4)
> Hi Ayman,
> I think that this could be related with
> https://bugzilla.mozilla.org/show_bug.cgi?id=888147, because in all your
> cases you have an 'invalid' phone number. In this case we will go directly
> to 'error' state without having 'sending', so that's why I could reproduce
> this issue during Taipei's WW. I would like to try with tomorrow's build due
> to the patch was uplifted some hours ago to B2G18. Assigning to me for
> checking this.

Cool, keep your eye on it though. The thing is that the phone is sending the message... it *just* requires a reboot for it the sent message to appear as a thread in the message app inbox.
blocking-b2g: leo? → leo+
Keywords: regression
Can whoever fixes this please write an integration test for this? There's no good reason to cause mid-cycle regressions like this.
Whiteboard: MMS_TEF → MMS_TEF, [u=commsapps-user c=messaging p=0]
Whiteboard: MMS_TEF, [u=commsapps-user c=messaging p=0] → MMS_TEF, [u=commsapps-user c=messaging p=2]
Attached file WIP Patch
Julien, I've seen that adding a setTimeout avoid the issue, but I think is related with Gecko in this case, due to we are doing 'getThreads' and 'getMessages' given a Thread at the same time, and Im retrieving a 'Internal error' through 'onerror' in getThreads... Could you take a look as well? The patch is avoiding the issue, but I think that there is more behind this. Thanks!
Flags: needinfo?(felash)
Gene, I would be happy if you can take a look as well. This 'internal error' in the 'this.error.name' in line https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/message_manager.js#L298 means that something in Gecko is happening with some transactions at the same time.
Flags: needinfo?(gene.lian)
I cannot reproduce this bug with the latest b2g18/v1-train builds (10 minutes ago).

I tried to send MMS to my iPhone phone number ("0960******") plus an invalid phone number (like "123"). That is, 2 receivers: one is valid and the other is invalid.

The MMS can be successfully sent without any errors or UI defects and my iPhone can receive that MMS for sure.

Hi Ayman and Borja, could you please update your codes and give it a try again? Thanks!
Flags: needinfo?(gene.lian)
Flags: needinfo?(fbsc)
Flags: needinfo?(aymanmaat)
Hi Gene,
As an invalid phone number we should consider the following STR:
- Go to 'new'
- Add recipients '123123123' (valid phone number) and an invalid one '%&%&**'
- Add an image for creating the MMS
- Tap on 'send'

CURRENTLY:
- We are getting a weird error and we retrieve 'internal error' from Gecko when requesting 'getThreads' :S
Flags: needinfo?(fbsc)
I tried the following 3 combinations (note that "12345678" is a valid number):

1. "12345678" + "999" - MMS can be sent and the "12345678" can receive that.
2. "12345678" + "aaa" - MMS can be sent and the "12345678" can receive that.
3. "12345678" + "%&%" - MMS cannot be sent and a prompt shows "Service currently unavailable" After tap on the "OK", it goes back to the normal message list view. I didn't see the 

None of the above cases can reproduce comment #2. Did I misunderstand any thing? Anyway, the return of case #3 is not reasonable.
(In reply to Gene Lian [:gene] from comment #11)

> 3. "12345678" + "%&%" - MMS cannot be sent and a prompt shows "Service
> currently unavailable" After tap on the "OK", it goes back to the normal
> message list view.  
>
> Anyway, the return of case #3 is not reasonable.

I filed bug 891756 this morning about this, you can go and comment there if you feel like it ;)
Flags: needinfo?(felash)
Isa, could you take a look? Thanks!
Flags: needinfo?(isabelrios)
Hi Borja, 
I can only reproduce what described by Gene on comment 11 with unagi 07/09 v1-train build ref ril:
Gecko-e78450a
Gaia-e251ee6
Flags: needinfo?(isabelrios)
Should we close this as WORKSFORME? Probably was fixed with some Gecko patch and that's why with Today's build is working. Wdyt?
(In reply to Julien Wajsberg [:julienw] from comment #12)
> I filed bug 891756 this morning about this, you can go and comment there if
> you feel like it ;)

I provide a patch at Bug 891756 to solve all the issues regarding the invalid phone numbers to make sure the SMS and MMS work in a consistent way.

Maybe, we can leave this open to properly handle the "InvalidAddressError" on the Gaia end.
Flags: needinfo?(aymanmaat)
(In reply to Borja Salguero [:borjasalguero] from comment #10)
> Hi Gene,
> As an invalid phone number we should consider the following STR:
> - Go to 'new'
> - Add recipients '123123123' (valid phone number) and an invalid one '%&%&**'
> - Add an image for creating the MMS
> - Tap on 'send'
> 
> CURRENTLY:
> - We are getting a weird error and we retrieve 'internal error' from Gecko
> when requesting 'getThreads' :S

I can still see the .getThreads(...) returns internal error. However, at least the bug doesn't show up anymore so maybe we can open separate bug to deal with this *potential* issue.

Hi Vicamo, do you know what's happening to the .getThreads(...)? No one is an expert than you to deal with the thread stuff.
Flags: needinfo?(vyang)
(In reply to Gene Lian [:gene] from comment #17)
> I can still see the .getThreads(...) returns internal error. However, at
> least the bug doesn't show up anymore so maybe we can open separate bug to
> deal with this *potential* issue.

Fire Bug 891860 and Bug 891855 and let's close this one with WORKSFORME.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
(In reply to Gene Lian [:gene] from comment #17)
> (In reply to Borja Salguero [:borjasalguero] from comment #10)
> > Hi Gene,
> > As an invalid phone number we should consider the following STR:
> > - Go to 'new'
> > - Add recipients '123123123' (valid phone number) and an invalid one '%&%&**'
> > - Add an image for creating the MMS
> > - Tap on 'send'
> > 
> > CURRENTLY:
> > - We are getting a weird error and we retrieve 'internal error' from Gecko
> > when requesting 'getThreads' :S
> 
> I can still see the .getThreads(...) returns internal error. However, at
> least the bug doesn't show up anymore so maybe we can open separate bug to
> deal with this *potential* issue.
> 
> Hi Vicamo, do you know what's happening to the .getThreads(...)? No one is
> an expert than you to deal with the thread stuff.

Don't quite know either.  '%&%&**' will be replaced with '**' in |PhoneNumberUtils.normalize()| on creating new participant record, but that doesn't account for the error.  Let's move to bug 891860 for further discussion.
Flags: needinfo?(vyang)
You need to log in before you can comment on or make changes to this bug.