B2G MMS: support sending M-Send.req PDUs

RESOLVED FIXED in mozilla17

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: vicamo, Assigned: vicamo)

Tracking

(Blocks: 1 bug)

unspecified
mozilla17
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-basecamp:-)

Details

Attachments

(4 attachments, 7 obsolete attachments)

9.96 KB, patch
Details | Diff | Splinter Review
87.49 KB, patch
Details | Diff | Splinter Review
10.09 KB, patch
Details | Diff | Splinter Review
2.85 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
This is the protocol part of MMS outgoing messages support. It should open an API that accepts a pre-contructed message object, encode it into a M-Send.req PDU, send it out and process returning M-Send.conf message as well.
(Assignee)

Updated

5 years ago
Assignee: nobody → vyang

Updated

5 years ago
blocking-basecamp: --- → ?

Comment 1

5 years ago
MMS is desired to be ready for basecamp, but does not actually block.
blocking-basecamp: ? → -
(Assignee)

Updated

5 years ago
Depends on: 775038
(Assignee)

Comment 2

5 years ago
Created attachment 643324 [details] [diff] [review]
Part 1: Add required encoders for M-Send.req/M-Send.conf PDU : WIP
(Assignee)

Comment 3

5 years ago
Created attachment 643325 [details] [diff] [review]
Part 2: send M-Send.req : WIP
(Assignee)

Comment 4

5 years ago
Created attachment 643326 [details] [diff] [review]
[Debug] DO NOT SUBMIT
(Assignee)

Comment 5

5 years ago
Created attachment 645708 [details] [diff] [review]
Part 1: disable some v1.4 parameters for compatibility
Attachment #645708 - Flags: review?(philipp)
(Assignee)

Comment 6

5 years ago
Created attachment 645715 [details] [diff] [review]
Part 2: Add required encoders for M-Send.req/M-Send.conf PDU

1. support email addressing model, see bug 761069
2. refactor MmsPduHelper.encodeHeader* and add test cases
3. decoded msg.content is an array, so refactor encoder to accept the same type
4. do not use magic pdu type numbers in test script
Attachment #643324 - Attachment is obsolete: true
Attachment #645715 - Flags: review?(philipp)
(Assignee)

Updated

5 years ago
Attachment #645715 - Attachment description: Part 1: Add required encoders for M-Send.req/M-Send.conf PDU → Part 2: Add required encoders for M-Send.req/M-Send.conf PDU
(Assignee)

Comment 7

5 years ago
Created attachment 645716 [details] [diff] [review]
Part 3: send M-Send.req

use msg.content rather than msg.contentStream
Attachment #643325 - Attachment is obsolete: true
Attachment #645716 - Flags: review?(philipp)
(Assignee)

Comment 8

5 years ago
Created attachment 645718 [details] [diff] [review]
[Debug] DO NOT SUBMIT : V2

updates for changes made in patch 2, 3
Attachment #643326 - Attachment is obsolete: true
Attachment #645708 - Flags: review?(philipp) → review+
Comment on attachment 645715 [details] [diff] [review]
Part 2: Add required encoders for M-Send.req/M-Send.conf PDU

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

::: dom/mms/src/ril/MmsPduHelper.jsm
@@ +115,5 @@
> +      throw new WSP.CodeError("Address: invalid value");
> +    }
> +
> +
> +    let [type, addr] = [value.type, value.address];

Sadly I think this creates at least one array object which seems a bit excessive just to save one line of code.

@@ +117,5 @@
> +
> +
> +    let [type, addr] = [value.type, value.address];
> +    let str;
> +    if (type == "email") {

It seems like a `switch` statement would fit better here.

@@ +122,5 @@
> +      if (addr.indexOf("@") > 0) {
> +        str = addr;
> +      }
> +    } else if (type == "num") {
> +      if (addr.match(/^[\+*#]\d+$/)) {

Please define these RegExps globally. You can `const` them or make them lazy getters on the global scope, e.g.:

  function defineLazyRegExp(obj, name, pattern) {
    obj.__defineGetter__(name, function() {
      delete obj[name];
      return obj[name] = new RegExp(pattern);
    });
  }

  defineLazyRegExp(this, "MMS_ADDR_TYPE_NUM", "^[\+*#]\d+$");
  defineLazyRegExp(this, "MMS_ADDR_TYPE_ALPHANUM", "^\w+$");
  etc.
Attachment #645715 - Flags: review?(philipp) → review+
Comment on attachment 645716 [details] [diff] [review]
Part 3: send M-Send.req

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

::: dom/mms/src/ril/MmsService.js
@@ +234,5 @@
> +    msg.headers["x-mms-message-type"] = MMS.MMS_PDU_TYPE_SEND_REQ;
> +    if (!msg.headers["x-mms-transaction-id"]) {
> +      // Create an unique transaction id
> +      let uuidgen = Cc["@mozilla.org/uuid-generator;1"]
> +                    .getService(Ci.nsIUUIDGenerator);

Please use a lazy getter for this in the top scope:

  XPCOMUtils.defineLazyServiceGetter(this, gUUIDGenerator, "@mozilla.org/uuid-generator;1", "nsIUUIDGenerator");
Attachment #645716 - Flags: review?(philipp) → review+
(Assignee)

Comment 11

5 years ago
Created attachment 646009 [details] [diff] [review]
Part 1: disable some v1.4 parameters for compatibility : V2

Update r=philikon only. Since these patches were previously reviewed+, won't set review flag here.
Attachment #645708 - Attachment is obsolete: true
(Assignee)

Comment 12

5 years ago
Created attachment 646010 [details] [diff] [review]
Part 2: Add required encoders for M-Send.req/M-Send.conf PDU : V2

Accommodate to comment #9. Thanks Philipp!
Attachment #645715 - Attachment is obsolete: true
(Assignee)

Comment 13

5 years ago
Created attachment 646011 [details] [diff] [review]
Part 3: send M-Send.req : V2

Accommodate to comment #10. Thanks Philipp!
Attachment #645716 - Attachment is obsolete: true
(Assignee)

Comment 14

5 years ago
Created attachment 646012 [details] [diff] [review]
[Debug] DO NOT SUBMIT : V3

add text/plain for easier debug
Attachment #645718 - Attachment is obsolete: true
(Assignee)

Comment 15

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/83254a09a38f
http://hg.mozilla.org/integration/mozilla-inbound/rev/7001e01f26ec
http://hg.mozilla.org/integration/mozilla-inbound/rev/13a837f5cba4
Flags: in-testsuite+
Target Milestone: --- → mozilla17
https://hg.mozilla.org/mozilla-central/rev/83254a09a38f
https://hg.mozilla.org/mozilla-central/rev/7001e01f26ec
https://hg.mozilla.org/mozilla-central/rev/13a837f5cba4
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.