810091 - B2G MMS: don't download message twice on receiving duplicated notification

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

As suggested in the summary, we should check whether there are already a message retrieved/retrieving with the same transaction id before starting the retrieval. See bug 810067 as well.

Comment 1

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.0

Comment 2

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.1

Comment 3

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.2

Comment 4

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.3

Comment 5

[Gene Lian (I already quit Mozilla)]

Comment on attachment 728892 [details] [diff] [review]
Patch v1.3

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

Need to review again when the following fix is included.

::: dom/mobilemessage/src/ril/MobileMessageDatabaseService.js
@@ +1267,5 @@
>      if (aMessage.type == "sms") {
>        addresses = [aMessage.receiver];
>      } else if (aMessage.type == "mms") {
>        addresses = aMessage.receivers;
> +      aMessage.transactionIdIndex = aMessage.headers["x-mms-transaction-id"];

This is not quite correct. We didn't prepare ["x-mms-transaction-id"] yet when calling .saveSendingMessage(). We should do something like below in the MmsService.js:

let savableMessage = this.createSavableFromParams(aParams);
let sendTransaction;
try {
  sendTransaction = new SendTransaction(savableMessage);
} catch (e) {
  aRequest.notifySendMmsMessageFailed(...);
  return;
}
gMobileMessageDatabaseService
  .saveSendingMessage(sendTransaction.msg, ...);

Comment 6

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.4

Comment 7

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.5

Comment 8

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.6

Comment 9

[Gene Lian (I already quit Mozilla)]

Comment on attachment 729363 [details] [diff] [review]
Patch v1.6

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

::: dom/mms/src/ril/MmsService.js
@@ +1040,5 @@
>     *
>     * @param notification
>     *        The parsed MMS message object.
>     */
>    handleNotificationIndication: function handleNotificationIndication(notification) {

Sorry. Bug 854422 lands first. You have to rebase again. ;P

@@ +1049,5 @@
> +        (function (aRv, aMessageRecord) {
> +      if (Ci.nsIMobileMessageCallback.SUCCESS_NO_ERROR === aRv
> +          && aMessageRecord) {
> +        debug("We already get the NotificationIndication = "
> +              + transactionId + " before.");

debug("We already got the NotificationIndication with transactionId = "

@@ +1301,5 @@
>          Services.obs.notifyObservers(aDomMessage, kMmsSentObserverTopic, null);
>        });
>      };
>  
>      let savableMessage = this.createSavableFromParams(aParams);

s/createSavableFromParams/createSendableFromParams
s/savableMessage/sendableMessage

and move the following lines

message["type"] = "mms";
message["deliveryStatusRequested"] = true;
message["timestamp"] = Date.now();
message["receivers"] = receivers;

to another new function .ceateSavableFromSendable(aSendable).

@@ +1304,5 @@
>  
>      let savableMessage = this.createSavableFromParams(aParams);
> +    let sendTransaction;
> +    try {
> +      sendTransaction = new SendTransaction(savableMessage);

s/savableMessage/sendableMessage

@@ +1307,5 @@
> +    try {
> +      sendTransaction = new SendTransaction(savableMessage);
> +    } catch (e) {
> +      debug("Exception: fail to create a SendTransaction instance.");
> +      sendTransactionCb(aDomMessage.id, false);

We cannot call this because .sendTransactionCb() calls .setMessageDelivery() internally, which cannot find a saved record. I think we should call directly call:

aRequest.notifySendMmsMessageFailed(Ci.nsIMobileMessageCallback.INTERNAL_ERROR);

@@ +1309,5 @@
> +    } catch (e) {
> +      debug("Exception: fail to create a SendTransaction instance.");
> +      sendTransactionCb(aDomMessage.id, false);
> +      return;
> +    }

Call:

let savableMessage = this.ceateSavableFromSendable(sendTransaction.msg);

or

let savableMessage = this.ceateSavableFromSendable(sendableMessage);

@@ +1310,5 @@
> +      debug("Exception: fail to create a SendTransaction instance.");
> +      sendTransactionCb(aDomMessage.id, false);
> +      return;
> +    }
> +    gMobileMessageDatabaseService.saveSendingMessage(sendTransaction.msg,

s/sendTransaction.msg/savableMessage

::: dom/mobilemessage/interfaces/nsIRilMobileMessageDatabaseService.idl
@@ +83,5 @@
>    void getMessageRecordById(in long aMessageId,
>                              in nsIRilMobileMessageDatabaseRecordCallback aCallback);
> +
> +  /**
> +   * |aTransactionId| DOMString: the transaction id of mms pdu.

s/mms/MMS

::: dom/mobilemessage/src/ril/MobileMessageDatabaseService.js
@@ +1176,5 @@
>        // TODO Bug 853384 - for some SIMs we cannot retrieve the vaild
>        // phone number, thus setting the SMS' receiver to be null.
>        aMessage.receiver = self;
>      } else if (aMessage.type == "mms") {
> +      aMessage.transactionIdIndex = aMessage.headers["x-mms-transaction-id"];

I'd prefer adding the following assignment in the .convertIntermediateToSavable(...):

intermediate.transactionId = intermediate.headers["x-mms-transaction-id"];

And add a sanity check to ensure the existence of |aMessage.transactionId| at the head of .saveReceivedMessage(). Also, update the comment in the .idl.

Also, I'd prefer moving this assignment to further below:

aMessage.deliveryIndex = [DELIVERY_RECEIVED, timestamp];
aMessage.readIndex = [FILTER_READ_UNREAD, timestamp];
if (aMessage.type == "mms") {
  aMessage.transactionIdIndex = aMessage.transactionId;
}

where deal with all the index assignment stuff.

@@ +1267,5 @@
>      if (aMessage.type == "sms") {
>        addresses = [aMessage.receiver];
>      } else if (aMessage.type == "mms") {
>        addresses = aMessage.receivers;
> +      aMessage.transactionIdIndex = aMessage.headers["x-mms-transaction-id"];

I'd prefer adding the following assignment in the .ceateSavableFromSendable(aSavable):

message["transactionId"] = aSavable.headers["x-mms-transaction-id"];

Add a sanity check for |aMessage.transactionId| at the head of .saveSendingmessage(). Also, update the comment in the .idl.

Also move this assignment to:

// Adding needed indexes and extra attributes for internal use.
// threadIdIndex & participantIdsIndex are filled in saveRecord().
...
if (aMessage.type == "mms") {
  aMessage.transactionIdIndex = aMessage.transactionId;
}

@@ +1374,5 @@
>          messageStore.put(messageRecord);
>        };
>      });
>    },
> +  getMessageRecordByTransactionId: function getMessageRecordByTransactionId(aTransactionId, aCallback) {

New line above this function.

Comment 10

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.7

Comment 11

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.8

Revised accroding to Gene's comment.

Comment 12

[Gene Lian (I already quit Mozilla)]

Comment on attachment 730034 [details] [diff] [review]
Patch v1.8

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

::: dom/mms/src/ril/MmsService.js
@@ +1180,5 @@
>    },
>  
>    /**
>     * A utility function to convert the MmsParameters dictionary object
> +   * to a sendable message.

sendable intermediate object.

@@ +1267,5 @@
> +  },
> +
> +  /**
> +   * A utility function to convert the sendable intermediate object
> +   * to a sendable intermediate object.

s/sendable/savable

@@ +1277,2 @@
>      // The following attributes are needed for saving message into DB.
> +    let saveable = aSendable;

s/saveable/savable and below.

::: dom/mobilemessage/interfaces/nsIRilMobileMessageDatabaseService.idl
@@ +39,5 @@
>     *   - If |type| == "mms", we also need:
>     *     - |delivery| DOMString: the delivery state of received message
>     *     - |deliveryStatus| DOMString Array: the delivery status of received message
>     *     - |receivers| DOMString Array: the phone numbers of receivers
> +   *     - |transactionId| DOMString: the transaction id from pdu header.

the transaction ID from MMS PDU header.

@@ +58,5 @@
>     *     - |receiver| DOMString: the phone number of receiver
>     *
>     *   - If |type| == "mms", we also need:
>     *     - |receivers| DOMString Array: the phone numbers of receivers
> +   *     - |transactionId| DOMString: the transaction id from pdu header.

Ditto.

@@ +85,5 @@
>    void getMessageRecordById(in long aMessageId,
>                              in nsIRilMobileMessageDatabaseRecordCallback aCallback);
> +
> +  /**
> +   * |aTransactionId| DOMString: the transaction id of MMS pdu.

the transaction ID of MMS PDU.

::: dom/mobilemessage/src/ril/MobileMessageDatabaseService.js
@@ +562,5 @@
>        });
>      };
>    },
>  
> +  // Add index of transaction id for MMS.

NIT: please use the following format for a function comment.

/**
 * Add index of transaction id for MMS.
 */

@@ +1162,5 @@
>          (aMessage.type == "sms" && aMessage.messageClass == undefined) ||
>          (aMessage.type == "mms" && (aMessage.delivery == undefined ||
>                                      !Array.isArray(aMessage.deliveryStatus) ||
>                                      !Array.isArray(aMessage.receivers))) ||
> +        (aMessage.type == "mms" && aMessage.transactionId == undefined) ||

Please combine this condition with the above one.

@@ +1220,5 @@
>    saveSendingMessage: function saveSendingMessage(aMessage, aCallback) {
>      if ((aMessage.type != "sms" && aMessage.type != "mms") ||
>          (aMessage.type == "sms" && !aMessage.receiver) ||
>          (aMessage.type == "mms" && !Array.isArray(aMessage.receivers)) ||
> +        (aMessage.type == "mms" && aMessage.transactionId == undefined) ||

Ditto.

@@ +1270,5 @@
>      if (aMessage.type == "sms") {
>        addresses = [aMessage.receiver];
>      } else if (aMessage.type == "mms") {
>        addresses = aMessage.receivers;
> +      aMessage.transactionIdIndex = aMessage.headers["x-mms-transaction-id"];

aMessage.transactionIdIndex = aMessage.transactionId;

@@ +1403,5 @@
> +      request.onerror = function onerror(event) {
> +        if (DEBUG) {
> +          if (event.target)
> +            debug("Caught error on transaction", event.target.errorCode);
> +          aCallback.notify(Ci.nsIMobileMessageCallback.INTERNAL_ERROR, null);

Why is the line in the |if (DEBUG) {}| block?

Comment 13

[Gene Lian (I already quit Mozilla)]

Btw, we still need Vicamo's final review since I'm not a formal RIL reviewer.

Comment 14

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.9

Comment 15

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

(In reply to Chia-hung Tai [:ctai :ctai_mozilla :cht] from comment #14)
> Created attachment 730574 [details] [diff] [review]
> Patch v1.9

Modified according to Gene's comment.

Comment 16

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

(In reply to Gene Lian [:gene] from comment #12)
> Comment on attachment 730034 [details] [diff] [review]
> Patch v1.8
> 
> Review of attachment 730034 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/mms/src/ril/MmsService.js
> @@ +1180,5 @@
> >    },
> >  
> >    /**
> >     * A utility function to convert the MmsParameters dictionary object
> > +   * to a sendable message.
> 
> sendable intermediate object.
> 
> @@ +1267,5 @@
> > +  },
> > +
> > +  /**
> > +   * A utility function to convert the sendable intermediate object
> > +   * to a sendable intermediate object.
> 
> s/sendable/savable
> 
> @@ +1277,2 @@
> >      // The following attributes are needed for saving message into DB.
> > +    let saveable = aSendable;
> 
> s/saveable/savable and below.
> 
> ::: dom/mobilemessage/interfaces/nsIRilMobileMessageDatabaseService.idl
> @@ +39,5 @@
> >     *   - If |type| == "mms", we also need:
> >     *     - |delivery| DOMString: the delivery state of received message
> >     *     - |deliveryStatus| DOMString Array: the delivery status of received message
> >     *     - |receivers| DOMString Array: the phone numbers of receivers
> > +   *     - |transactionId| DOMString: the transaction id from pdu header.
> 
> the transaction ID from MMS PDU header.
> 
> @@ +58,5 @@
> >     *     - |receiver| DOMString: the phone number of receiver
> >     *
> >     *   - If |type| == "mms", we also need:
> >     *     - |receivers| DOMString Array: the phone numbers of receivers
> > +   *     - |transactionId| DOMString: the transaction id from pdu header.
> 
> Ditto.
> 
> @@ +85,5 @@
> >    void getMessageRecordById(in long aMessageId,
> >                              in nsIRilMobileMessageDatabaseRecordCallback aCallback);
> > +
> > +  /**
> > +   * |aTransactionId| DOMString: the transaction id of MMS pdu.
> 
> the transaction ID of MMS PDU.
> 
> ::: dom/mobilemessage/src/ril/MobileMessageDatabaseService.js
> @@ +562,5 @@
> >        });
> >      };
> >    },
> >  
> > +  // Add index of transaction id for MMS.
> 
> NIT: please use the following format for a function comment.
> 
> /**
>  * Add index of transaction id for MMS.
>  */
> 
> @@ +1162,5 @@
> >          (aMessage.type == "sms" && aMessage.messageClass == undefined) ||
> >          (aMessage.type == "mms" && (aMessage.delivery == undefined ||
> >                                      !Array.isArray(aMessage.deliveryStatus) ||
> >                                      !Array.isArray(aMessage.receivers))) ||
> > +        (aMessage.type == "mms" && aMessage.transactionId == undefined) ||
> 
> Please combine this condition with the above one.
> 
> @@ +1220,5 @@
> >    saveSendingMessage: function saveSendingMessage(aMessage, aCallback) {
> >      if ((aMessage.type != "sms" && aMessage.type != "mms") ||
> >          (aMessage.type == "sms" && !aMessage.receiver) ||
> >          (aMessage.type == "mms" && !Array.isArray(aMessage.receivers)) ||
> > +        (aMessage.type == "mms" && aMessage.transactionId == undefined) ||
> 
> Ditto.
> 
> @@ +1270,5 @@
> >      if (aMessage.type == "sms") {
> >        addresses = [aMessage.receiver];
> >      } else if (aMessage.type == "mms") {
> >        addresses = aMessage.receivers;
> > +      aMessage.transactionIdIndex = aMessage.headers["x-mms-transaction-id"];
> 
> aMessage.transactionIdIndex = aMessage.transactionId;
> 
> @@ +1403,5 @@
> > +      request.onerror = function onerror(event) {
> > +        if (DEBUG) {
> > +          if (event.target)
> > +            debug("Caught error on transaction", event.target.errorCode);
> > +          aCallback.notify(Ci.nsIMobileMessageCallback.INTERNAL_ERROR, null);
> 
> Why is the line in the |if (DEBUG) {}| block?

Thanks. :D

Comment 17

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

This is mms conformance feature. Nominate for leo+.

Comment 18

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

Comment on attachment 730574 [details] [diff] [review]
Patch v1.9

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

::: dom/mms/src/ril/MmsService.js
@@ +1047,4 @@
>  
>      let transactionId = notification.headers["x-mms-transaction-id"];
> +    gMobileMessageDatabaseService.getMessageRecordByTransactionId(transactionId,
> +        (function (aRv, aMessageRecord) {

Please tear down this long long function into small pieces. It's getting difficult to read, let alone review it :(

@@ +1272,5 @@
> +   *
> +   * @param aSendable
> +   *        The sendable intermediate object.
> +   */
> +  ceateSavableFromSendable : function ceateSavableFromSendable(aSendable) {

nit: c'r'eateSavableFromSendable

@@ +1283,5 @@
> +    let receivers = savable["receivers"] = [];
> +    for (let idx in headersTo) {
> +      receivers.push(headersTo[idx].address);
> +    }
> +    savable["transactionId"] = aSendable.headers["x-mms-transaction-id"];

Transaction id is mandatory for M-Send.req, but not really meaningful in this bug.

::: dom/mobilemessage/interfaces/nsIRilMobileMessageDatabaseService.idl
@@ +58,5 @@
>     *     - |receiver| DOMString: the phone number of receiver
>     *
>     *   - If |type| == "mms", we also need:
>     *     - |receivers| DOMString Array: the phone numbers of receivers
> +   *     - |transactionId| DOMString: the transaction ID from MMS pdu header.

Ditto

::: dom/mobilemessage/src/ril/MobileMessageDatabaseService.js
@@ +200,5 @@
>              if (DEBUG) debug("Upgrade to version 8. Add participant/thread stores.");
>              self.upgradeSchema7(db, event.target.transaction);
>              break;
> +          case 8:
> +            if (DEBUG) debug("Upgrade to version 9. Add index of transaction id for MMS.");

for incoming MMS.

@@ +589,5 @@
> +        cursor.continue();
> +      }
> +      messageRecord.transactionIdIndex = messageRecord.transactionId;
> +      cursor.update(messageRecord);
> +      cursor.continue();

What about:

  if (is incoming mms) {
    update transaction index.
  }
  continue();

Because you missed a return statement after line 589 and transaction id is only meaningful when it's an incoming MMS.

@@ +1222,5 @@
>    saveSendingMessage: function saveSendingMessage(aMessage, aCallback) {
>      if ((aMessage.type != "sms" && aMessage.type != "mms") ||
>          (aMessage.type == "sms" && !aMessage.receiver) ||
> +        (aMessage.type == "mms" && (!Array.isArray(aMessage.receivers) ||
> +                                    aMessage.transactionId == undefined)) ||

Ditto.

@@ +1272,5 @@
>      if (aMessage.type == "sms") {
>        addresses = [aMessage.receiver];
>      } else if (aMessage.type == "mms") {
>        addresses = aMessage.receivers;
> +      aMessage.transactionIdIndex = aMessage.transactionId;

Ditto.

@@ +1390,5 @@
> +        return;
> +      }
> +      let request = messageStore.index("transactionId").get(aTransactionId);
> +
> +      request.onsuccess = function onsuccess(event) {

txn.oncomplete to shorten database transaction period.

@@ +1401,5 @@
> +        }
> +        aCallback.notify(Ci.nsIMobileMessageCallback.SUCCESS_NO_ERROR, messageRecord);
> +      };
> +
> +      request.onerror = function onerror(event) {

txn.onerror

Comment 19

[Alex Keybl [:akeybl]]

Mandatory MMS conformance bug, blocking.

Comment 20

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.10

Comment 21

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.11

Comment 22

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.12

Comment 23

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

(In reply to Chia-hung Tai [:ctai :ctai_mozilla :cht] from comment #22)
> Created attachment 735658 [details] [diff] [review]
> Patch v1.12
Modifiied according to comment 18.

Comment 24

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

Comment on attachment 735658 [details] [diff] [review]
Patch v1.12

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

::: dom/mms/src/ril/MmsService.js
@@ +1093,5 @@
> +                                      reportAllowed);
> +      transaction.run();
> +
> +      if (!success) {
> +        // At this point we could send a message to content to

nit: align to 80 char edge.

@@ +1106,5 @@
> +      // Broadcasting an 'sms-received' system message to open apps.
> +      this.broadcastMmsSystemMessage("sms-received", domMessage);
> +
> +      // Notifying observers an MMS message is received.
> +      Services.obs.notifyObservers(domMessage, kMmsReceivedObserverTopic, null);

Please extract these four lines into another utility function.

@@ +1167,5 @@
> +
> +    // For RETRIEVAL_MODE_AUTOMATIC or RETRIEVAL_MODE_AUTOMATIC_HOME but not
> +    // roaming, proceed to retrieve MMS.
> +    this.retrieveMessage(url, this.retrieveMessageCallback.bind(this, wish, savableMessage));
> +  },

Empty line, please.

::: dom/mobilemessage/src/ril/MobileMessageDatabaseService.js
@@ +588,5 @@
> +      let messageRecord = cursor.value;
> +      if ("mms" == messageRecord.type &&
> +          (DELIVERY_NOT_DOWNLOADED == messageRecord.delivery ||
> +           DELIVERY_RECEIVED == messageRecord.delivery)) {
> +        messageRecord.transactionIdIndex = messageRecord.transactionId;

At the time we land this patch, every mms record has no |transactionId| field, doesn't it? You have to fetch it from |messageRecord.headers["x-mms-transaction-id"]|.

@@ +1047,5 @@
>      aMessage.deliveryIndex = [DELIVERY_SENDING, timestamp];
>      aMessage.readIndex = [FILTER_READ_READ, timestamp];
>      aMessage.delivery = DELIVERY_SENDING;
>      aMessage.messageClass = MESSAGE_CLASS_NORMAL;
>      aMessage.read = FILTER_READ_READ;

Please keep this empty line.

Comment 25

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.13

Revised the patch for comment 24.

Comment 26

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.14

Changed this to self in |send|.

Comment 27

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

Comment on attachment 736100 [details] [diff] [review]
Patch v1.14

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

::: dom/mms/src/ril/MmsService.js
@@ +1057,5 @@
> +   *         The MMS observer topic.
> +   * @params aDomMessage
> +   *         The nsIDOMMozMmsMessage object.
> +   */
> +  broadcastSysMagAndNotifyObservers: function broadcastSysMagAndNotifyObservers(aName, aObserverTopic, aDomMessage) {

Please have two functions broadcast{Sent,Received}MessageEvent() that takes one parameter |aDomMessage| only.

::: dom/mobilemessage/src/ril/MobileMessageDatabaseService.js
@@ +201,5 @@
>              if (DEBUG) debug("Upgrade to version 8. Add participant/thread stores.");
>              self.upgradeSchema7(db, event.target.transaction);
>              break;
> +          case 8:
> +            if (DEBUG) debug("Upgrade to version 9. Add index of transaction id for incoming MMS.");

nit: Add transactionId index for ...

@@ +564,5 @@
>      };
>    },
>  
> +  /**
> +   * Add index of transaction id for MMS.

ditto.

@@ +588,5 @@
> +      let messageRecord = cursor.value;
> +      if ("mms" == messageRecord.type &&
> +          (DELIVERY_NOT_DOWNLOADED == messageRecord.delivery ||
> +           DELIVERY_RECEIVED == messageRecord.delivery)) {
> +        messageRecord.transactionIdIndex = messageRecord.headers["x-mms-transaction-id"];

Line wrap

Comment 28

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.15

Rebased and revised.

Comment 29

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Patch v1.16

Clean space and some mistakes.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0614d7cadd76

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0614d7cadd76

Comment 32

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

Uplift WIP: https://github.com/vicamo/b2g_mozilla-central/tree/bugzilla/838467/b2g18 , verifying.

Comment 33

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

https://hg.mozilla.org/releases/mozilla-b2g18/rev/addd4bc121bd

Comment 34

[Gene Lian (I already quit Mozilla)]

Comment on attachment 736707 [details] [diff] [review]
Patch v1.16

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

::: dom/mms/src/ril/MmsService.js
@@ +1351,5 @@
>            Services.obs.notifyObservers(aDomMessage, kSmsFailedObserverTopic, null);
>            return;
>          }
>  
> +        self.broadcastSentMessageEvent(domMessage);

s/domMessage/aDomMessage

I'm really surprised. This is impossible to work... How could we send an MMS with this typo? No one has been pointing this out since it was checked in on 4/17...

Will fire a bug for this later.