773592 - B2G MMS: support multiple instances for some header fields

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Comment 1

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

Some MMS headers fields may appear multiple times. They are:

1. To: in M-Send.req, M-Retrieve.conf, M-Forward.req, M-Mbox-Descr
2. Cc: in M-Send.req, M-Retrieve.conf, M-Forward.req, M-Mbox-Descr
3. Bcc: in M-Send.req, M-Forward.req, M-Mbox-Descr
4. X-Mms-Previously-Sent-By: in M-Retrieve.conf, M-Mbox-Descr
5. X-Mms-Previously-Sent-Date: in M-Retrieve.conf, M-Mbox-Descr
6. X-Mms-MM-Flags: in M-Retrieve.conf, M-Forward.req, M-Mbox-Store.req, M-Mbox-View.req, M-Mbox-View.conf, M-Mbox-Descr, M-Mbox-Upload.req
7. X-Mms-Content-Location: in M-Mbox-View.req, M-Mbox-View.req, M-Mbox-View.conf, M-Mbox-Delete.req, M-Mbox-Delete.conf, M-Delete.req, M-Delete.conf
8. X-Mms-MM-State: in M-Mbox-View.req, M-Mbox-View.conf
9. X-Mms-Attributes: in M-Mbox-View.req, M-Mbox-View.conf
10. X-Mms-Response-Status: in M-Mbox-Delete.conf, M-Delete.conf
11. X-Mms-Response-Text: in M-Mbox-Delete.conf, M-Delete.conf

Comment 2

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

Some of above fields may appear only once in some PDU types:

X-Mms-MM-Flags: M-Send.req
X-Mms-Content-Location: M-Send.req, M-Notification.ind, M-Forward.req, M-Forward.conf, M-Mbox-Store.req, M-Mbox-Store.conf, M-Mbox-Descr, M-Mbox-Upload.conf
X-Mms-MM-State: M-Send.req, M-Retrieve.conf, M-Forward.req, M-Mbox-Store.req, M-Mbox-Descr
X-Mms-Response-Status: M-Send.conf, M-Forward.conf
X-Mms-Response-Text: M-Send.conf, M-Forward.conf

Comment 3

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

Attachment: v0

simple implementation & test cases

Comment 4

[Philipp von Weitershausen [:philikon]]

Comment on attachment 642353 [details] [diff] [review]
v0

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

::: dom/mms/src/ril/MmsPduHelper.jsm
@@ +980,5 @@
> +            headers[header.name] = [orig, header.value];
> +          }
> +        } else {
> +          headers[header.name] = header.value;
> +        }

Nit: this could be one flat if/else if/else block:

  let orig = headers[header.name];
  if (Array.isArray(orig)) {
    headers[header.name].push(header.value);
  } else if (orig) {
    headers[header.name] = [orig, header.value];
  } else {
    headers[header.name] = header.value;
  }

::: dom/mms/tests/test_mms_pdu_helper.js
@@ +555,5 @@
> +//
> +
> +//// PduHelper.parseHeaders ////
> +
> +add_test(function test_PduHelper_parseHeaders() {

I don't see a test for multiple header values.

r=me with those.

Comment 5

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

Attachment: v1

Address review comment #4.

Philipp, test case for multi-header values is already in patch v0 chunk #2. I added a second case in the same test function:

+  // Parse header fields with multiple entries
+  expect = {
+    to: [
+      { address: "+123", type: "PLMN" },
+      { address: "+456", type: "num" },
+    ],
+  };

Comment 6

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

http://hg.mozilla.org/integration/mozilla-inbound/rev/126a1ebf55e6

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/126a1ebf55e6