Last Comment Bug 773592 - B2G MMS: support multiple instances for some header fields
: B2G MMS: support multiple instances for some header fields
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla17
Assigned To: Vicamo Yang [:vicamo][:vyang]
:
:
Mentors:
Depends on:
Blocks: b2g-mms b2g-mms-dom-api
  Show dependency treegraph
 
Reported: 2012-07-13 04:11 PDT by Vicamo Yang [:vicamo][:vyang]
Modified: 2012-07-20 06:41 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v0 (3.02 KB, patch)
2012-07-15 01:47 PDT, Vicamo Yang [:vicamo][:vyang]
philipp: review+
Details | Diff | Splinter Review
v1 (2.97 KB, patch)
2012-07-18 03:28 PDT, Vicamo Yang [:vicamo][:vyang]
philipp: review+
Details | Diff | Splinter Review

Description Vicamo Yang [:vicamo][:vyang] 2012-07-13 04:11:30 PDT

    
Comment 1 Vicamo Yang [:vicamo][:vyang] 2012-07-13 04:36:38 PDT
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] 2012-07-13 05:08:39 PDT
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] 2012-07-15 01:47:24 PDT
Created attachment 642353 [details] [diff] [review]
v0

simple implementation & test cases
Comment 4 Philipp von Weitershausen [:philikon] 2012-07-16 14:51:39 PDT
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] 2012-07-18 03:28:17 PDT
Created attachment 643307 [details] [diff] [review]
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] 2012-07-19 23:42:40 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/126a1ebf55e6
Comment 7 Ed Morley [:emorley] 2012-07-20 06:41:48 PDT
https://hg.mozilla.org/mozilla-central/rev/126a1ebf55e6

Note You need to log in before you can comment on or make changes to this bug.