859215 - [OPEN_][SMS]It does not show SMSC number.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Firefox_Mozilla]

Steps to reproduce:
1.go into SMS or other settings to check whether it contains SMSC number.

Actual result:
1.it does not have menu to show SMSC number;

Expected result:
1.it contain SMSC number and user can edit this number and save.

Comment 1

[Kai-Chih Hu [:khu]]

Hi, Ken, can you help this case? 

This is related to certification process we're having right now. They just need a way to change the SMSC number, no matter how to do it. Reporter, am I right? In this case, I think either providing a tool or script might be good enough. Of course, suggestions and/or comments are welcome. 

Thanks!

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

We have a similar Gecko bug 736701, SMS Reply Path, which suggests we should expose per message SMSC address to content and allow specifying SMSC address in send() as well. However, this is a rare use case and it will take a lot works to implement. The SMSC parameter is now filled by modem, so maybe we can find some other tool to send such setting to modem directly instead.

Comment 3

[Ken Chang[:kenkai]]

chuck, please implement this feature. You can use the setSMSCAddress() to set the SMSC address.
And the data of SMSC address should come from settings.

Comment 4

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Please confirm what you need per comment 1, or describe what you expect more detail, thanks.

Comment 5

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

A interface change is required to support this feature, I propose adding |attribute DOMString smsc;| to nsIDOMMozMobileMessageManager.

Comment 6

[Ken Chang[:kenkai]]

(In reply to Chuck Lee [:chucklee] from comment #5)
> A interface change is required to support this feature, I propose adding
> |attribute DOMString smsc;| to nsIDOMMozMobileMessageManager.
Can we just get SMSC data from seetingDB?

Comment 7

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Ken Chang from comment #6)
> (In reply to Chuck Lee [:chucklee] from comment #5)
> > A interface change is required to support this feature, I propose adding
> > |attribute DOMString smsc;| to nsIDOMMozMobileMessageManager.
> Can we just get SMSC data from seetingDB?

Oh, yes. You are right.

Comment 8

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001.Read SMSC address from modem to settings.

Comment 9

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002.Write SMSC address from settings to modem.

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 737324 [details] [diff] [review]
0001.Read SMSC address from modem to settings.

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

::: dom/system/gonk/ril_worker.js
@@ +784,5 @@
>      this.IMEI = null;
>      this.IMEISV = null;
>      this.ESN = null;
>      this.MEID = null;
> +

extra empty line.

@@ +791,5 @@
> +     * SMSC data object
> +     *
> +     * SMSC address string got from modem is s not simply a string, it's in
> +     * to format "\"SMSC_ADDR\",VALUE". And it requires same format to set
> +     * SMSC address to modem.

There is no such format at least in Galaxy S2, and AOSP doesn't have it, either. Please still keep SMSC a raw string.

@@ +5097,5 @@
> +  // Parse SMSC address string, the format is "\"SMSC_ADDR\",VALUE"
> +  let smscString = Buf.readString();
> +  let smscData = smscString.split(",");
> +  this.SMSC.address = smscData[0].substr(1, smscData[0].length - 2);  // trim heading and tailing \"
> +  this.SMSC.value = parseInt(smscData[1], 10);

ditto.

@@ +5104,5 @@
> +  this.sendDOMMessage({
> +    rilMessageType: "getSMSCAddress",
> +    SMSC: this.SMSC.address
> +  });
> +

extra empty line.

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 737324 [details] [diff] [review]
0001.Read SMSC address from modem to settings.

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +729,5 @@
> +              return;
> +
> +            if (!aResult) {
> +              // Settings is empty, load SMSC address to settings.
> +              lock.set("ril.sms.smsc", message.SMSC, null, null);

Recently we received some request from Gaia developers, having control on a settings at both Gecko & Gaia brings much trouble in UI implementation. Settings should be only for user preference, don't change it back on failure.

So, in order to reflect set failures, we'll need a setSMSCAddress() API in at least MobileMessageManager, and at least a read-only attribute smscAddress for reading as well.

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 737325 [details] [diff] [review]
0002.Write SMSC address from settings to modem.

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +302,5 @@
>    lock.get("ril.supl.user", this);
>    lock.get("ril.supl.passwd", this);
>    lock.get("ril.supl.httpProxyHost", this);
>    lock.get("ril.supl.httpProxyPort", this);
> +  lock.get("ril.sms.smsc", this);

We'll need APIs, not settings entries.

@@ +2060,5 @@
> +      case "ril.sms.smsc":
> +        if (aResult) {
> +          this.worker.postMessage({rilMessageType: "setSMSCAddress", SMSC: aResult});
> +        }
> +        break;

Ditto.

::: dom/system/gonk/ril_worker.js
@@ +1988,5 @@
>    setSMSCAddress: function setSMSCAddress(options) {
> +
> +    // We can't set SMSC Address before we got current SMSC address first, or
> +    // SMSC.value will be lost forever.
> +    if (this.SMSC.address && options.SMSC !== this.SMSC.address) {

Please keep it the format it originally was.

Comment 13

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #11)
> Comment on attachment 737324 [details] [diff] [review]
> 0001.Read SMSC address from modem to settings.
> 
> Review of attachment 737324 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/RadioInterfaceLayer.js
> @@ +729,5 @@
> > +              return;
> > +
> > +            if (!aResult) {
> > +              // Settings is empty, load SMSC address to settings.
> > +              lock.set("ril.sms.smsc", message.SMSC, null, null);
> 
> Recently we received some request from Gaia developers, having control on a
> settings at both Gecko & Gaia brings much trouble in UI implementation.
> Settings should be only for user preference, don't change it back on failure.
> 
> So, in order to reflect set failures, we'll need a setSMSCAddress() API in
> at least MobileMessageManager, and at least a read-only attribute
> smscAddress for reading as well.

How about a |smscAddress| attribute, it can provide both read and write.

By the way, if there are more than one SIM on device, how should we deal with the SMSC setting?

Comment 14

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. RIL IDL change

Comment 15

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. Get SMSC address from modem to RIL

Comment 16

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0003. Set SMSC address from RIL to modem

Comment 17

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0004. IPDL change to support get/set SMSC address

Comment 18

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0005. Mobilemessage IDL change

Comment 19

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0006. Get SMSC from RIL

Comment 20

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0007. Set SMSC to RIL

Comment 21

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Based on my test, SMSC set to SIM card can't be restored in any way, including restart, power off then on, remove battery, remove SIM card.

Comment 22

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. Get/Set SMSC address in RIL

Merge modifications in RIL into single patch, and remove unnecessary IPC.

Comment 23

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0003. IPDL change to support get/set SMSC address

Reorder

Comment 24

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0004. Mobilemessage IDL change

Reorder patch

Comment 25

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0005. Get/Set SMSC address in mobilemessage

Merge modifications on mobile message DOM into single patch.

Comment 26

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 744995 [details] [diff] [review]
0002. Get/Set SMSC address in RIL

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

We don't really need SMSC in ril_worker. If sync them in both RadioInterfaceLayer and ril_worker brings trouble to you, I suggest you remove SMSC attribute from ril_worker.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +2262,5 @@
> +    return this.SMSC;
> +  },
> +  set smscAddress(value) {
> +    if (value) {
> +      this.SMSC = value;

Please bail-out early and set |this.SMSC| in handler of the setSMSCAddress message.

::: dom/system/gonk/ril_worker.js
@@ +792,5 @@
> +    /**
> +     * SMSC Address
> +     *
> +     * SMSC address string got from modem is platform-dependent.
> +     * We get "\"SMSC_ADDR\",VALUE" from MOZ RIL, but might be different

Certainly not "from MOZ RIL". It's on Unagi or the device you're working on.

@@ +794,5 @@
> +     *
> +     * SMSC address string got from modem is platform-dependent.
> +     * We get "\"SMSC_ADDR\",VALUE" from MOZ RIL, but might be different
> +     * on other platform.
> +     * So we store raw string.

nit: line break at near 80 chars.

@@ +1997,5 @@
>     */
>    setSMSCAddress: function setSMSCAddress(options) {
> +    // We can't set SMSC Address before we got current SMSC address first, or
> +    // SMSC.value will be lost forever.
> +    if (this.SMSC && options.SMSC !== this.SMSC) {

All I see here is the control flow may break and get lost forever and results in different values of RadioInterfaceLayer::SMSC and of RIL::SMSC.

@@ +5129,5 @@
>  };
>  RIL[REQUEST_EXIT_EMERGENCY_CALLBACK_MODE] = null;
>  RIL[REQUEST_GET_SMSC_ADDRESS] = function REQUEST_GET_SMSC_ADDRESS(length, options) {
>    if (options.rilRequestError) {
>      return;

Same here.

Comment 27

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. Get/Set SMSC address in RIL. V1.1

Modify based on comment 26.
ril_worker doesn't cache SMSC address.
Instead, it returns message indicates if Get/Set SMSC command is successful, with current SMSC address.
This also helps keeping the control flow.

Comment 28

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0001. IDL change

Merge all IDL change into single file

Comment 29

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0002. IPDL change to support get/set SMSC address.

Modify IPDL since API is changed to DOMRequest-based structure.

Comment 30

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0002. IPDL change to support get/set SMSC address.

Modify IPDL since API is changed to DOMRequest-based structure.

Comment 31

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0003. Get/Set SMSC address in RIL

Modify in DOMRequest-based structure.

Comment 32

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0004. Get/Set SMSC address in mobilemessage

Modify in DOMRequest-based structure.

Comment 33

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 753184 [details] [diff] [review]
WIP - 0002. IPDL change to support get/set SMSC address.

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

I can't find changes made to |SmsRequestChild::Recv__delete__()|, do you miss something?

::: dom/mobilemessage/src/MobileMessageCallback.cpp
@@ +169,5 @@
> +NS_IMETHODIMP
> +MobileMessageCallback::NotifyGetSmscAddress(const nsAString& aSmscAddress)
> +{
> +  AutoJSContext cx;
> +  JS::Rooted<JS::Value> val(cx, STRING_TO_JSVAL(JS_NewUCStringCopyZ(cx, (jschar *)(aSmscAddress.BeginReading()))));

nsAString is not necessarily null terminated.  Besides, JS_NewUCStringCopy{N,Z} may fail and need additional checks before calling STRING_TO_JSVAL().

::: dom/mobilemessage/src/ipc/PSmsRequest.ipdl
@@ +85,5 @@
> +{
> +  int32_t error;
> +};
> +
> +

nit: redundant empty line.

Comment 34

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 753186 [details] [diff] [review]
WIP - 0003. Get/Set SMSC address in RIL

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1028,5 @@
> +      return;
> +    }
> +    delete this._sentSmsEnvelopes[message.envelopeId];
> +    if (!message.rilRequestError) {
> +      options.request.notifySetSmscAddress(message.smscAddress);

rild's response to REQUEST_SET_SMSC_ADDRESS has no extra bytes at all.  You may assign smscAddress at creating envelope and use options.smscAddress instead.

@@ +2780,5 @@
> +      request: request,
> +    });
> +
> +    this.worker.postMessage(options);
> +  },

nit: add an empty line

::: dom/system/gonk/ril_worker.js
@@ +3134,5 @@
>    _processVoiceRegistrationState: function _processVoiceRegistrationState(state) {
>      let rs = this.voiceRegistrationState;
>      let stateChanged = this._processCREG(rs, state);
>      if (stateChanged && rs.connected) {
> +      RIL.getSmscAddress();

Please just remove it.

@@ +5167,5 @@
> +};
> +RIL[REQUEST_SET_SMSC_ADDRESS] = function REQUEST_SET_SMSC_ADDRESS(length, options) {
> +  let smscAddress = null;
> +  if (!options.rilRequestError) {
> +    smscAddress = Buf.readString();

rild's response to REQUEST_SET_SMSC_ADDRESS has no extra bytes at all.

Comment 35

[Vicamo Yang [:vicamo][:vyang]]

Please also create test cases for this.  Or, since the reporter never response to your inquiry, I'm glad to mark this bug as WONTFIX.

Comment 36

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. IDL change

Comment 37

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. IPDL change to support get/set SMSC address.

Address comment 33

Comment 38

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0003. Get/Set SMSC address in RIL

Address comment 34

Comment 39

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0004. Get/Set SMSC address in mobilemessage

Comment 40

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 754362 [details] [diff] [review]
0001. IDL change

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

Hi Mounir,

This task wants to add methods for SMSC address get/set.  Gecko/Android always goes SMS PDU mode so we don't actually need SMSC address for sending messages.  This address is stored in SIM card and we don't have a specific SIM FILE to read/write for it, so we have to go DOMRequest to enable further error handlings.  Please see 3GPP TS 27.005 section 3.3.1	"Service Centre Address +CSCA".

Comment 41

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 754366 [details] [diff] [review]
0003. Get/Set SMSC address in RIL

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

Thank you :) r=me with following nits addressed.

::: dom/mobilemessage/src/ipc/SmsChild.cpp
@@ +192,5 @@
>      case MessageReply::TReplyMarkeMessageReadFail:
>        mReplyRequest->NotifyMarkMessageReadFailed(aReply.get_ReplyMarkeMessageReadFail().error());
>        break;
> +    case MessageReply::TReplyGetSmscAddress:
> +      mReplyRequest->NotifyGetSmscAddress(aReply.get_ReplyGetSmscAddress().smscAddress());

nit: these changes should belong to previous patch.

::: dom/system/gonk/ril_worker.js
@@ +5158,5 @@
> +  this.sendDOMMessage({
> +    rilMessageType: "getSmscAddress",
> +    smscAddress: smscAddress,
> +    envelopeId: options.envelopeId,
> +    rilRequestError: options.rilRequestError

We use 'errorMsg' since bug 848688.

Comment 42

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 754367 [details] [diff] [review]
0004. Get/Set SMSC address in mobilemessage

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

Please also fix fallback/android backends.

Comment 43

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. IPDL change to support get/set SMSC address. V1.1

Move patch for SmsChild.cpp from 0003 to 0002 per comment 41.

Comment 44

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0003. Get/Set SMSC address in RIL. V1.1

Address comment 41.

Comment 45

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0004. Get/Set SMSC address in mobilemessage. V1.1

Add patch for other backend, per comment 42.

Corrsponding try: https://tbpl.mozilla.org/?tree=Try&rev=5e2a61bb8b54

Comment 46

[Mounir Lamouri (:mounir)]

Comment on attachment 754362 [details] [diff] [review]
0001. IDL change

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

Why can't that be a setting?

Comment 47

[Mila Davydova]

New testcase is created for this issue. 
https://moztrap.mozilla.org/manage/cases/?filter-id=8220

Comment 48

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0005. Add support for SMSC command in ril

Comment 49

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - 0006. Add support for SMSC command in emulator

Comment 50

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Mounir Lamouri (:mounir) from comment #46)
> Comment on attachment 754362 [details] [diff] [review]
> 0001. IDL change
> -----------------------------------------------------------------
> Why can't that be a setting?

Like what I've said in comment 40, setting SMSC doesn't necessarily always success.  When it fails, we'll create another setting/reality conflict.  We used to reset that setting back like what's done in cell broadcast channel lists, but that causes troubles to UI developers because they're not the only setter of that setting.

Besides, each SIM card has its own SMSC setting, and a global preference doesn't apply to all SIM cards.  If we keep such string as a preference and applies to a new SIM card of another service provider, it will unlikely to function correctly.

Do you think that reasonable?

Comment 51

[Mounir Lamouri (:mounir)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #50)
> (In reply to Mounir Lamouri (:mounir) from comment #46)
> > Comment on attachment 754362 [details] [diff] [review]
> > 0001. IDL change
> > -----------------------------------------------------------------
> > Why can't that be a setting?
> 
> Like what I've said in comment 40, setting SMSC doesn't necessarily always
> success.  When it fails, we'll create another setting/reality conflict.  We
> used to reset that setting back like what's done in cell broadcast channel
> lists, but that causes troubles to UI developers because they're not the
> only setter of that setting.

Settings need to be set by Gecko too. The UI needs to handle that.

> Besides, each SIM card has its own SMSC setting, and a global preference
> doesn't apply to all SIM cards.  If we keep such string as a preference and
> applies to a new SIM card of another service provider, it will unlikely to
> function correctly.
> 
> Do you think that reasonable?

I do not know exactly how we are going to handle multi-SIM for a bunch of stuff so I don't really have an opinion on this. We should just try to be consistent with other things we do.

Finally, I would have one last question: what's the use case for having these methods? According to comment 0, this would be used by the Settings App. Do we expect other apps to use it?

Comment 52

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 755210 [details] [diff] [review]
WIP - 0005. Add support for SMSC command in ril

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

::: reference-ril/reference-ril.c
@@ +2058,5 @@
> +    ATResponse *p_response = NULL;
> +    int err;
> +    char *line;
> +
> +    err = at_send_command_singleline("AT+CSCA?", "+CSCA: ", &p_response);

nit: SP after "+CSCA:" is not necessary here.

Comment 53

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Mounir Lamouri (:mounir) from comment #51)
> (In reply to Vicamo Yang [:vicamo][:vyang] from comment #50)
> > (In reply to Mounir Lamouri (:mounir) from comment #46)
> > > Comment on attachment 754362 [details] [diff] [review]
> > > 0001. IDL change
> > > -----------------------------------------------------------------
> > > Why can't that be a setting?
> > 
> > Like what I've said in comment 40, setting SMSC doesn't necessarily always
> > success.  When it fails, we'll create another setting/reality conflict.  We
> > used to reset that setting back like what's done in cell broadcast channel
> > lists, but that causes troubles to UI developers because they're not the
> > only setter of that setting.
> 
> Settings need to be set by Gecko too. The UI needs to handle that.

Such Settings has two meanings, user preference and the current effective setting.  When it fails to do something that users wants, should we turn the value back?

For example, "ril.data.enabled" means the user wants to turn on data connection.  When it fails, we don't turn the setting off.

Another example, "ril.cellbroadcast.searchlist", we do turn the setting back to the original one when the operation fails.

So actually there is no consistency now, and that's something we're going to fix.  We're going to establish a rule that RIL never overwrites users' preferences.  Cell Broadcast search list thing will be fixed in bug 865970.

> > Besides, each SIM card has its own SMSC setting, and a global preference
> > doesn't apply to all SIM cards.  If we keep such string as a preference and
> > applies to a new SIM card of another service provider, it will unlikely to
> > function correctly.
> > 
> > Do you think that reasonable?
> 
> I do not know exactly how we are going to handle multi-SIM for a bunch of
> stuff so I don't really have an opinion on this. We should just try to be
> consistent with other things we do.

My example is not about MultiSIM, but, yes, MultiSIM is another problem for these settings, and things just get worse when some radio interfaces return success and others return failures.

> Finally, I would have one last question: what's the use case for having
> these methods? According to comment 0, this would be used by the Settings
> App. Do we expect other apps to use it?

No, SMSC setting is mostly only for production tests.  Normal users usually don't have to modify it.

Comment 54

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 755211 [details] [diff] [review]
WIP - 0006. Add support for SMSC command in emulator

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

::: android/console.c
@@ +1778,5 @@
> +static int
> +do_sms_smsc( ControlClient  client, char*  args )
> +{
> +    SmsAddressRec smscRec, *pSmscRec;
> +    char          smsc[32] = {0};

Please move declarations of pSmscRec and smsc into |if (!args)| block.

::: telephony/android_modem.c
@@ +556,5 @@
>  
>      modem->subscription_source = _amodem_get_cdma_subscription_source( modem );
>      modem->roaming_pref = _amodem_get_cdma_roaming_preference( modem );
> +
> +    sms_address_from_str( &modem->smsc_address, SMSC_ADDRESS, strlen(SMSC_ADDRESS));

Please have:

  tmp = amodem_nvram_get_str( modem, NV_MODEM_SMSC_ADDRESS, SMSC_ADDRESS);
  sms_address_from_str(&modem->smsc_address, tmp, strlen(tmp));

@@ +2528,5 @@
> +
> +        // Get tosca if possible
> +        const char *toa_pos = strchr(addr_end, ',');
> +        if (!toa_pos) {
> +            goto EndCommand;

No <tosca> passed here and we have already set SMSC address successfully above, you should return NULL instead.

@@ +2532,5 @@
> +            goto EndCommand;
> +        }
> +
> +        toa_pos++;
> +        modem->smsc_address.toa = (unsigned char)atoi(toa_pos);

or combined:

  char sca[32];
  int sca_len;
  SmsAddressRec addr;
  if (sms_address_from_str(&addr, sca, sca_len)) {
      goto BadCommand;
  }

  const char *toa_pos = strchr(addr_end, ',');
  if (toa_pos) {
      toa_pos++;
      toa = atoi(toa_pos);
      if (toa != addr.toa) {
          // 3GPP TS 27.005 section 3.2.5 "Message Service Failure Result Code"
          // - Invalid PDU mode parameter.
          return "+CMS ERROR: 304";
      }
  }

  amodem_set_smsc_address( modem, &addr);
  return NULL;

@@ +2535,5 @@
> +        toa_pos++;
> +        modem->smsc_address.toa = (unsigned char)atoi(toa_pos);
> +    }
> +EndCommand:
> +    return "";

Please return "+CMS ERROR: 304"

Comment 55

[Mounir Lamouri (:mounir)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #53)
> (In reply to Mounir Lamouri (:mounir) from comment #51)
> > Finally, I would have one last question: what's the use case for having
> > these methods? According to comment 0, this would be used by the Settings
> > App. Do we expect other apps to use it?
> 
> No, SMSC setting is mostly only for production tests.  Normal users usually
> don't have to modify it.

Do we expect developers to use this?

Comment 56

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Add support for SMSC command in emulator. V1.1

Comment 57

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Comment on attachment 755857 [details]
Add support for SMSC command in emulator. V1.1

Link to Pull Request

Comment 58

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Add support for SMSC command in ril

Link to Pull Request

Comment 59

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Mounir Lamouri (:mounir) from comment #55)
> (In reply to Vicamo Yang [:vicamo][:vyang] from comment #53)
> > (In reply to Mounir Lamouri (:mounir) from comment #51)
> > > Finally, I would have one last question: what's the use case for having
> > > these methods? According to comment 0, this would be used by the Settings
> > > App. Do we expect other apps to use it?
> > 
> > No, SMSC setting is mostly only for production tests.  Normal users usually
> > don't have to modify it.
> 
> Do we expect developers to use this?

Honestly, since the reporter doesn't want to defence the necessity here, I don't really think it's very important.

The either ways, Settings and DOM API, for setting SMSC address affects the whole system and I suspect any developer would ever want to write such app that may have different effects on different users because they may have SIM cards from different carriers.  Actually, I prefer having bug 736701 (TP-RP, Reply Path) implemented instead.  That's more likely to be used by a developer because he may want to send one single SMS message to a special SMSC without touching system preference.

Mounir, like comment 35, close this bug as WONTFIX is also an nice option for me.

Comment 60

[Vicamo Yang [:vicamo][:vyang]]

"[e-mail]" is a shared account for people in ZTE.  Since time proves setting needinfo to this account is useless, I'll skip every message sent by this address from now on.

Comment 61

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 755857 [details]
Add support for SMSC command in emulator. V1.1

r=me with comments on GitHub addressed. Thank you :)

Comment 62

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 754721 [details] [diff] [review]
0004. Get/Set SMSC address in mobilemessage. V1.1

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

::: dom/mobilemessage/src/android/SmsService.cpp
@@ +53,5 @@
>  
> +NS_IMETHODIMP
> +SmsService::GetSmscAddress(nsIMobileMessageCallback *aRequest)
> +{
> +  // We don't have corrsponding bridge function on android

Please have a TODO to bug 878016.

@@ +61,5 @@
> +NS_IMETHODIMP
> +SmsService::SetSmscAddress(const nsAString &aSmscAddress,
> +                           nsIMobileMessageCallback *aRequest)
> +{
> +  // We don't have corrsponding bridge function on android

ditto

Comment 63

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0004. Get/Set SMSC address in mobilemessage. V1.2

Address comment 62

Comment 64

[Vicamo Yang [:vicamo][:vyang]]

I'm going to close this bug as WONTFIX, but parts in hardware/ril and external/qemu are still appreciated and they do enables possibilities to perform some more tests on SMS.  Thank you :)

Merged on GitHub https://github.com/mozilla-b2g/platform_external_qemu/commit/080263703769e6194061802fb1cab53d89fa96b4
Merged on GitHub https://github.com/mozilla-b2g/platform_hardware_ril/commit/d73c84543e3c351ab00ed28994d8b35b92563382

Comment 67

[Vicamo Yang [:vicamo][:vyang]]

From bug 903938 comment 8, at least being able to show SMSC address.

Comment 68

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. IDL Change.

Rebase.

Comment 69

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. IPDL change to support get_set SMSC address.

Rebase.

Comment 70

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0003. Get_Set SMSC address in RIL.

Rebase.

Comment 71

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0004. Get_Set SMSC address in mobilemessage.

Rebase.

Comment 72

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: (WIP) 0005. Test case.

Test for set SMSC address API can't work due to emulator limitation.

Comment 73

[Vicamo Yang [:vicamo][:vyang]]

Julien, I would like to have confirmation from you and our PM.  Is getting SMSC address a feature we __really__ need?  Last time I resolved this bug as WONTFIX because no one has positive answer to this question, and I'm glad to resolve it with the same reason if there is still no strong input for this.

Comment 74

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 805125 [details] [diff] [review]
0001. IDL Change.

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

The request I have now is to have the ability to read SMSC address only.  Sorry, might take you a bit more time.  And I suggest you suspend the work until we have a confirmed need.

Comment 75

[Julien Wajsberg [:julienw]]

This was a request from Support, I'll need info Hermina who requested it, she will have more information.

Comment 76

[Hermina]

Hi,

From a support perspective it is critical to offer our partners all possible debug information via the settings app. When this issue appeared TEF was frustrated as they didn't even have a way to check if the SMSC address was written correctly in the customer's phone. In some cases is as simple as that. The address is incorrect and it can be fixed on the spot by their first level support by giving the customer step by step instructions on how to modify it.TEF's point was that this is a basic info they should be able to see for easy and fast debugging. Could you please advise? Thanks!

Comment 77

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. IDL Change.

Add API for getting SMSC address.

Comment 78

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. IPDL change to support get SMSC address.

Comment 79

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0003. Get SMSC address in RIL.

Comment 80

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0004. Get SMSC address in mobilemessage.

Comment 81

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0005. Test case.

marionette test for getting SMSC.

Comment 82

[Vicamo Yang [:vicamo][:vyang]]

(In reply to hcondei from comment #76)
> Hi,
> 
> From a support perspective it is critical to offer our partners all possible
> debug information via the settings app. When this issue appeared TEF was
> frustrated as they didn't even have a way to check if the SMSC address was
> written correctly in the customer's phone. In some cases is as simple as
> that. The address is incorrect and it can be fixed on the spot by their
> first level support by giving the customer step by step instructions on how
> to modify it.TEF's point was that this is a basic info they should be able
> to see for easy and fast debugging. Could you please advise? Thanks!

I'm sure you need some method to examine SMSC address in some cases, either mass production or online debug.  The thing we care most is whether such reason strong enough to promote this API a public, always available DOM entry.

I can still come out some other solutions that don't touch WebAPI:

  1) Provide an internal API and have a private chrome page to show
     SMSC address, a.k.a Engineering Mode.  I think we have already
     use this method for mass production in some partners.  The user
     have to perform some manufacturer defined steps to launch/trigger
     Engineering Mode app.

     TODO: have an API in nsISmsService.

  2) Define a manufacturer specific MMI code and return that SMSC
     address.  We have already SMSC address in ril_worker, so this
     takes only a few additional lines in the same file.  All that
     an user has to do is dial a pre-defined string in Dialer app.

     TODO: none in Mozilla side because that will be a manufacturer
     specific MMI code.

  3) Settings?  We're going to implement multisim for v1.3 now.
     Such thing would become a problem and I really want to avoid
     them.

ni Sicking for arbitration.

Comment 83

[Julien Wajsberg [:julienw]]

I think any of these possibilities will please the support team.

Comment 84

[Julien Wajsberg [:julienw]]

(clearing NI as Hermina answered my question earlier)

Comment 85

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I don't have a strong opinion here, so ultimately I think this is up to the RIL team.

However, the support usecase definitely sounds important, so we should figure out some way to support it.

I don't think the engineering mode ever got fully built. When we do built it, we should do so using public WebAPIs. I know we debated other solutions, but they were intended to be temporary hacks and not final solutions. The final solution should definitely use public WebAPIs (though probably limited to certified apps).

Either way it seems like haing something in the icc manager API could work. Leaving this up to Hsin-Yi as she's owning that API these days.

Comment 86

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

Sorry for the delay. I just discussed the possibilities with Vicamo.

(In reply to Jonas Sicking (:sicking) Please ni? if you want feedback from comment #85)
> I don't have a strong opinion here, so ultimately I think this is up to the
> RIL team.
> 
> However, the support usecase definitely sounds important, so we should
> figure out some way to support it.
> 
> I don't think the engineering mode ever got fully built. When we do built
> it, we should do so using public WebAPIs.

Yup, I also agree with that so engineering mode isn't a good solution to this use case.

Considering the fact of the asyn operation in getting SMSC and possible multisim scenario, using Settings isn't a good choice either as we learned a lot from previous experiences of abusing Settings API. 

Though the getting/setting SMSC function needs to be restricted to only certified apps due to the fact of the real use case, using public WebAPIs sounds reasonable.


> I know we debated other solutions,
> but they were intended to be temporary hacks and not final solutions. The
> final solution should definitely use public WebAPIs (though probably limited
> to certified apps).
>
> Either way it seems like haing something in the icc manager API could work.
> Leaving this up to Hsin-Yi as she's owning that API these days.

Comment 87

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 809036 [details] [diff] [review]
0002. IPDL change to support get SMSC address.

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

::: dom/mobilemessage/src/MobileMessageCallback.cpp
@@ +212,5 @@
> +{
> +  AutoJSContext cx;
> +  JSString *smsc = JS_NewUCStringCopyN(cx,
> +                                       static_cast<const jschar *>(aSmscAddress.BeginReading()),
> +                                       aSmscAddress.Length());

JSString* smsc = JS_NewUCStringCopyZ(cx, aSmscAddress.get());

Comment 88

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 809039 [details] [diff] [review]
0003. Get SMSC address in RIL.

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +2749,5 @@
>    },
>  
> +  getSmscAddress: function getSmscAddress(request) {
> +    this.workerMessenger.send("getSmscAddress",
> +                              {},

nit: you can place a 'null' here.

::: dom/system/gonk/ril_worker.js
@@ -283,5 @@
>      this.IMEI = null;
>      this.IMEISV = null;
>      this.ESN = null;
>      this.MEID = null;
> -    this.SMSC = null;

Please still allow SMSC string caching.

Comment 89

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. IDL Change.

Rebase.

Comment 90

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. IPDL change to support get SMSC address.

Rebase.

Comment 91

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0003. Get SMSC address in RIL. V2

Address comment 88.

Comment 92

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0004. Get SMSC address in mobilemessage.

Rebase.

Comment 93

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0005. Test case.

Rebase.

Comment 94

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try for this patch set : https://tbpl.mozilla.org/?tree=Try&rev=c1745acf24b4

Comment 95

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 824495 [details] [diff] [review]
0003. Get SMSC address in RIL. V2

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

::: dom/system/gonk/ril_worker.js
@@ +1793,5 @@
>    /**
>     * Get the Short Message Service Center address.
>     */
> +  getSmscAddress: function getSmscAddress(options) {
> +    Buf.simpleRequest(REQUEST_GET_SMSC_ADDRESS, options);

Sorry, I should have had a more clear comment.  We can save some traffic between rild by checking validity of |this.SMSC| here before invocation of a new parcel transaction.  So I'm expecting something like:

  getSmscAddress: function getSmscAddress(options) {
    if (this.SMSC) {
      options.smscAddress = this.SMSC;
      options.errorMsg = RIL_ERROR_TO_GECKO_ERROR[ERROR_SUCCESS];
      this.sendChromeMessage(options);
      return;
    }

    Buf.simpleRequest(REQUEST_GET_SMSC_ADDRESS, options);
  },

@@ -3330,5 @@
>      let rs = this.voiceRegistrationState;
>      let stateChanged = this._processCREG(rs, state);
> -    if (stateChanged && rs.connected) {
> -      RIL.getSMSCAddress();
> -    }

Please keep it.

@@ +5908,5 @@
> +    rilMessageType: options.rilMessageType,
> +    rilMessageToken: options.rilMessageToken,
> +    smscAddress: this.SMSC,
> +    errorMsg: options.rilRequestError
> +  });

Just do:

  this.SMSC = Buf.readString();

  options.smscAddress = this.SMSC;
  options.errorMsg = RIL_ERROR_TO_GECKO_ERROR[options.rilRequestError];
  this.sendChromeMessage(options);

Comment 96

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 824497 [details] [diff] [review]
0005. Test case.

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

::: dom/mobilemessage/tests/marionette/test_smsc_address.js
@@ +2,5 @@
> + * http://creativecommons.org/publicdomain/zero/1.0/ */
> +
> +MARIONETTE_TIMEOUT = 60000;
> +
> +SpecialPowers.setBoolPref("dom.sms.enabled", true);

Don't set "dom.sms.enabled" from now on.  See bug 915604.

@@ +26,5 @@
> +  let cmd = "sms smsc " + smsc;
> +  runEmulatorCmd(cmd, function (result) {
> +    --pendingEmulatorCmdCount;
> +
> +    is(result[0], "OK", "Failed to set SMSC to Emulator");

"Emulator response".  The third field is the description or name of the first field.  See http://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-simpletest.js#40

@@ +38,5 @@
> +  runEmulatorCmd(cmd, function (result) {
> +    --pendingEmulatorCmdCount;
> +
> +    is(result[1], "OK", "Failed to check SMSC on Emulator");
> +    is(result[0], formatSmscAddress(smsc), "SMSC on emulator doesn't match");

ditto

@@ +86,5 @@
> +     "manager is instance of " + manager.constructor);
> +
> +  // Initialize SMSC address of emulator
> +  setEmulatorSmsc(SMSC);
> +  waitFor(taskNextWrapper, function() {

|tasks.next.bind(tasks)|, so you don't need taskNextWrapper.

@@ +94,5 @@
> +
> +tasks.push(function readSmscAddress() {
> +  log("read SMSC address");
> +
> +  let req = manager.getSmscAddress();

This probably either has racing condition or fails always because of SMSC caching.  Can't we just check current SMSC address equals to "[phone]"?  See https://github.com/mozilla-b2g/platform_external_qemu/blob/master/telephony/android_modem.c#L84

@@ +95,5 @@
> +tasks.push(function readSmscAddress() {
> +  log("read SMSC address");
> +
> +  let req = manager.getSmscAddress();
> +  ok(req, "readSmscAddress(): can't get returned req");

The second argument of |ok| is again the name of the first field, so it shouldn't be error messages like this.

@@ +118,5 @@
> +
> +  manager.onreceived = null;
> +  SpecialPowers.removePermission("sms", document);
> +  SpecialPowers.setBoolPref("dom.sms.enabled", false);
> +  log("Finish!!!");

doesn't look like an important log message.  Please remove it.

Comment 97

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0003. Get SMSC address in RIL. V3

Address comment 95.

Comment 98

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0005. Test case. V2

Address comment 96.

Comment 99

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try : https://tbpl.mozilla.org/?tree=Try&rev=54838f2f779d

Comment 100

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 825774 [details] [diff] [review]
0005. Test case. V2

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

Thank you :)

Comment 101

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0003. Get SMSC address in RIL. V4

Only update SMSC but don't send message to DOM when |getSmscAddress()| is called with incorrect options format.

Comment 102

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0003. Get SMSC address in RIL. V5

Apply some code refactory suggested by vicamo.

Comment 103

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 825824 [details] [diff] [review]
0003. Get SMSC address in RIL. V5

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

Thank you :)

Comment 104

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try : https://tbpl.mozilla.org/?tree=Try&rev=58c9aa183c0c (I don't know why it mixed with other's patch)

Comment 105

[Vicamo Yang [:vicamo][:vyang]]

Hi Chuck, these patches have to be rebased on bug 854326 and bug 921919.

Comment 106

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. IDL Change.

Rebase.

Comment 107

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. IPDL change to support get SMSC address.

Rebase.

Comment 108

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0003. Get SMSC address in RIL. V5

Rebase.

Comment 109

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0004. Get SMSC address in mobilemessage.

Rebase.

Comment 110

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0005. Test case. V2

Rebase.

Comment 111

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try on inbound : https://tbpl.mozilla.org/?tree=Try&rev=686144079d89

Comment 112

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

It need more than rebase.

Comment 113

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. IDL Change. V2

Support multi-sim.

Comment 114

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0002. IPDL change to support get SMSC address. V2

Support multi-sim.

Comment 115

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0004. Get SMSC address in mobilemessage. V2

Support multi-sim.

Comment 116

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try for patch set : https://tbpl.mozilla.org/?tree=Try&rev=b557c266b0ef

Comment 117

[Vicamo Yang [:vicamo][:vyang]]

We don't have OOP test cases yet, so please have your patches applied to emulator/device along with some hook code in Gaia to actually verify it again within OOP environment.

Comment 118

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #117)
> We don't have OOP test cases yet, so please have your patches applied to
> emulator/device along with some hook code in Gaia to actually verify it
> again within OOP environment.

I have verified that the patch works on unagi.

Comment 119

[Vicamo Yang [:vicamo][:vyang]]

Attachment: 0002. IPDL change to support get SMSC address. V3

Rebase after bug 927711, which is now in b2g-inbound only.

Comment 120

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/b2g-inbound/rev/77d3d0fc48fe
https://hg.mozilla.org/integration/b2g-inbound/rev/60e2f255b0bd
https://hg.mozilla.org/integration/b2g-inbound/rev/94f9ee1c5007
https://hg.mozilla.org/integration/b2g-inbound/rev/515e1593c2df
https://hg.mozilla.org/integration/b2g-inbound/rev/ca77b56117ea

Comment 121

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/77d3d0fc48fe
https://hg.mozilla.org/mozilla-central/rev/60e2f255b0bd
https://hg.mozilla.org/mozilla-central/rev/94f9ee1c5007
https://hg.mozilla.org/mozilla-central/rev/515e1593c2df
https://hg.mozilla.org/mozilla-central/rev/ca77b56117ea

Comment 122

[Julien Wajsberg [:julienw]]

To make it clear for everybody, could you please explain what the final behaviour of this patch is?

Comment 123

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Julien Wajsberg [:julienw] from comment #122)
> To make it clear for everybody, could you please explain what the final
> behaviour of this patch is?

maybe a sample code could do the job?

var request = window.navigator.mozMobileMessage.getSmscAddress();
request.onsuccess = function() {
  var smsc = this.result;
}


Also |getSmscAddress()| supports multi-sim and takes service ID as optional parameter to select SIM card, and default SIM card is selected if no service ID is provided.

Comment 124

[Julien Wajsberg [:julienw]]

Ok so now we need a Settings bug to show it there!