Lionel Mauritson Reporter
	Description • 11 years ago

Description:
When there is an MMS attachment pending if the user switches the SIM card and tries to then download the MMS attachment the message "Missing SIM card" appears even though the new SIM card can be used. Essentially the message is inappropriate for the situation.

Repro Steps:
1) Update a Buri to BuildID: 20140307040203
2) Go to Settings -> Messaging settings and turn Auto retrieve OFF.
3) Send an MMS from another device to the test device.
4) Open the message app to view the message, but do not download the attachment.
5) Power down the test device and change to another valid SIM.
6) Power back on the device and go to the message app.
7) Attempt to download the MMS attachment.

Actual:
The message "Missing SIM card" appears

Expected:
A message properly explaining why the attachment cannot be downloaded is displayed.

1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140307040203
Gaia: 04eb7996543f114133d1367f834a4d88b68c60ac
Gecko: b0e7f63c2138
Version: 30.0a1
Firmware Version: v1.2-device.cfg


Keywords: SMS


Notes: Likely related to https://bugzilla.mozilla.org/show_bug.cgi?id=924178


Repro frequency: 100%
Link to failed test case: https://moztrap.mozilla.org/manage/case/8275/
See attached: screenshot of message

	Jason Smith [:jsmith]
	Comment 1 • 11 years ago

QA Wanted to check behavior on 1.3.

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 2 • 11 years ago

Hi Julien,

Is it an wording problem when handling |NonActiveSimCardError| if
the iccId of the the received MMS notification is not the same to
the iccId that was inserted in step#5 of the reproducible steps?

	Julien Wajsberg [:julienw]
	Comment 3 • 11 years ago

yes, I think we could handle it in Gaia. We normally don't do much iccId-related stuff in Single SIM devices, but this is definitely possible.

Another possibility is to have different error codes from Gecko for these different cases: one case is "we don't have any ready SIM card" and the other case is "this is not the right SIM card for this MMS message". In the past, in such situations, Gene usually added new error codes, but you might ask him his thoughts on this.


I also wonder how it worked before we had iccId on the message, I mean 1.1 and 1.2.

Could QA please check this on 1.1 too?

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 4 • 11 years ago

Before 1.3, MmsService always tried to retrieve the message without checking the iccId.
Hence, if the inserted SIM is provided by the same operator of the one related to the MMS notification.
It shall be able to be downloaded due to the same MMS APN configuration.
If not, I think it is always not able to be downloaded.
However, there shall be some privacy problem if a MMS can be downloaded by different SIM.

For reusing NonActiveSimCardError or defining a new one, I'll discuss internally and feedback later.

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 5 • 11 years ago

Sorry, I've to correct what I said in comment 2 and comment 4, the error we throw in this scenario is |NoSimCardError| instead of |NonActiveSimCardError|.

Currently, 2 scenario of retrieval hit this warning message:
1. retrieve without SIM.
2. retrieve with different SIM.

	Julien Wajsberg [:julienw]
	Comment 6 • 11 years ago

Bevis, after reading the code a little more, I think we can now do this in Gaia: we can check if the right iccId is registered and present before trying to retrieve the MMS content. This will also lead to a clearer code in Gaia.

Tell me if you prefer to do this in Gecko.

	Montoya Clemmons (MClemmons)
	Comment 7 • 11 years ago

(In reply to Jason Smith [:jsmith] from comment #1)
> QA Wanted to check behavior on 1.3.

Following the STR from Comment 0, the 1.3 Buri behavior is a red exclamation point icon appearing to the left of the device screen near the location where user taps the download button for the MMS. At no time does the message reported for the 1.4 Buri "Missing SIM card" display on the 1.3 Buri. 

Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140311004003
Gaia: 6b1180917fa4af4e4b7e31b51fc06a4bbaa2ad0f
Gecko: 8976860941f2
Version: 28.0
Base image: v1.2-device.cfg

	Montoya Clemmons (MClemmons)
	Comment 8 • 11 years ago

(In reply to Julien Wajsberg [:julienw] from comment #3)
> yes, I think we could handle it in Gaia. We normally don't do much
> iccId-related stuff in Single SIM devices, but this is definitely possible.
> 
> Another possibility is to have different error codes from Gecko for these
> different cases: one case is "we don't have any ready SIM card" and the
> other case is "this is not the right SIM card for this MMS message". In the
> past, in such situations, Gene usually added new error codes, but you might
> ask him his thoughts on this.
> 
> 
> I also wonder how it worked before we had iccId on the message, I mean 1.1
> and 1.2.
> 
> Could QA please check this on 1.1 too?

Following the STR from Comment 0, the behavior on Buri 1.1 is a service denied message appearing after user selects the download button. At no time does the message reported for the 1.4 Buri "Missing SIM card" display on 1.1 Buri.

Environmental Variables:
Device: Buri 1.1 MOZ
BuildID: 20140311041204
Gaia: 44a2ddf63373f8e95c784faf4ed4d60081699c61
Gecko: 1421a6b7fc51
Version: 18.0
Base image: v1.2-device.cfg

	Jason Smith [:jsmith]
	Comment 9 • 11 years ago

If I understand this bug correctly - we're just firing a bad error message here, right? We should never be able to download the MMS attachment in this case, as we're using a different SIM card, right?

I don't think I would call this is a regression, as the UX doesn't really appear to be good in any branch right now.

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 10 • 11 years ago

Yes, you are right.

(In reply to Jason Smith [:jsmith] from comment #9)
> If I understand this bug correctly - we're just firing a bad error message
> here, right? We should never be able to download the MMS attachment in this
> case, as we're using a different SIM card, right?
> 
> I don't think I would call this is a regression, as the UX doesn't really
> appear to be good in any branch right now.

	Julien Wajsberg [:julienw]
	Comment 11 • 11 years ago

Bevis, there is another question for you in comment 6 ;)

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 12 • 11 years ago

Thanks! Julien.

IMHO, no matter gaia has a solution or not.
For Gecko, I think it is still important for MmsService to identify these 2 scenario correctly:
1. retrieve without SIM.
2. retrieve with different SIM.
Hence, new ErrorCode has to be introduced to cover the 2nd one. :)

	Julien Wajsberg [:julienw]
	Comment 13 • 11 years ago

Ok you're right, and actually 2. is also especially important to distinguish DSDS and DSDA in the future :) Thanks !

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 18 • 11 years ago

update the uuid of nsIRadioInterfaceLayer since interface is changed.

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 19 • 11 years ago

rebase due to dependency changed.

	Vicamo Yang [:vicamo][:vyang]
	Comment 20 • 11 years ago

Comment on attachment 8396214 [details] [diff] [review]
Part2: Define MmsService-owned getConnByIccId() to identify NO_SIM_CARD_ERROR and SIM_NOT_MATCHED_ERROR. r=vyang

Review of attachment 8396214 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/mobilemessage/src/gonk/MmsService.js
@@ +502,5 @@
>        conn = this._connections[id] = new MmsConnection(id);
>        conn.init();
>        return conn;
>      },
> +    getConnByIccId: function(aIccId) {

nit: an empty line between methods.

@@ +513,5 @@
> +        // download due to the wrong MMSC and proxy settings.
> +        return this.getConnByServiceId(0);
> +      }
> +
> +      let numCardAbsent = 0;

let cardPresent = false;
for (...) {
  ...
  if (iccId == aIccId) {
    return mmsConnection;
  }

  if (iccId) {
    cardPresent = true;
  }
}

throw (cardPresent ? _MMS_ERROR_SIM_NOT_MATCHED : _MMS_ERROR_NO_SIM_CARD);

	Vicamo Yang [:vicamo][:vyang]
	Comment 21 • 11 years ago

Comment on attachment 8396961 [details] [diff] [review]
Part 4: Add test case for SIM_NOT_MATCHED_ERROR while manually retrieving MMS message. r=vyang

Review of attachment 8396961 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/nsIRadioInterfaceLayer.idl
@@ +141,5 @@
>  
> +  /**
> +   * If not available, throws exception; otherwise, a valid number.
> +   */
> +  unsigned long getClientIdByIccId(in DOMString iccId);

???

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 22 • 11 years ago

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #21)
> Comment on attachment 8396961 [details] [diff] [review]
> Part 4: Add test case for SIM_NOT_MATCHED_ERROR while manually retrieving
> MMS message. r=vyang
> 
> Review of attachment 8396961 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/nsIRadioInterfaceLayer.idl
> @@ +141,5 @@
> >  
> > +  /**
> > +   * If not available, throws exception; otherwise, a valid number.
> > +   */
> > +  unsigned long getClientIdByIccId(in DOMString iccId);
> 
> ???

Oh, no. Sorry, it seems that I accidentally UNDO the change of this file in my editor.

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 23 • 11 years ago

address comment 22.

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 24 • 11 years ago

try server result is green:
https://tbpl.mozilla.org/?tree=Try&rev=981e7f548e3b

	Ryan VanderMeulen [:RyanVM]
	Comment 25 • 11 years ago

Part 2 doesn't apply to b2g-inbound. Please rebase.

	Ryan VanderMeulen [:RyanVM]
	Comment 26 • 11 years ago

Nevermind, missed that this was on top of bug 981577.
https://hg.mozilla.org/integration/b2g-inbound/rev/c3202ada540e
https://hg.mozilla.org/integration/b2g-inbound/rev/4009d99f1e07
https://hg.mozilla.org/integration/b2g-inbound/rev/09774230aa3f
https://hg.mozilla.org/integration/b2g-inbound/rev/972133dfe9df

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 27 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/c3202ada540e
https://hg.mozilla.org/mozilla-central/rev/4009d99f1e07
https://hg.mozilla.org/mozilla-central/rev/09774230aa3f
https://hg.mozilla.org/mozilla-central/rev/972133dfe9df

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 28 • 11 years ago

Hi Julien,

Please be informed to have the corresponding warning message to be displayed for 
the new SimNotMatchedError. :)

	Julien Wajsberg [:julienw]
	Comment 29 • 11 years ago

Filed bug 990020