983315 - [Messages][DSDS] Provide a feedback while we're switching the data APN to send a MMS

Status: RESOLVED FIXED

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

Description

[Julien Wajsberg [:julienw]]

Follow-up to Bug 947139

When the SMS and MMS serviceIds are different, or when the required serviceId is different than the current MMS serviceId, we need to switch the data APN, and wait that it's registered before sending the MMS.

This has several implications:
1. we don't have the "sending" event until we call the gecko API. So that means that we don't switch to the thread (when we're in the composer) or we don't add the new message (when we're in the thread) until the data APN switching is finished.
2. if the Messages app is killed, we're loosing the MMS, because it's not persisted in the Gecko DB

So therefore, I have these comments:
1. it would be vastly awesome if Gecko would do this itself if we specify a serviceId as parameter. That way, we could have the "sending" event right away, and the MMS would be persisted.
2. it seems that we don't get an error if we're trying to send a message while the data apn switch is ongoing. So maybe we can send a MMS as soon as we changed the pref, and it would just work.
3. if all this is not possible, then we need to find something.
 3a. Ayman suggested having a modal dialog to address the immediate feedback. Given that we don't have an error when sending before the connection is established, I think we should just have a "dismiss" button to remove the modal dialog.
 3b. We need to keep the MMS in the composer, so that if the user presses "home" it would be saved in a draft, and if the Messages app is killed, the draft would still be saved. Not ideal but at least we don't lose data. In that case, the composer would need to be disabled until the user presses back or the app is killed. Pressing back would also save a draft. Not ideal at all :(

Needinfo Bevis to know whether 1 or 2 is possible. Maybe 2 is already possible now and we should just do it. I recall it was not working for retrieveMMS, but maybe we need to wait 1 second or something?

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #0)
> So therefore, I have these comments:
> 1. it would be vastly awesome if Gecko would do this itself if we specify a
> serviceId as parameter. That way, we could have the "sending" event right
> away, and the MMS would be persisted.
Sorry to say that, for |MMS transaction in DSDS|, as we discussed offline in the email,
it is not possible to internally switch the active serviceId according the the |serviceId| 
parameter in the sendMMS API. This unexpectedly breaks any ongoing data transimission of 
the active SIM which might be important to the user.

> 2. it seems that we don't get an error if we're trying to send a message
> while the data apn switch is ongoing. So maybe we can send a MMS as soon as
> we changed the pref, and it would just work.
No, same as retrieving, 
we should also wait for the datachange event before sending it.

Comment 2

[Joe Cheng [:jcheng] (please needinfo)]

switch component to RIL as it seems to be a gecko solution

Comment 3

[Preeti Raghunath(:Preeti)]

Bevis,

What would be the user impact here?

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

It's not a user impact. It's a limitation for DSDS device instead.
Just like retrieving a MMS message from non-active SIM,
We need user's confirmation to switch the active SIM before 
sending MMS to prevent any unexpected drop of the ongoing data transmission.

Not a gecko solution but a clarification of 2 comments in Julien's description.

Comment 5

[Preeti Raghunath(:Preeti)]

This sounds like an enhancement request. ni Wilfred for the need of the enhancement.

Comment 6

[Joe Cheng [:jcheng] (please needinfo)]

triage: adding this to backlog

Comment 7

[Julien Wajsberg [:julienw]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #1)
> (In reply to Julien Wajsberg [:julienw] (away until March 24) from comment
> #0)
> > So therefore, I have these comments:
> > 1. it would be vastly awesome if Gecko would do this itself if we specify a
> > serviceId as parameter. That way, we could have the "sending" event right
> > away, and the MMS would be persisted.
> Sorry to say that, for |MMS transaction in DSDS|, as we discussed offline in
> the email,
> it is not possible to internally switch the active serviceId according the
> the |serviceId| 
> parameter in the sendMMS API. This unexpectedly breaks any ongoing data
> transimission of 
> the active SIM which might be important to the user.

I said this in a mail somewhere, maybe not in a bug though.

My ideal API would have as optional parameters:

{
 serviceId: <serviceId>,
 forceSwitch: true/false
}

If forceSwitch is absent or false, the API would yield an "error" event. With this error, in Gaia we would display the confirmation dialog.
If forceSwitch is present and true, the API would do the switch itself. That means:
* save the MMS in the DB
* send the "sending" event
* change the preference, wait for datachange
* actually send the MMS

> 
> > 2. it seems that we don't get an error if we're trying to send a message
> > while the data apn switch is ongoing. So maybe we can send a MMS as soon as
> > we changed the pref, and it would just work.
> No, same as retrieving, 
> we should also wait for the datachange event before sending it.

How comes we don't get an error if we try to send a message while the switch is ongoing? I actually tried to send a message and it was actually sent when the switch was finished.

Why do we need to wait if the API has already a mechanism to handle this properly?



User impact is:
* There is no user feedback while the MMS SIM switch is happening. This can last from 20 seconds to 5 minutes (possibly more when the user has a bad network).
* Worse: if the app is killed before that the MMS SIM switch is done, the MMS is lost. This is a dataloss.

That's why I'm requesting blocking 1.4 again, because this behavior is a blocker for me. To me the ideal solution is either the solution 1 or solution 2. Solution 1 looks a lot nicer to me though.

NI Bevis for giving feedback.

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #7)
> (In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #1)
> > (In reply to Julien Wajsberg [:julienw] (away until March 24) from comment
> > #0)
> > > So therefore, I have these comments:
> > > 1. it would be vastly awesome if Gecko would do this itself if we specify a
> > > serviceId as parameter. That way, we could have the "sending" event right
> > > away, and the MMS would be persisted.
> > Sorry to say that, for |MMS transaction in DSDS|, as we discussed offline in
> > the email,
> > it is not possible to internally switch the active serviceId according the
> > the |serviceId| 
> > parameter in the sendMMS API. This unexpectedly breaks any ongoing data
> > transimission of 
> > the active SIM which might be important to the user.
> 
> I said this in a mail somewhere, maybe not in a bug though.
> 
> My ideal API would have as optional parameters:
> 
> {
>  serviceId: <serviceId>,
>  forceSwitch: true/false
> }
> 
> If forceSwitch is absent or false, the API would yield an "error" event.
> With this error, in Gaia we would display the confirmation dialog.
> If forceSwitch is present and true, the API would do the switch itself. That
> means:
> * save the MMS in the DB
> * send the "sending" event
> * change the preference, wait for datachange
> * actually send the MMS
> 
For DSDS, current principle of switching the data connection are all controlled by
changing the preference and we MUST get user's confirmation before switching.
It has to be done before either sending/retrieving MMS in DSDS.
Hence, it looks duplicated for me to have a "forceSwitch" parameter here.
Besides, as we discussed earlier, the |serviceId| is defined for more general purpose of the multiple SIM solution like DSDA.

> > 
> > > 2. it seems that we don't get an error if we're trying to send a message
> > > while the data apn switch is ongoing. So maybe we can send a MMS as soon as
> > > we changed the pref, and it would just work.
> > No, same as retrieving, 
> > we should also wait for the datachange event before sending it.
> 
> How comes we don't get an error if we try to send a message while the switch
> is ongoing? I actually tried to send a message and it was actually sent when
> the switch was finished.

To have better feedback of the data switching, 
I'd prefer to enhance what we have in switching the default service ID in DSDS and
provide a feedback for it if the switching is failed.

NI Hsinyi to see if we need better feedback of SIM switching for this.
Or maybe we need to review this further for better design of switching active SIM for DSDS.


> Why do we need to wait if the API has already a mechanism to handle this
> properly?
> 
> 
> 
> User impact is:
> * There is no user feedback while the MMS SIM switch is happening. This can
> last from 20 seconds to 5 minutes (possibly more when the user has a bad
> network).
> * Worse: if the app is killed before that the MMS SIM switch is done, the
> MMS is lost. This is a dataloss.

May I know this in more detail about how the application could be killed?
Is it a normal case?

NI Julien for further feedback.

> NI Bevis for giving feedback.

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #8)
> > * Worse: if the app is killed before that the MMS SIM switch is done, the
> > MMS is lost. This is a dataloss.
> 
> May I know this in more detail about how the application could be killed?
> Is it a normal case?
> 
> NI Julien for further feedback.

In addition, this could also be covered if 
we save it as a draft first while switching the data connection.

Comment 10

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #8)
> (In reply to Julien Wajsberg [:julienw] (away until March 24) from comment
> #7)
> > (In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #1)
> > > (In reply to Julien Wajsberg [:julienw] (away until March 24) from comment
> > > #0)
> > > > So therefore, I have these comments:
> > > > 1. it would be vastly awesome if Gecko would do this itself if we specify a
> > > > serviceId as parameter. That way, we could have the "sending" event right
> > > > away, and the MMS would be persisted.
> > > Sorry to say that, for |MMS transaction in DSDS|, as we discussed offline in
> > > the email,
> > > it is not possible to internally switch the active serviceId according the
> > > the |serviceId| 
> > > parameter in the sendMMS API. This unexpectedly breaks any ongoing data
> > > transimission of 
> > > the active SIM which might be important to the user.
> > 
> > I said this in a mail somewhere, maybe not in a bug though.
> > 
> > My ideal API would have as optional parameters:
> > 
> > {
> >  serviceId: <serviceId>,
> >  forceSwitch: true/false
> > }
> > 
> > If forceSwitch is absent or false, the API would yield an "error" event.
> > With this error, in Gaia we would display the confirmation dialog.
> > If forceSwitch is present and true, the API would do the switch itself. That
> > means:
> > * save the MMS in the DB
> > * send the "sending" event
> > * change the preference, wait for datachange
> > * actually send the MMS
> > 

Looks that 'forceSwitch' is introduced for DSDA?  I agree that when we want to support DSDA, we will need some attribute to reveal the capability. But seems this is kinda out of the scope here?

I filed Bug 985945 for exposing capabilities of DSDS or DSDA to gaia.

> For DSDS, current principle of switching the data connection are all
> controlled by
> changing the preference and we MUST get user's confirmation before switching.
> It has to be done before either sending/retrieving MMS in DSDS.
> Hence, it looks duplicated for me to have a "forceSwitch" parameter here.

IIRC, in DSDS, we agree to have user's confirmation before switching data so gecko won't do that automatically; hence that makes not much sense to me to have "forceSwitch". 

> Besides, as we discussed earlier, the |serviceId| is defined for more
> general purpose of the multiple SIM solution like DSDA.
> 
> > > 
> > > > 2. it seems that we don't get an error if we're trying to send a message
> > > > while the data apn switch is ongoing. So maybe we can send a MMS as soon as
> > > > we changed the pref, and it would just work.
> > > No, same as retrieving, 
> > > we should also wait for the datachange event before sending it.
> > 
> > How comes we don't get an error if we try to send a message while the switch
> > is ongoing? I actually tried to send a message and it was actually sent when
> > the switch was finished.
> 

I think our API should handle the error cases more carefully. For example, 1) if gaia tries to send MMS using a wrong serviceId or 2) if gaia tries to send MMS when data registration state isn't ready. Does it help gaia?

> To have better feedback of the data switching, 
> I'd prefer to enhance what we have in switching the default service ID in
> DSDS and
> provide a feedback for it if the switching is failed.
> 
> NI Hsinyi to see if we need better feedback of SIM switching for this.

We are using Settings API for setting and getting default service id. [1] fires error when the switching fails. But the Settings API is lack of a capability to tell if everyone interested in a key has received the settings change. That said, if message APP changes defaultServiceId, how does it know that MMSService receives the change? Agree with Bevis that having this kinda callback seems benefit gaia. How do you think, Julien?

[1] http://dxr.mozilla.org/mozilla-central/source/dom/webidl/SettingsManager.webidl#14

> Or maybe we need to review this further for better design of switching
> active SIM for DSDS.
> 
> 
> > Why do we need to wait if the API has already a mechanism to handle this
> > properly?
> > 
> > 
> > 
> > User impact is:
> > * There is no user feedback while the MMS SIM switch is happening. This can
> > last from 20 seconds to 5 minutes (possibly more when the user has a bad
> > network).
> > * Worse: if the app is killed before that the MMS SIM switch is done, the
> > MMS is lost. This is a dataloss.
> 
> May I know this in more detail about how the application could be killed?
> Is it a normal case?
> 
> NI Julien for further feedback.
> 
> > NI Bevis for giving feedback.

Comment 11

[Julien Wajsberg [:julienw]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #8)
> (In reply to Julien Wajsberg [:julienw] (away until March 24) from comment
> #7)
> > (In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #1)
> > > (In reply to Julien Wajsberg [:julienw] (away until March 24) from comment
> > > #0)
> > > > So therefore, I have these comments:
> > > > 1. it would be vastly awesome if Gecko would do this itself if we specify a
> > > > serviceId as parameter. That way, we could have the "sending" event right
> > > > away, and the MMS would be persisted.
> > > Sorry to say that, for |MMS transaction in DSDS|, as we discussed offline in
> > > the email,
> > > it is not possible to internally switch the active serviceId according the
> > > the |serviceId| 
> > > parameter in the sendMMS API. This unexpectedly breaks any ongoing data
> > > transimission of 
> > > the active SIM which might be important to the user.
> > 
> > I said this in a mail somewhere, maybe not in a bug though.
> > 
> > My ideal API would have as optional parameters:
> > 
> > {
> >  serviceId: <serviceId>,
> >  forceSwitch: true/false
> > }
> > 
> > If forceSwitch is absent or false, the API would yield an "error" event.
> > With this error, in Gaia we would display the confirmation dialog.
> > If forceSwitch is present and true, the API would do the switch itself. That
> > means:
> > * save the MMS in the DB
> > * send the "sending" event
> > * change the preference, wait for datachange
> > * actually send the MMS
> > 
> For DSDS, current principle of switching the data connection are all
> controlled by
> changing the preference and we MUST get user's confirmation before switching.
> It has to be done before either sending/retrieving MMS in DSDS.

You're missing my point:
* gaia calls "sendMMS" without "forceSwitch" _first_.
** if it's in DSDS and the current Id is not the requested Id, then fire an error
** otherwise, send the message
* when gaia gets the error, then it asks confirmation to the user
* then it calls "sendMMS" with "forceSwitch".

> Hence, it looks duplicated for me to have a "forceSwitch" parameter here.
> Besides, as we discussed earlier, the |serviceId| is defined for more
> general purpose of the multiple SIM solution like DSDA.

That's exactly the beauty of my proposal: in DSDA mode, we wouldn't have an error without forceSwitch. Which means that gaia would be completely independent to DSDS/DSDA mode.

This would also be a lot more consistent API between the SMS and MMS API. Currently, Gaia needs to do really different actions for SMS and MMS.
> > 
> > 
> > User impact is:
> > * There is no user feedback while the MMS SIM switch is happening. This can
> > last from 20 seconds to 5 minutes (possibly more when the user has a bad
> > network).
> > * Worse: if the app is killed before that the MMS SIM switch is done, the
> > MMS is lost. This is a dataloss.
> 
> May I know this in more detail about how the application could be killed?
> Is it a normal case?

If switching the serviceID takes 5 minutes, it's really likely that the user will do something else with his phone, and the app might be killed as a result.

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #9)
> 
> In addition, this could also be covered if 
> we save it as a draft first while switching the data connection.

Yes, this is possibility of a workaround in Gaia, but IMO this is just a workaround for the underlying API problem.

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #10)
> 
> Looks that 'forceSwitch' is introduced for DSDA?  I agree that when we want
> to support DSDA, we will need some attribute to reveal the capability. But
> seems this is kinda out of the scope here?

I didn't follow the DSDA work but I don't understand why "forceSwitch" is needed for DSDA (since we don't need to switch?). To me it's really useful for DSDS.

Also, the name "forceSwitch" can be changed if it does not suit you, obviously.

> 
> I filed Bug 985945 for exposing capabilities of DSDS or DSDA to gaia.
> 
> > For DSDS, current principle of switching the data connection are all
> > controlled by
> > changing the preference and we MUST get user's confirmation before switching.
> > It has to be done before either sending/retrieving MMS in DSDS.
> > Hence, it looks duplicated for me to have a "forceSwitch" parameter here.
> 
> IIRC, in DSDS, we agree to have user's confirmation before switching data so
> gecko won't do that automatically; hence that makes not much sense to me to
> have "forceSwitch". 

See above: Gaia would use "forceSwitch" if the user confirmed it. We wouldn't use it before.
I think this is essentially the same way of working, except we use a more consistent API.

> > > How comes we don't get an error if we try to send a message while the switch
> > > is ongoing? I actually tried to send a message and it was actually sent when
> > > the switch was finished.
> > 
> 
> I think our API should handle the error cases more carefully. For example,
> 1) if gaia tries to send MMS using a wrong serviceId or 2) if gaia tries to
> send MMS when data registration state isn't ready. Does it help gaia?

Currently, when we try to send a MMS (manually) when the data registration state is not ready... it works.

Therefore maybe we don't need to wait for the datastatechange event, but just wait the "setting set" operation's success event. If it works like this this is good enough for me.

> 
> > To have better feedback of the data switching, 
> > I'd prefer to enhance what we have in switching the default service ID in
> > DSDS and
> > provide a feedback for it if the switching is failed.
> > 
> > NI Hsinyi to see if we need better feedback of SIM switching for this.
> 
> We are using Settings API for setting and getting default service id. [1]
> fires error when the switching fails. But the Settings API is lack of a
> capability to tell if everyone interested in a key has received the settings
> change. That said, if message APP changes defaultServiceId, how does it know
> that MMSService receives the change? Agree with Bevis that having this kinda
> callback seems benefit gaia. How do you think, Julien?

As said above, this would be good enough. When is the "success" event sent?

Comment 12

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #11)
> > Looks that 'forceSwitch' is introduced for DSDA?  I agree that when we want
> > to support DSDA, we will need some attribute to reveal the capability. But
> > seems this is kinda out of the scope here?
> 
> I didn't follow the DSDA work but I don't understand why "forceSwitch" is
> needed for DSDA (since we don't need to switch?). To me it's really useful
> for DSDS.
> 
> Also, the name "forceSwitch" can be changed if it does not suit you,
> obviously.
> 

Oh... sorry that I didn't get your point at the first moment. :( Now I get it!

Just had a long discussion with Bevis and Gene, and we thought that our API should be more powerful, i.e. make 'serviceId' really work. That means, the API will enforce to use the specified 'serviceId' for MMS sending. I remember that you've offered this proposal, right? I didn't vote for it, but after reviewing all the issue, I think I'd like to go for this. Bevis should be able to provide all the details. :) Please wait for his reply, and I clean ni for now.

Thanks, Julien and Bevis!!!

Comment 13

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi Julien,

After further discussion, here is our proposal for your information.
Please let us know if this sounds good for you.
If you have other suggestion, please let us know. :)
In DSDS,
1. For Both |Sending| and |Manually Retrieval| of a MMS Message, we need Gaia's help to Show the Confirm/Error Dialog to the user according to the exposed APIs currently available.
2. For sendMMS() and retrieveMMS(), there is no need to introduce new parameter of |forceSwitch|.
   Internally, MmsService will switch the |dom.mms.defaultServiceId| if the specified |serviceId| in sendMMS() or the |serviceId| mapped from the iccId of the message in retrieveMMS() is not matched.
3, New Error like SWITCH_TIMEOUT, SWITCH_FAILED will be introduced for better feedback.
   Then, the behavior will be compliant to the API we define no matter it's DSDS or not.
4. This also implies that both |dom.mms.defaultServiceId| and |ril.data.defaultServiceId| 
   are switched internally if not matched and won't be restored after finishing the transactions.

Comment 14

[bhavana bajaj [:bajaj]]

NI, :jcheng to take an action here as it is really hard to decrypt the comments to mkae a blocking call here.

Comment 15

[Julien Wajsberg [:julienw]]

Bhavana, see the end of comment 7 for the user impact. This should definitely block.

The other comments are more about how to resolve it.

Comment 16

[Julien Wajsberg [:julienw]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #13)
> Hi Julien,
> 
> After further discussion, here is our proposal for your information.
> Please let us know if this sounds good for you.
> If you have other suggestion, please let us know. :)
> In DSDS,
> 1. For Both |Sending| and |Manually Retrieval| of a MMS Message, we need
> Gaia's help to Show the Confirm/Error Dialog to the user according to the
> exposed APIs currently available.
> 2. For sendMMS() and retrieveMMS(), there is no need to introduce new
> parameter of |forceSwitch|.

Currently, for retrieveMMS, we show the dialog when we get the error "NonActiveSimCardError". In my proposal, this error would stay and we could work like we do today. With your proposal, I guess this error would be removed and we need to check the current serviceId before calling retrieveMMS?

I liked my proposal because the DSDS/DSDA distinction would be handled completely by Gecko. In your proposal, we still need to have a quite complex condition:
* if DSDS and the target iccId is not the current one, then show the confirmation dialog
* otherwise (DSDA or single SIM or right iccId) we call directly.

I think Gecko is better suited to check this condition, and fire an error in the first case (if forceSwitch absent or false). And that's why I still think my first proposal is better ;)

>    Internally, MmsService will switch the |dom.mms.defaultServiceId| if the
> specified |serviceId| in sendMMS() or the |serviceId| mapped from the iccId
> of the message in retrieveMMS() is not matched.

> 3, New Error like SWITCH_TIMEOUT, SWITCH_FAILED will be introduced for
> better feedback.
>    Then, the behavior will be compliant to the API we define no matter it's
> DSDS or not.

> 4. This also implies that both |dom.mms.defaultServiceId| and
> |ril.data.defaultServiceId| 
>    are switched internally if not matched and won't be restored after
> finishing the transactions.

In my proposal, we'd do this only when requesting "forceSwitch", whereas in your proposal, we'd do this transparently. That's another reason I prefer my more consistent and explicit proposal.

Now, to fix this specific issue, your proposal works if the message is stored in the DB and  "sending" event is sent before switching the iccId. Still prefer mine ;)

Comment 17

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #15)
> Bhavana, see the end of comment 7 for the user impact. This should
> definitely block.
> 
> The other comments are more about how to resolve it.

Hi Julien, 

May I know what's the different in the Gaia part between 1.3 and 1.4 that cause this a blocker for 1.4?
From Gecko's perspective, it's seems to be an enhancement from 1.3 instead.
Because, this only happens if we need to switching the serviceId for sending on the fly.
Is this a new feature in 1.4 UX design?
If yes, what we can do in a short time is to leverage the SIM switch mechanism of retrieval that already done in gaia. (I know this is a very bad idea. :( )
Otherwise, we need to lock down these API (That's what we are doing now) and change the design in both gecko/gaia to support this new UX design.

Regards,
Bevis Tseng

Comment 18

[Julien Wajsberg [:julienw]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #17)
> (In reply to Julien Wajsberg [:julienw] (away until March 24) from comment
> #15)
> > Bhavana, see the end of comment 7 for the user impact. This should
> > definitely block.
> > 
> > The other comments are more about how to resolve it.
> 
> Hi Julien, 
> 
> May I know what's the different in the Gaia part between 1.3 and 1.4 that
> cause this a blocker for 1.4?

In 1.3, retrieveMMS was the only case where we switched the service Id. For retrieveMMS there is no dataloss, because the message is already stored in the DB. If the Messages app is killed, maybe the retrieveMMS operation would not happen, but this would not be too bad.

in 1.4, since we need to switch before calling sendMMS, and therefore before the message is stored in the main DB, there is currently a dataloss.

If there is no Gecko change, we'll need to store the MMS in a local database (using a draft is an option), and I'm sure this will lead to subtle bugs that will be painful to fix.

> From Gecko's perspective, it's seems to be an enhancement from 1.3 instead.
> Because, this only happens if we need to switching the serviceId for sending
> on the fly.
> Is this a new feature in 1.4 UX design?
> If yes, what we can do in a short time is to leverage the SIM switch
> mechanism of retrieval that already done in gaia. (I know this is a very bad
> idea. :( )
> Otherwise, we need to lock down these API (That's what we are doing now) and
> change the design in both gecko/gaia to support this new UX design.

I don't know what is the amount of work needed for doing this in Gecko. For sure, for us in Gaia it's easier if it's done in Gecko (and it feels cleaner) but if it's too much work we can start to think about a workaround in Gaia.

Comment 19

[Joe Cheng [:jcheng] (please needinfo)]

this bug will show up in comms team triage, will be discussed during triage

Comment 20

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi all,

I'd like to summarize what we have discussed so far here:
[3 major issues regarding sending/retrieving MMS under DSDS device]:
1. How to inform user and get user's confirmation if the selected serviceId is not active.
2. How to make the transaction to be done smoothly and provide feedback accordingly 
   when switching active SIM is needed.
3. Shall we restore the active SIM after transaction is done no matter it's success or not?
   (This one is not discussed in previous discussion yet. 
    However, after internal discussion, we should do this to prevent the caller 
    to be confused why the active SIM was changed after request is done. 
    Then, treat this a bug or a side-effect of this API.)

[Proposal from Gaia]
1. 1 additional parameter |forceSwitch| is introduced in sendMMS/retrieveMMS WebAPI.
2. when |forceSwitch| was not specified, gecko returns a specified error for gaia
   to handle the issue#1.
3. Then, gaia can call either send/retrieve again with |forceSwitch| set to |true| for
   gecko to handle the switching and then the transaction accordingly.

The advantage is that both issue#1 and issue#2 are covered in these 2 APIs.
The disadvantage are that 
1. From API design perspective, the |forceSwitch| parameter is too DSDS-specific 
   when |serviceId| is already available in these API.
2. The UX behavior of issue#1 was bounded too closely to this API which
   can actually be covered by bug Bug 985945 when ready and
   it looks strange to call this API twice for different purposes.

[Proposal from Gecko]
1. Bug 985945 has to be introduced to provide a general solution for
   apps to define their-own UX behaviors.
2. For the implementation of sendMMS/retrieveMMS API, to make sure that it matches 
   the semantic meaning of these APIs:
   a. Gecko has to accomplish the MMS transaction to the specified serviceId including
      switching the active SIM when needed during the transaction for DSDS.
   b. For sendMMS, if there is no serviceId specified, 
      default serviceId in the settings will be used.
   c. In DSDS, the switching is triggered on the fly per sending/retriving request.
      Hence, there shall be no change to both |dom.mms.defaultServiceId| and 
      |ril.data.defaultServiceId| during the transaction.
   D. The switching has to be restored after request is completed for DSDS.
      (Note: This has to be implemented, reviewed, and verified very very carefully.)

This advantage is that
1. We achieve the ultimate implementation of sendMMS/retrieveMMS API.
2. We have clear cut of the responsibility of each API.
3. 3 major issues can all be resolved in a good manner.
The only disadvantage is that
Message App has to handle issue#1 with the new API in Bug 985945.
However, this shall not be too complicated in our internal discussion.

[Schedule to support this in 1.4]
No matter which proposal mentioned above, it will be a big design change in gecko side.
So far, this is an unexpected change especially after 1.4 FC.

Comment 21

[Joe Cheng [:jcheng] (please needinfo)]

Base on comment 20, this sound like a error handling case where the change is too big for 1.4

QAWANTED on the current behavior and video recording to understand if we can live with the current behavior under regular use cases

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #7)
> User impact is:
> * There is no user feedback while the MMS SIM switch is happening. This can
> last from 20 seconds to 5 minutes (possibly more when the user has a bad
> network).
> * Worse: if the app is killed before that the MMS SIM switch is done, the
> MMS is lost. This is a dataloss.
> 
> That's why I'm requesting blocking 1.4 again, because this behavior is a
> blocker for me. To me the ideal solution is either the solution 1 or
> solution 2. Solution 1 looks a lot nicer to me though.
> 
> NI Bevis for giving feedback.

Comment 22

[Julien Wajsberg [:julienw]]

(In reply to Joe Cheng [:jcheng] from comment #21)
> Base on comment 20, this sound like a error handling case where the change
> is too big for 1.4

This is not a error handling case, this is also a UX matter as the user has no feedback after pressing "confirm". I already discussed with Ayman about what we could do in Gaia but nothing really satisfies me so far.

> 
> QAWANTED on the current behavior and video recording to understand if we can
> live with the current behavior under regular use cases

Really, it's not possible.
I tested it myself in Paris while developing. Paris is not exactly an emerging market, and yet, it was unexpectable from my point of view.


(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #20)
> Hi all,
> 
> I'd like to summarize what we have discussed so far here:
> [3 major issues regarding sending/retrieving MMS under DSDS device]:
> 1. How to inform user and get user's confirmation if the selected serviceId
> is not active.
> 2. How to make the transaction to be done smoothly and provide feedback
> accordingly 
>    when switching active SIM is needed.
> 3. Shall we restore the active SIM after transaction is done no matter it's
> success or not?
>    (This one is not discussed in previous discussion yet. 
>     However, after internal discussion, we should do this to prevent the
> caller 
>     to be confused why the active SIM was changed after request is done. 
>     Then, treat this a bug or a side-effect of this API.)

4. The sending message needs to be saved somewhere.

> 
> [Proposal from Gaia]
> 1. 1 additional parameter |forceSwitch| is introduced in sendMMS/retrieveMMS
> WebAPI.
> 2. when |forceSwitch| was not specified, gecko returns a specified error for
> gaia
>    to handle the issue#1.
> 3. Then, gaia can call either send/retrieve again with |forceSwitch| set to
> |true| for
>    gecko to handle the switching and then the transaction accordingly.
> 
> The advantage is that both issue#1 and issue#2 are covered in these 2 APIs.
> The disadvantage are that 
> 1. From API design perspective, the |forceSwitch| parameter is too
> DSDS-specific 
>    when |serviceId| is already available in these API.
> 2. The UX behavior of issue#1 was bounded too closely to this API which
>    can actually be covered by bug Bug 985945 when ready and
>    it looks strange to call this API twice for different purposes.

For me it's a "hint" that the API can do the switch. We can call it "withSwitch" or "switchAccepted" if you prefer. To me, it's really like the "will-change" CSS property that gives a hint that some part will be animated.

What I like a lot in this proposal, it's that it is DSDS/DSDA/SingleSim agnostic for Gaia, it wouldn't need any change in Gaia when we start supporting DSDA. All other proposals need a lot of conditions everywhere to detect whether we're in DSDS/DSDA/SingleSim scenario (not to mention DSDS with one SIM). Of course the other proposals would work, but would yield in my opinion much complexity. Now, maybe the complexity would be in Gecko, but I think Gecko has much more information to know the current status.

> 
> [Proposal from Gecko]
> 1. Bug 985945 has to be introduced to provide a general solution for
>    apps to define their-own UX behaviors.
> 2. For the implementation of sendMMS/retrieveMMS API, to make sure that it
> matches 
>    the semantic meaning of these APIs:
>    a. Gecko has to accomplish the MMS transaction to the specified serviceId
> including
>       switching the active SIM when needed during the transaction for DSDS.
>    b. For sendMMS, if there is no serviceId specified, 
>       default serviceId in the settings will be used.
>    c. In DSDS, the switching is triggered on the fly per sending/retriving
> request.
>       Hence, there shall be no change to both |dom.mms.defaultServiceId| and 
>       |ril.data.defaultServiceId| during the transaction.
>    D. The switching has to be restored after request is completed for DSDS.
>       (Note: This has to be implemented, reviewed, and verified very very
> carefully.)
> 
> This advantage is that
> 1. We achieve the ultimate implementation of sendMMS/retrieveMMS API.
> 2. We have clear cut of the responsibility of each API.
> 3. 3 major issues can all be resolved in a good manner.
> The only disadvantage is that
> Message App has to handle issue#1 with the new API in Bug 985945.
> However, this shall not be too complicated in our internal discussion.
> 
> [Schedule to support this in 1.4]
> No matter which proposal mentioned above, it will be a big design change in
> gecko side.
> So far, this is an unexpected change especially after 1.4 FC.

This change would also yield many changes in Gaia so it's clearly not what we should do in 1.4 in my opinion.


I'm not sure about restoring the switching, especially because it can take a lot of time. Also, we'll probably need to change the UX and the texts...

I had also another proposal that you didn't comment much, for DSDS (so it doesn't solve the complexity issue I outlined above):

1. User wants to send a MMS using SIM2 whereas SIM1 is the current default one
2. Gaia asks confirmation, user confirms
3. Gaia changes the serviceId setting (like it does now)
4. Gaia sends the MMS when the "setting set" operation's success event is sent
5. Gecko stores the message in the DB, send the "sending" event
5. Gecko checks that the serviceId setting match the requested serviceId
  5a. Fire an error if it's not right
6. Gecko handles the datachange event
  6a. Fire an error if the wrong serviceId is registered (for example the user could have changed the setting because it was taking too long)
7. Gecko sends the MMS once the requested serviceId is "registered"


In my local tests, I think that much of it already just works. I didn't try 4. yet but I did try to send a MMS while the switch was happening (so about 30 seconds after the switch was requested), and this MMS was sent correctly.

This has the advantage of being a lot simpler compared to the other proposal, and to work at least for now that we do DSDS only.

What do you think?

Comment 23

[Preeti Raghunath(:Preeti)]

David,

Given Julien's active participation on this bug, can he please take on this bug as assignee.

Comment 24

[Julien Wajsberg [:julienw]]

Preeti, David, we first need to know if we do something in Gecko or not. This is still an ongoing discussion with Bevis and Hsin-Yi.

Comment 25

[David Scravaglieri [:scravag]]

clear n?

Comment 26

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #22)
> 4. The sending message needs to be saved somewhere.
Not an issue IMHO, because this can be handled either 
by saving it to the draft or calling sendMMS() to save it into the MobileMessageDB.

> 
> For me it's a "hint" that the API can do the switch. We can call it
> "withSwitch" or "switchAccepted" if you prefer. To me, it's really like the
> "will-change" CSS property that gives a hint that some part will be animated.
> 
> What I like a lot in this proposal, it's that it is DSDS/DSDA/SingleSim
> agnostic for Gaia, it wouldn't need any change in Gaia when we start
> supporting DSDA. All other proposals need a lot of conditions everywhere to
> detect whether we're in DSDS/DSDA/SingleSim scenario (not to mention DSDS
> with one SIM). Of course the other proposals would work, but would yield in
> my opinion much complexity. Now, maybe the complexity would be in Gecko, but
> I think Gecko has much more information to know the current status.

We know your viewpoint.

However, from the perspective the WebAPI design, 
we prefer to think more about how this API shall be designed to ensure that
1. Each API precisely do what it should do (neither less nor more), instead
   of adding too much logic into 1 API.
2. Keep UX policy independent from the API.
This increases the flexibility for application to design different UX.
For example, maybe in some market, customers prefer to send/retrieve MMS without
any warning about switching the SIM.

> 
> > 
> > [Proposal from Gecko]
> > 1. Bug 985945 has to be introduced to provide a general solution for
> >    apps to define their-own UX behaviors.
> > 2. For the implementation of sendMMS/retrieveMMS API, to make sure that it
> > matches 
> >    the semantic meaning of these APIs:
> >    a. Gecko has to accomplish the MMS transaction to the specified serviceId
> > including
> >       switching the active SIM when needed during the transaction for DSDS.
> >    b. For sendMMS, if there is no serviceId specified, 
> >       default serviceId in the settings will be used.
> >    c. In DSDS, the switching is triggered on the fly per sending/retriving
> > request.
> >       Hence, there shall be no change to both |dom.mms.defaultServiceId| and 
> >       |ril.data.defaultServiceId| during the transaction.
> >    D. The switching has to be restored after request is completed for DSDS.
> >       (Note: This has to be implemented, reviewed, and verified very very
> > carefully.)
> > 
> > This advantage is that
> > 1. We achieve the ultimate implementation of sendMMS/retrieveMMS API.
> > 2. We have clear cut of the responsibility of each API.
> > 3. 3 major issues can all be resolved in a good manner.
> > The only disadvantage is that
> > Message App has to handle issue#1 with the new API in Bug 985945.
> > However, this shall not be too complicated in our internal discussion.
> > 
> > [Schedule to support this in 1.4]
> > No matter which proposal mentioned above, it will be a big design change in
> > gecko side.
> > So far, this is an unexpected change especially after 1.4 FC.
> 
> This change would also yield many changes in Gaia so it's clearly not what
> we should do in 1.4 in my opinion.

Yes, that's why I rise my suggestion and question in comment 9 and comment comment 17. 

> 
> I'm not sure about restoring the switching, especially because it can take a
> lot of time. Also, we'll probably need to change the UX and the texts...
> 
> I had also another proposal that you didn't comment much, for DSDS (so it
> doesn't solve the complexity issue I outlined above):
> 
> 1. User wants to send a MMS using SIM2 whereas SIM1 is the current default
> one
> 2. Gaia asks confirmation, user confirms
> 3. Gaia changes the serviceId setting (like it does now)
> 4. Gaia sends the MMS when the "setting set" operation's success event is
> sent
> 5. Gecko stores the message in the DB, send the "sending" event
> 5. Gecko checks that the serviceId setting match the requested serviceId
>   5a. Fire an error if it's not right
> 6. Gecko handles the datachange event
>   6a. Fire an error if the wrong serviceId is registered (for example the
> user could have changed the setting because it was taking too long)
> 7. Gecko sends the MMS once the requested serviceId is "registered"
> 
> 
> In my local tests, I think that much of it already just works. I didn't try
> 4. yet but I did try to send a MMS while the switch was happening (so about
> 30 seconds after the switch was requested), and this MMS was sent correctly.

Actually, step 4 looks buggy to us because there is no promise that 
MmsService will be aware of the change of the setting, after 
Gaia requests sendMMS() in step. This will introduce a bad UX due to 
a timing issue that causes the failure mentioned in 5a.

In addition, it is also an complex logic for MmsSevice to handle the switching 
with RadioInterfaceLayer smoonthly. Hence, it also looks risky to 
introduce it in 1.4 in a rush.

> This has the advantage of being a lot simpler compared to the other
> proposal, and to work at least for now that we do DSDS only.
> 
> What do you think?

Comment 27

[Bevis Tseng[:bevis][:btseng](Exited)]

> Actually, step 4 looks buggy to us because there is no promise that 
> MmsService will be aware of the change of the setting, after 
> Gaia requests sendMMS() in step. This will introduce a bad UX due to 
Sorry, correct typo here:
  Gaia requests sendMMS() in step 4.

Comment 28

[Paul Yang [:pyang] (away)]

Clean qawanted since we review current behavior already

Comment 29

[Steve Chung [:steveck]]

Just have some discussion with gecko devs yesterday. Here is some brief summarize:

The biggest problem in the current codebase is composing message will be lost if message app is killed (no matter it's oom or user tired of waiting for sending) when SIM switching before sending because of default outgoing message SIM and default data SIM is unmatched.

So Julien proposed to:
- Modify the sendMMS api to consider the SIM data status and other options. The best case is gecko can send the mms automatically when data is ready. But gecko devs against this idea because of they think message sending behavior should not bind with data status change.

Gecko devs proposed:
- Saving the message into draft before message actually send. The good news is message will be save to draft automatically when composer out of focus. We don't need to do too much additional work for that. But we might need to block user to edit the message during SIM switching and having some UI feedback to notify user we are switching the SIM.

So, without schedlue concern, can we make a conclusion about whether gecko will the change api like Julien said in the future? If it's true, I believe it's non-trivial task for them and we might create another bug for that.(ni? Bevis)

Is this issue blocked by data lost or also pending message should be sent automatically? If this issue is only blocked by data lost concern, is saving message to draft a resonable solution? (The data will not lost when app killed but the content will stay in composer and will not sent automatically) ni? PM and UX.

Comment 30

[Julien Wajsberg [:julienw]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #26)

> 
> However, from the perspective the WebAPI design, 
> we prefer to think more about how this API shall be designed to ensure that
> 1. Each API precisely do what it should do (neither less nor more), instead
>    of adding too much logic into 1 API.
> 2. Keep UX policy independent from the API.
> This increases the flexibility for application to design different UX.
> For example, maybe in some market, customers prefer to send/retrieve MMS
> without
> any warning about switching the SIM.

I don't see how any proposal would impair this possibility.

> 
> > 
> > I'm not sure about restoring the switching, especially because it can take a
> > lot of time. Also, we'll probably need to change the UX and the texts...
> > 
> > I had also another proposal that you didn't comment much, for DSDS (so it
> > doesn't solve the complexity issue I outlined above):
> > 
> > 1. User wants to send a MMS using SIM2 whereas SIM1 is the current default
> > one
> > 2. Gaia asks confirmation, user confirms
> > 3. Gaia changes the serviceId setting (like it does now)
> > 4. Gaia sends the MMS when the "setting set" operation's success event is
> > sent
> > 5. Gecko stores the message in the DB, send the "sending" event
> > 5. Gecko checks that the serviceId setting match the requested serviceId
> >   5a. Fire an error if it's not right
> > 6. Gecko handles the datachange event
> >   6a. Fire an error if the wrong serviceId is registered (for example the
> > user could have changed the setting because it was taking too long)
> > 7. Gecko sends the MMS once the requested serviceId is "registered"
> > 
> > 
> > In my local tests, I think that much of it already just works. I didn't try
> > 4. yet but I did try to send a MMS while the switch was happening (so about
> > 30 seconds after the switch was requested), and this MMS was sent correctly.
> 
> Actually, step 4 looks buggy to us because there is no promise that 
> MmsService will be aware of the change of the setting, after 
> Gaia requests sendMMS() in step. 

Sorry, I don't understand this: why the API couldn't retrieve the current serviceId at that moment ?


> This will introduce a bad UX due to 
> a timing issue that causes the failure mentioned in 5a.
> 
> In addition, it is also an complex logic for MmsSevice to handle the
> switching 
> with RadioInterfaceLayer smoonthly. Hence, it also looks risky to 
> introduce it in 1.4 in a rush.

How do you explain that sending a MMS while the switching is happening currently _just_ _works_ ?
What I mean is that maybe everything is already working as expected?

I can try to implement this to see whether this fails. In the past, when we worked with retrieveMMS, we didn't wait for the "set" success event to call retrieveMMS, and I believe this is why it failed.


(In reply to Steve Chung [:steveck] from comment #29)
> Just have some discussion with gecko devs yesterday. Here is some brief
> summarize:

Thanks ! Discussing in the same room is easier ;)

 
> Gecko devs proposed:
> - Saving the message into draft before message actually send. The good news
> is message will be save to draft automatically when composer out of focus.
> We don't need to do too much additional work for that. But we might need to
> block user to edit the message during SIM switching and having some UI
> feedback to notify user we are switching the SIM.

Yep, that's the main issue here: the UX is not easy to get right, and blocking the user is  new behavior. Also, do we really want to block the user so much time? Remember also the SIM switching might never succeed.

I think the ideal UX is to do as if the message is saved in the MMS database, and simulate it in Gaia. Maybe implement my ideal API in MessageManager, as a wrapper to the real API.

> 
> So, without schedlue concern, can we make a conclusion about whether gecko
> will the change api like Julien said in the future? If it's true, I believe
> it's non-trivial task for them and we might create another bug for that.(ni?
> Bevis)
> 
> Is this issue blocked by data lost or also pending message should be sent
> automatically? If this issue is only blocked by data lost concern, is saving
> message to draft a resonable solution? (The data will not lost when app
> killed but the content will stay in composer and will not sent
> automatically) ni? PM and UX.

Saving into draft would save the dataloss issue, but the UX would still be awkward. I've already spent a significant time with Ayman trying to solve this problem, and nothing really satisfied me. In my opinion, from the user point of view, the message should just look "pending", and be in "error" state after some time or if the app is restarted.

Comment 31

[Steve Chung [:steveck]]

BTW, If we still need to save the message into db for 1.4, I have a rough idea about having a minimal changes in both side:

We just call sendMMS anyway without checking anything first.

And for gecko: save the message to db and only 'check' if SIM is matched. If not, return failed event to gaia directly without actually sent it(There is no api changes here but it seems gecko devs have some difficulty for this part)

For gaia: We already handled failed event in gaia side, but the callback will not contain any error related message. That means we need to check the service ID when failed. If SIM unmatch, popup confirm prompt and:

a) If user decides not switch, stay the message as error status seems reasonable.
b) If user decides switch, we can do what we did now. Since message already saved in db, there is no worry about the data lost, and user can resend any time if the app is killed before sending. User may still sees the error icon in the bubble while SIM switching, but gaia can do something tricky to force the bubble shown as pending style.

Does it sounds fine for both side(for 1.4)?

Comment 32

[Julien Wajsberg [:julienw]]

(In reply to Steve Chung [:steveck] from comment #31)
> BTW, If we still need to save the message into db for 1.4, I have a rough
> idea about having a minimal changes in both side:
> 
> We just call sendMMS anyway without checking anything first.
> 
> And for gecko: save the message to db and only 'check' if SIM is matched. If
> not, return failed event to gaia directly without actually sent it(There is
> no api changes here but it seems gecko devs have some difficulty for this
> part)

Yep, I think the Gecko developers will have the same concern as Comment 26 :(

> 
> For gaia: We already handled failed event in gaia side, but the callback
> will not contain any error related message. That means we need to check the
> service ID when failed. If SIM unmatch, popup confirm prompt

I think we can have a specific error code like we do for retrieveMMS?

> and:
> 
> a) If user decides not switch, stay the message as error status seems
> reasonable.

I agree with this.

> b) If user decides switch, we can do what we did now. Since message already
> saved in db, there is no worry about the data lost, and user can resend any
> time if the app is killed before sending. User may still sees the error icon
> in the bubble while SIM switching, but gaia can do something tricky to force
> the bubble shown as pending style.

We could reuse our "resend" logic. We need to do DSDS for this "resend" logic anyway, that's not done yet.

> 
> Does it sounds fine for both side(for 1.4)?

It would work for me and is certainly easier than what I suggested in the end of comment 30!


Note that I couldn't try what I said in comment 30 yet (about trying to send the MMS just after setting the setting) because gaia was in very bad shape today :(

Comment 33

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #30)
> (In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #26)
> 
> > 
> > However, from the perspective the WebAPI design, 
> > we prefer to think more about how this API shall be designed to ensure that
> > 1. Each API precisely do what it should do (neither less nor more), instead
> >    of adding too much logic into 1 API.
> > 2. Keep UX policy independent from the API.
> > This increases the flexibility for application to design different UX.
> > For example, maybe in some market, customers prefer to send/retrieve MMS
> > without
> > any warning about switching the SIM.
> 
> I don't see how any proposal would impair this possibility.
> 

No, the difference to introdue Bug 985945 is that besides 
checking SIM Switch for sendMMS & retrieveMMS, we'd like to 
have a more general API for Applications to know the capability of the devices.
This allows app developers to define their own UX for the functionality in 
mozMobileMessage, mozTelephony, and etc.


> > 
> > Actually, step 4 looks buggy to us because there is no promise that 
> > MmsService will be aware of the change of the setting, after 
> > Gaia requests sendMMS() in step. 
> 
> Sorry, I don't understand this: why the API couldn't retrieve the current
> serviceId at that moment ?
> 
> 
> > This will introduce a bad UX due to 
> > a timing issue that causes the failure mentioned in 5a.
> > 
> > In addition, it is also an complex logic for MmsSevice to handle the
> > switching 
> > with RadioInterfaceLayer smoonthly. Hence, it also looks risky to 
> > introduce it in 1.4 in a rush.
> 
> How do you explain that sending a MMS while the switching is happening
> currently _just_ _works_ ?
> What I mean is that maybe everything is already working as expected?
> 
> I can try to implement this to see whether this fails. In the past, when we
> worked with retrieveMMS, we didn't wait for the "set" success event to call
> retrieveMMS, and I believe this is why it failed.

Sorry that I didn't explain it well in detail.
In current implementation, we use observer in MmsService to know if 
the preference of defaultServiceId is changed.
So, there is no guarantee that MmsService will aware the change when 
Gaia invokes sendMMS right after gaia change the settings.
There must be potential timing issue that cause it failed just like 
what you have met in retrieving mentioned above.

Comment 34

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Steve Chung [:steveck] from comment #29)
> So, without schedlue concern, can we make a conclusion about whether gecko
> will the change api like Julien said in the future? If it's true, I believe
> it's non-trivial task for them and we might create another bug for that.(ni?
> Bevis)
> 
Yes, we'd definitely need to enhance these APIs BUT with our proposal in
comment 20 due to the reasons we mentioned in comment 20 and comment 33.

Comment 35

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Steve Chung [:steveck] from comment #31)
> And for gecko: save the message to db and only 'check' if SIM is matched. If
> not, return failed event to gaia directly without actually sent it(There is
> no api changes here but it seems gecko devs have some difficulty for this
> part)
I'll try this today for short term solution and feedback you the result soon. :)

Comment 36

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Steve Chung [:steveck] from comment #31)
> BTW, If we still need to save the message into db for 1.4, I have a rough
> idea about having a minimal changes in both side:
> 
> We just call sendMMS anyway without checking anything first.
> 
> And for gecko: save the message to db and only 'check' if SIM is matched. If
> not, return failed event to gaia directly without actually sent it(There is
> no api changes here but it seems gecko devs have some difficulty for this
> part)
The difficulty during our discussion is to have saving & switching SIM ready 
in gecko in current stage. :)

Comment 37

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Steve Chung [:steveck] from comment #31)
> BTW, If we still need to save the message into db for 1.4, I have a rough
> idea about having a minimal changes in both side:
> 
> We just call sendMMS anyway without checking anything first.
> 
> And for gecko: save the message to db and only 'check' if SIM is matched. If
> not, return failed event to gaia directly without actually sent it(There is
> no api changes here but it seems gecko devs have some difficulty for this
> part)
> 
> For gaia: We already handled failed event in gaia side, but the callback
> will not contain any error related message. That means we need to check the
> service ID when failed. If SIM unmatch, popup confirm prompt and:

"NonActiveSimCardError" shall be returned in this case for the solution in comment 35.

Comment 38

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: WIP: Return NonActiveSimCardError for sending MMS if specified service Id is not the default one in DSDS.

WIP patch to address the short-term solution in comment 31.

Comment 39

[Steve Chung [:steveck]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #34)
> (In reply to Steve Chung [:steveck] from comment #29)
> > So, without schedlue concern, can we make a conclusion about whether gecko
> > will the change api like Julien said in the future? If it's true, I believe
> > it's non-trivial task for them and we might create another bug for that.(ni?
> > Bevis)
> > 
> Yes, we'd definitely need to enhance these APIs BUT with our proposal in
> comment 20 due to the reasons we mentioned in comment 20 and comment 33.

Would you mind creating another follow-up RIL bug including the you and Julien's discussion?

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #32)
> Note that I couldn't try what I said in comment 30 yet (about trying to send the MMS just after
> setting the setting) because gaia was in very bad shape today :(

I can't not get a DSDS device right now but I got some concern about sending the mms before SIM status ready: Based on previous discussion, it seems not easy for mozMoblieMessage to spy the status of data, and it will increase the complexity of sending logic for sure. Another thing is the message will probably be stuck in resend state easily, and it might take very looong time for current retry mechanism. But I think Bevis could know more detail about what could happend when we send a message while data is not ready yet.

Comment 40

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi Julien,

For the short-term solution that Steve has suggested in comment 31,
I've uploaded a WIP patch in gecko part for you to see if this can help you to resolve this 1.4 blocker.
The solution is to return a error with "NonActiveSimCardError" if the specified service Id is not active after saving the sending message.

The switching logic is still remained in gaia by reusing what you already have to switch SIM while retrieving the MMS with non active SIM.

Marked as WIP because the corresponding test case is still needed. If it's workable, I would like to
file another bug to block this bug to follow up.

NI you to give it a try. :)

Comment 41

[Julien Wajsberg [:julienw]]

Steve, do you want to take over this bug, as obviously I didn't find the time to work on it?

Comment 42

[Steve Chung [:steveck]]

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #41)
> Steve, do you want to take over this bug, as obviously I didn't find the
> time to work on it?

Sure I can take it since Bevis already came up a WIP here.

Comment 43

[Steve Chung [:steveck]]

Attachment: Link to github

Hi Julien, it's just a WIP without test case fixing and changes. The patch might not so straightforward because of the limitation in the error callback and error handling structure has some compromise for that. Feel free to leave any thought if you got better idea against this limitation, thanks!

Comment 44

[Julien Wajsberg [:julienw]]

Comment on attachment 8403201 [details] [review]
Link to github

gave some feedback on github

Comment 45

[Steve Chung [:steveck]]

Comment on attachment 8403201 [details] [review]
Link to github

Hi Julien, I updated the patch with some changes:
- Add showErrorInFailedEvent to check the error message dialog should show or not.(The callback return sequence is request error -> on message failed)

- Refine the resend. Use option for onsuccess/onerror/... instead of single callback function.

And thanks for the feedbacks :)

Comment 46

[Julien Wajsberg [:julienw]]

Comment on attachment 8403201 [details] [review]
Link to github

this starts to look good :)

some more feedback on github.

Comment 47

[Julien Wajsberg [:julienw]]

Hey Bevis, I'd like to make sure that you can't add the message itself (or its id) to the sendMMS onerror callback for 'NonActiveSimCardError'. This would save a lot of code for us and I don't think this would add complexity to the Gecko API side. However I know this involves a lot of glue work...

Comment 48

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi Julien,

Are you talking about bug 824717?
We need to consider more not only for sendMMS with error of NonActiveSimCardError.
The callback of MobileMessageCallback::notifySendMessageFailed() [1] is commonly used for both sendMMS and sendSMS and is also used in Android platform.

I am sorry to say that currently I am blocked on other critical bugs and have no time to look into this yet. :(

If this is a MUST for 1.4, please don't hesitate to let me know.
I'll discuss internally to see if we can re-arrange this in priority.

[1] http://dxr.mozilla.org/mozilla-central/search?q=notifySendMessageFailed&case=false

Comment 49

[chris hofmann]

Hard to follow exactly what is going on here, but do the UX changes, and dialog mentioned several places in the bug involve adding new strings.  we are past string freeze so the bug should be marked with the  "late-l10n" keyword if that is the case.  we can start all the final localization work until the last strings are landed.

Comment 50

[Julien Wajsberg [:julienw]]

Chris, there is no string changes here, we're reusing previous dialogs we landed a few weeks ago :)

Bevis, I was talking about bug 824717 but since this is too much work to do it for all errors (as I saw when I looked at the codem itself) we could really benefit _now_ from having it only for NonActiveSimCardError (which could be less work using my WIP patch, but I don't really know).

Comment 51

[Julien Wajsberg [:julienw]]

Bevis, if the idea works for you, I could even try to provide a patch for only this error, so that you can focus on other work.

Comment 52

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi Julien,

I'd prefer to have a complete solution of bug 824717 to 
return the message id for all sent failure cases.
There is no difference for either NonActiveSimCardError or all other errors because 
the DOMRequest has to be changed.

Can we have this in 1.5 instead if there is already a solution available in gaia?

Comment 53

[Julien Wajsberg [:julienw]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #52)
> Hi Julien,
> 
> I'd prefer to have a complete solution of bug 824717 to 
> return the message id for all sent failure cases.
> There is no difference for either NonActiveSimCardError or all other errors
> because 
> the DOMRequest has to be changed.

As far as I've seen in the existing code, there is a lot of duplicate code which makes it very uneasy to change anything in this part. Changing the DOMRequest is unfortunately not enough, I've done it in the WIP patch, actually changing the existing code is more tedious and error prone :( That's why I offered to change it only for this error, and do the other errors in another patch.


But well I guess we can do the workaround using a boolean state in Gaia, which is really _really_ ugly, that's why I wanted to avoid it.

But then, please really fix bug 824717 in 1.5 as this is open for ages and I am really sad that it's not available to fix this bug now.

Comment 54

[Julien Wajsberg [:julienw]]

Steve, let's move forward here, thanks for your work !

Comment 55

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] from comment #53)
> (In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #52)
> As far as I've seen in the existing code, there is a lot of duplicate code
> which makes it very uneasy to change anything in this part. Changing the
> DOMRequest is unfortunately not enough, I've done it in the WIP patch,
> actually changing the existing code is more tedious and error prone :(
> That's why I offered to change it only for this error, and do the other
> errors in another patch.

May I have a look at your WIP patch for reference?

> 
> But well I guess we can do the workaround using a boolean state in Gaia,
> which is really _really_ ugly, that's why I wanted to avoid it.
> 
> But then, please really fix bug 824717 in 1.5 as this is open for ages and I
> am really sad that it's not available to fix this bug now.

Sorry about this. I'll look into it in 1.5. :(

Comment 56

[Wilfred Mathanaraj [:WDM]]

We can backlog it, but i agree its a bad experience - I would though not block the release for it - please work on as time permits

Comment 57

[Julien Wajsberg [:julienw]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #55)
> (In reply to Julien Wajsberg [:julienw] from comment #53)
> > (In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #52)
> > As far as I've seen in the existing code, there is a lot of duplicate code
> > which makes it very uneasy to change anything in this part. Changing the
> > DOMRequest is unfortunately not enough, I've done it in the WIP patch,
> > actually changing the existing code is more tedious and error prone :(
> > That's why I offered to change it only for this error, and do the other
> > errors in another patch.
> 
> May I have a look at your WIP patch for reference?
> 

It's the feedbacked patch on this bug :) https://github.com/mozilla-b2g/gaia/pull/18072

Comment 58

[Steve Chung [:steveck]]

Comment on attachment 8403201 [details] [review]
Link to github

Hi Julien, this patch is based on your suggestion in WIP with test cases changes. Although it's moved to backlog, I think it's still nice to apply it because of some refinement.

Comment 59

[Julien Wajsberg [:julienw]]

Comment on attachment 8403201 [details] [review]
Link to github

I'd like another round of review. I also didn't try on the fugu yet, but I'm flashing it now with a new central/master and I'll try it tonight.

Comment 60

[Steve Chung [:steveck]]

Comment on attachment 8403201 [details] [review]
Link to github

Update for switchMmsSimHandler promise reject part(and with some test changes), thanks!

Comment 61

[Julien Wajsberg [:julienw]]

Comment on attachment 8403201 [details] [review]
Link to github

I tested the previous patch on the Fugu and I had some weird issues: when I pressed "next" it looks like we were trying to send the message asap, and then I got the same error again.

I don't know if this is due to the fugu being slow or if it's the new comment I left on the pull request, but I'd like to try again once you make the new changes. Sorry that I didn't see this before...

Comment 62

[Steve Chung [:steveck]]

Comment on attachment 8403201 [details] [review]
Link to github

Hey Julien, thanks for discovering this bug after applying promise on resend. It's because the wrapped error event in reject will become slower than message failed event, and sending dialog will not display correctly if the calling sequence is wrong... So I revert the promise change in resned and sync with sendMMS api in the latset patch(and with some nits fixing for sure).

Comment 63

[Julien Wajsberg [:julienw]]

(In reply to Steve Chung [:steveck] from comment #62)
> Comment on attachment 8403201 [details] [review]
> Link to github
> 
> Hey Julien, thanks for discovering this bug after applying promise on
> resend. It's because the wrapped error event in reject will become slower
> than message failed event, and sending dialog will not display correctly if
> the calling sequence is wrong... So I revert the promise change in resned
> and sync with sendMMS api in the latset patch(and with some nits fixing for
> sure).

ok, too bad, I guess we'll need to wait for bug 824717 to do it properly.. because doing code to properly handle this is otherwise not very fun :(

Comment 64

[Julien Wajsberg [:julienw]]

Comment on attachment 8403201 [details] [review]
Link to github

r=me
This works fine both on fugu and buri.

Thanks !

Please squash and rebase and resolve confict and wait for a green travis :)

Comment 65

[Steve Chung [:steveck]]

Commits squashed with green travis. Thanks for the review!
in master: d0c04fc2a93b68c754b6fb438737e0b912a25958

Comment 67

[Wei Gao (Spreadtrum)]

Dear Steve

Could the patch be landed on v1.4?
I think v1.4 also needs it.
Thanks.

Comment 68

[Julien Wajsberg [:julienw]]

I tried quite hard to make this bug a blocker but it was rejected twice, in comment 6 and comment 56.

I think it should apply on v1.4 though, and as far as I know there was no regression because of this patch, so it should be reasonnably safe. Still it's a quite big patch so please test your build carefully.

Comment 70

[Wei Gao (Spreadtrum)]

(In reply to Julien Wajsberg [:julienw] (PTO 08/20 -> 09/15; contact schung instead) from comment #68)
> I tried quite hard to make this bug a blocker but it was rejected twice, in
> comment 6 and comment 56.
> 
> I think it should apply on v1.4 though, and as far as I know there was no
> regression because of this patch, so it should be reasonnably safe. Still
> it's a quite big patch so please test your build carefully.

Dear Julien

Thanks for your hardwork.
Do you mean we can't land this whole patch on v1.4 anyhow. 
If that's the case, can a small patch be granted to landed? or I can only make a patch locally?
After all, I hope it can be landed on 1.4 thread.

Thanks.

Comment 71

[Julien Wajsberg [:julienw]]

Answered in bug 1054911 already.

I think relman won't allow a patch to land on our 1.4 branch now. Let's ask Bhavana!
Hey Bhavana, Spreadtrum produced a much smaller and safer patch in bug 1054911, do you think it could go to v1.4? It's not clear to me who choose what lands on v1.4 branch these days :)

Comment 72

[bhavana bajaj [:bajaj]]

(In reply to Julien Wajsberg [:julienw] (PTO 08/20 -> 09/15; contact schung instead) from comment #71)
> Answered in bug 1054911 already.
> 
> I think relman won't allow a patch to land on our 1.4 branch now. Let's ask
> Bhavana!
> Hey Bhavana, Spreadtrum produced a much smaller and safer patch in bug
> 1054911, do you think it could go to v1.4? It's not clear to me who choose
> what lands on v1.4 branch these days :)

HI Julien,

Switching the NI to wayne who is helping with 1.4 branch these days. wayne, the patch in 1054911 looks low risk, but we would have to get that on master first after its reviewed and tested followed by 1.4 landing if we made the blocking call.

Comment 73

[Wayne Chang [:wchang]]

Let's resolve comment 70 with bug 1054911 following bhavana's comment 72 since that seems to be a smaller patch resolving a smaller part of the problem in question.