869291 - Support Receiving WAP-Push-CP

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

Receive and decode WAP Push CP from WBXML to XML.

Comment 1

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

Attachment: 0001. Add CP decoder.

Add decoder for OTA Provisioning

Comment 2

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

Attachment: 0002. Handle CP message.

Handle OTA Provisioning on receiving.

Comment 3

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

Attachment: 0003.Add test case for CP message.

Test cases for OTA Provisioning decoding, including plain text and WBXML format.

Comment 4

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

Comment on attachment 765181 [details] [diff] [review]
0001. Add CP decoder.

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

::: dom/wappush/src/gonk/CpPduHelper.jsm
@@ +120,5 @@
> +    names[name] = names[number] = entry;
> +  }
> +
> +  add("name",     "",                                 0x05);
> +  add("value",    "",                                 0x06);

I don't really understand what do you mean by giving a empty string here while it's going to be an attribute name.  Please don't just follow previous patterns because of those patterns.  Do it as you need it.

Comment 5

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

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #4)
> Comment on attachment 765181 [details] [diff] [review]
> 0001. Add CP decoder.
> 
> Review of attachment 765181 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/wappush/src/gonk/CpPduHelper.jsm
> @@ +120,5 @@
> > +    names[name] = names[number] = entry;
> > +  }
> > +
> > +  add("name",     "",                                 0x05);
> > +  add("value",    "",                                 0x06);
> 
> I don't really understand what do you mean by giving a empty string here
> while it's going to be an attribute name.  Please don't just follow previous
> patterns because of those patterns.  Do it as you need it.

empty string indicates the value of name/value attribute is not defined as well-known value, so its value will be defined right after, in string.
In such case, these two attribute will be decoded into "name=" and "value=", and concatenated with the string value part.
This is defined in WAP-183-ProvCont-20010724-A, 8.2.3.

This might happen on provider-specific values, specially in VENDORCONFIG section.
I think they are necessary.

Comment 6

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

Attachment: 0001. Handle attribute token and value token separately.

Refactory : handle attribute token and value token separately in main WBXML decoder.
corresponding modifications are applied to SI and SL helper.

Comment 7

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

Attachment: 0002. Add CP decoder. V2

Address comment 4.

Comment 8

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

Attachment: 0003. Handle CP message.

Rebase.

Comment 9

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

Attachment: 0004.Add test case for CP message.

Comment 10

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

There is a problem on how to handle OPAQUE data in VENDORCONFIG Characteristics.
It's application-specific, but we can't keep track on VENDORCONFIG of every provider.

Comment 11

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

After discussion, if we found VENDORCONFIG Characteristics in WBXML encoded CP message, we will not decode it and send notification with message in original form( WBXML encoded message).

Comment 12

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

Attachment: 0001. Refactory existing WAP PUSH handlers.

1. Handle attribute token and value token separately.
2. Throw error on invalid WBXML token.

Comment 13

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

Attachment: 0002. Add CP decoder. V3

Notify with raw data on existence of VENDORCONFIG Characteristics, as mentioned in comment 11.

Comment 14

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

Attachment: 0003. Handle CP message.

Rebase

Comment 15

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

Attachment: 0004. Add test case for CP message. V2

Add test for appearance of VENDORCONFIG in WBXML message.

Comment 16

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

Comment on attachment 770611 [details] [diff] [review]
0002. Add CP decoder. V3

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

::: dom/wappush/src/gonk/CpPduHelper.jsm
@@ +90,5 @@
> +   * @return An instance of nsIMultiplexInputStream or null in case of errors.
> +   */
> +compose: function compose_cp(multiStream, msg) {
> +    // Composing CP message is not supported
> +    return null;

Please just remove it.

Comment 17

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

Comment on attachment 770610 [details] [diff] [review]
0001. Refactory existing WAP PUSH handlers.

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

::: dom/wappush/src/gonk/SiPduHelper.jsm
@@ +90,5 @@
>        let appToken = {
>          publicId: PUBLIC_IDENTIFIER_SI,
>          tagToken: SI_TAG_FIELDS,
>          attrToken: SI_ATTRIBUTE_FIELDS,
> +        valueToken: SI_VALUE_FIELDS,

Just have a re-scan.  Could you rename all the three token attributes to |fooTokenList|, e.g. |tagToken| => |tagTokenList|.

@@ +174,5 @@
>    add("href",         "",               0x0B);
>    add("href",         "http://",        0x0C);
>    add("href",         "http://www.",    0x0D);
>    add("href",         "https://",       0x0E);
>    add("href",         "https://www.",   0x0F);

Hi, in the |add()| function in this SI_ATTRIBUTE_FIELDS initiator we have:

  names[name] = names[number] = entry;

Since there are several entries with the same name here, will that cause potential errors?  Besides, when do we need the lookup |SI_ATTRIBUTE_FIELDS[name]| actually?  I think we need only |SI_ATTRIBUTE_FIELDS[number]|, right?

Comment 18

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

Comment on attachment 770610 [details] [diff] [review]
0001. Refactory existing WAP PUSH handlers.

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

I also find we have:

  this.SiPduHelper = {
    parse: function parse_si(data, contentType) {
      let msg = {};
      ...
      WBXML.PduHelper.parse(data, appToken, msg);
      ...
    }
  };

  this.WbxmlPduHelper = {
    parse: function parse_wbxml(data, appToken, msg) {
      if (!msg) {
        msg = {};
      }
      ...
    }
  };

This doesn't really help when somebody calls |WbxmlPduHelper.parse()| with a null/undefined |msg|.  Please always check the return value.

::: dom/wappush/src/gonk/SiPduHelper.jsm
@@ +116,5 @@
> +        let stringData = WSP.Octet.decodeMultiple(data, data.array.length);
> +        msg.content = WSP.PduHelper.decodeStringContent(stringData, "UTF-8");
> +      } catch (e) {
> +        // Provide raw data if we failed to parse.
> +        msg.content = data.array;

Why do you skip assigning |msg.contentType = "application/vnd.wap.si";| here?

::: dom/wappush/src/gonk/SlPduHelper.jsm
@@ +72,5 @@
> +        let stringData = WSP.Octet.decodeMultiple(data, data.array.length);
> +        msg.content = WSP.PduHelper.decodeStringContent(stringData, "UTF-8");
> +      } catch (e) {
> +        // Provide raw data if we failed to parse.
> +        msg.content = data.array;

ditto.

Comment 19

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

Comment on attachment 770611 [details] [diff] [review]
0002. Add CP decoder. V3

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

::: dom/wappush/src/gonk/CpPduHelper.jsm
@@ +77,5 @@
> +      }
> +      return msg;
> +    }
> +
> +    return null;

Please have re-examine on the call path and make sure all errors result in an event with raw data.

Comment 20

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

Comment on attachment 770611 [details] [diff] [review]
0002. Add CP decoder. V3

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

::: dom/wappush/src/gonk/CpPduHelper.jsm
@@ +40,5 @@
> +     * Message is compressed by WBXML, decode into string.
> +     *
> +     * @see WAP-192-WBXML-20010725-A
> +     */
> +    if (!contentType || contentType === "application/vnd.wap.connectivity-wbxml") {

You have following code in WapPushManager.js:

  if (contentType === "text/vnd.wap.connectivity-xml" ||
      contentType === "application/vnd.wap.connectivity-wbxml") {
    msg = CP.PduHelper.parse(data, contentType);
  }

So how could |contentType| be evaluated as false here?

Comment 21

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

> ::: dom/wappush/src/gonk/SiPduHelper.jsm
> @@ +116,5 @@
> > +        let stringData = WSP.Octet.decodeMultiple(data, data.array.length);
> > +        msg.content = WSP.PduHelper.decodeStringContent(stringData, "UTF-8");
> > +      } catch (e) {
> > +        // Provide raw data if we failed to parse.
> > +        msg.content = data.array;
> 
> Why do you skip assigning |msg.contentType = "application/vnd.wap.si";| here?

I think we can't assume that data can't be decoded as "text/vnd.wap.si" must be "application/vnd.wap.sic", so I keep its original type.

Comment 22

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

Attachment: 0001. Refactory existing WAP PUSH handlers. V2

1. Address comment 17 and comment 18.
2. Make sure all errors throws an error.

Comment 23

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

Attachment: 0002. Add CP decoder. V4

1. Address comment 19 and comment 20.
2. Make changes corresponding to patch 0001.

Comment 24

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

Comment on attachment 778376 [details] [diff] [review]
0001. Refactory existing WAP PUSH handlers. V2

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

Please pay attention to data binding of a function, especially a javascript one.  How many attributes are supposed to be included in the returned object?  What are they?  Please document them since there are already too many similar errors now.

::: dom/wappush/src/gonk/SiPduHelper.jsm
@@ +99,5 @@
>  
> +      try {
> +        let parseResult = WBXML.PduHelper.parse(data, appToken);
> +        msg.content = parseResult.content;
> +        msg.contentType = "text/vnd.wap.si";

You missed |publicId|, |version| and maybe still others.  You can really have:

  let msg = WBXML.PduHelper.parse(data, appToken);
  msg.contentType = "text/vnd.wap.si";

@@ +102,5 @@
> +        msg.content = parseResult.content;
> +        msg.contentType = "text/vnd.wap.si";
> +      } catch (e) {
> +        // Provide raw data if we failed to parse.
> +        msg.content = data.array;

Same here.

Comment 25

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

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #24)
> Comment on attachment 778376 [details] [diff] [review]
> 0001. Refactory existing WAP PUSH handlers. V2
> 
> Review of attachment 778376 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Please pay attention to data binding of a function, especially a javascript
> one.  How many attributes are supposed to be included in the returned
> object?  What are they?  Please document them since there are already too
> many similar errors now.
> 
> ::: dom/wappush/src/gonk/SiPduHelper.jsm
> @@ +99,5 @@
> >  
> > +      try {
> > +        let parseResult = WBXML.PduHelper.parse(data, appToken);
> > +        msg.content = parseResult.content;
> > +        msg.contentType = "text/vnd.wap.si";
> 
> You missed |publicId|, |version| and maybe still others.  You can really
> have:
> 
>   let msg = WBXML.PduHelper.parse(data, appToken);
>   msg.contentType = "text/vnd.wap.si";
> 

I think these headers are mainly providing information for decoder, since we have decoded the message into text, we might don't have to bring those info anymore.
If we failed to decode, then those information are not available.
So the function returns only |content| and |contentType|.

Attribute |publicId| added in text parsing will be removed.

Comment 26

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

Attachment: 0001. Refactory existing WAP PUSH handlers. V3

Address comment 24.

Comment 27

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

Attachment: 0002. Add CP decoder. V5

Apply same modification as part 0001 to CP helper.

Comment 28

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

Comment on attachment 778409 [details] [diff] [review]
0001. Refactory existing WAP PUSH handlers. V3

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

::: dom/wappush/src/gonk/WbxmlPduHelper.jsm
@@ +149,5 @@
> +      globalTagTokenList[token] = WBXML_GLOBAL_TOKENS[token];
> +    }
> +    for (let token in globalTokenOverrideList) {
> +      globalTagTokenList[token] = globalTokenOverrideList[token];
> +    }

You can have:

  let globalTagTokenList = Object.create(WBXML_GLOBAL_TOKENS);
  if (appToken.globalTokenOverride) {
    let globalTokenOverrideList = appToken.globalTokenOverride;
    for (let token in globalTokenOverrideList) {
      globalTagTokenList[token] = globalTokenOverrideList[token];
    }
  }

Comment 29

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

Comment on attachment 770613 [details] [diff] [review]
0004. Add test case for CP message. V2

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

::: dom/wappush/tests/test_cp_pdu_helper.js
@@ +13,5 @@
> + * CP in plain text
> + *
> + * Test case from OMA-TS-WAP_ProvCont-V1_1-20090421-C.pdf, clause 6.1
> + */
> +add_test(function test_cp_parse_plain_text() {

Please create an utility function to testing CP decoding.  Something like:

  function foo(array, contentType, result) {
    let data = { array: array, offset: 0 };
    let msg = CP.PduHelper.parse(data, contentType);
    do_check_eq(msg.contentType, result.contentType);
    do_check_eq(msg.content, result.content);    
  }

@@ +213,5 @@
> +                  0x72, 0x6F, 0x76, 0x69, 0x73, 0x69, 0x6F, 0x6E,
> +                  0x69, 0x6E, 0x67, 0x64, 0x6F, 0x63, 0x3E
> +                ]);
> +  data.offset = 0;
> +  let result = "<?xml version=\"1.0\"?><!DOCTYPE wap-provisioningdoc PUBLIC \"-//WAPFORUM//DTD PROV 1.0//EN\" \"http://www.wapforum.org/DTD/prov.dtd\"><wap-provisioningdoc version=\"1.0\"><characteristic type=\"PXLOGICAL\"><parm name=\"PROXY-ID\" value=\"170.187.51.4\"/><parm name=\"NAME\" value=\"BankMainProxy\"/><parm name=\"STARTPAGE\" value=\"http://www.bank.com/startpage.wml\"/><characteristic type=\"PXAUTHINFO\"><parm name=\"PXAUTH-TYPE\" value=\"HTTP-BASIC\"/><parm name=\"PXAUTH-ID\" value=\"pxusername\"/><parm name=\"PXAUTH-PW\" value=\"pxuserpasswd\"/></characteristic><characteristic type=\"PXPHYSICAL\"><parm name=\"PHYSICAL-PROXY-ID\" value=\"PROXY 1\"/><parm name=\"DOMAIN\" value=\"www.bank.com/\"/><parm name=\"PXADDR\" value=\"170.187.51.3\"/><parm name=\"PXADDRTYPE\" value=\"IPV4\"/><parm name=\"TO-NAPID\" value=\"INTERNET\"/><parm name=\"TO-NAPID\" value=\"NAP1\"/><characteristic type=\"PORT\"><parm name=\"PORTNBR\" value=\"9203\"/></characteristic></characteristic></characteristic><characteristic type=\"NAPDEF\"><parm name=\"NAPID\" value=\"NAP1\"/><parm name=\"BEARER\" value=\"GSM-CSD\"/><parm name=\"NAME\" value=\"MY ISP CSD\"/><parm name=\"NAP-ADDRESS\" value=\"[phone]\"/><parm name=\"NAP-ADDRTYPE\" value=\"E164\"/><parm name=\"CALLTYPE\" value=\"ANALOG-MODEM\"/><characteristic type=\"NAPAUTHINFO\"><parm name=\"AUTHTYPE\" value=\"PAP\"/><parm name=\"AUTHNAME\" value=\"wwwmmmuser\"/><parm name=\"AUTHSECRET\" value=\"wwwmmmsecret\"/></characteristic><characteristic type=\"VALIDITY\"><parm name=\"COUNTRY\" value=\"228\"/><parm name=\"NETWORK\" value=\"001\"/></characteristic></characteristic></wap-provisioningdoc>";

Could you help a little bit more by formatting this long long string with some line breaks and make it more human-readable?

@@ +214,5 @@
> +                  0x69, 0x6E, 0x67, 0x64, 0x6F, 0x63, 0x3E
> +                ]);
> +  data.offset = 0;
> +  let result = "<?xml version=\"1.0\"?><!DOCTYPE wap-provisioningdoc PUBLIC \"-//WAPFORUM//DTD PROV 1.0//EN\" \"http://www.wapforum.org/DTD/prov.dtd\"><wap-provisioningdoc version=\"1.0\"><characteristic type=\"PXLOGICAL\"><parm name=\"PROXY-ID\" value=\"170.187.51.4\"/><parm name=\"NAME\" value=\"BankMainProxy\"/><parm name=\"STARTPAGE\" value=\"http://www.bank.com/startpage.wml\"/><characteristic type=\"PXAUTHINFO\"><parm name=\"PXAUTH-TYPE\" value=\"HTTP-BASIC\"/><parm name=\"PXAUTH-ID\" value=\"pxusername\"/><parm name=\"PXAUTH-PW\" value=\"pxuserpasswd\"/></characteristic><characteristic type=\"PXPHYSICAL\"><parm name=\"PHYSICAL-PROXY-ID\" value=\"PROXY 1\"/><parm name=\"DOMAIN\" value=\"www.bank.com/\"/><parm name=\"PXADDR\" value=\"170.187.51.3\"/><parm name=\"PXADDRTYPE\" value=\"IPV4\"/><parm name=\"TO-NAPID\" value=\"INTERNET\"/><parm name=\"TO-NAPID\" value=\"NAP1\"/><characteristic type=\"PORT\"><parm name=\"PORTNBR\" value=\"9203\"/></characteristic></characteristic></characteristic><characteristic type=\"NAPDEF\"><parm name=\"NAPID\" value=\"NAP1\"/><parm name=\"BEARER\" value=\"GSM-CSD\"/><parm name=\"NAME\" value=\"MY ISP CSD\"/><parm name=\"NAP-ADDRESS\" value=\"[phone]\"/><parm name=\"NAP-ADDRTYPE\" value=\"E164\"/><parm name=\"CALLTYPE\" value=\"ANALOG-MODEM\"/><characteristic type=\"NAPAUTHINFO\"><parm name=\"AUTHTYPE\" value=\"PAP\"/><parm name=\"AUTHNAME\" value=\"wwwmmmuser\"/><parm name=\"AUTHSECRET\" value=\"wwwmmmsecret\"/></characteristic><characteristic type=\"VALIDITY\"><parm name=\"COUNTRY\" value=\"228\"/><parm name=\"NETWORK\" value=\"001\"/></characteristic></characteristic></wap-provisioningdoc>";
> +  let msg = CP.PduHelper.parse(data, contentType);

|msg| was originally defined in line 18 and is re-defined here.

@@ +280,5 @@
> +                  0x03, 0x30, 0x30, 0x31, 0x00, 0x01, 0x01, 0x01,
> +                  0x01
> +                ]);
> +  data.offset = 0;
> +  let result = "<wap-provisioningdoc version=\"1.0\"><characteristic type=\"PXLOGICAL\"><parm name=\"PROXY-ID\" value=\"170.187.51.4\"/><parm name=\"NAME\" value=\"BankMainProxy\"/><parm name=\"STARTPAGE\" value=\"http://www.bank.com/startpage.wml\"/><characteristic type=\"PXAUTHINFO\"><parm name=\"PXAUTH-TYPE\" value=\"HTTP-BASIC\"/><parm name=\"PXAUTH-ID\" value=\"pxusername\"/><parm name=\"PXAUTH-PW\" value=\"pxuserpasswd\"/></characteristic><characteristic type=\"PXPHYSICAL\"><parm name=\"PHYSICAL-PROXY-ID\" value=\"PROXY 1\"/><parm name=\"DOMAIN\" value=\"www.bank.com/\"/><parm name=\"PXADDR\" value=\"170.187.51.3\"/><parm name=\"PXADDRTYPE\" value=\"IPV4\"/><parm name=\"TO-NAPID\" value=\"INTERNET\"/><parm name=\"TO-NAPID\" value=\"NAP1\"/><characteristic type=\"PORT\"><parm name=\"PORTNBR\" value=\"9203\"/></characteristic></characteristic></characteristic><characteristic type=\"NAPDEF\"><parm name=\"NAPID\" value=\"NAP1\"/><parm name=\"BEARER\" value=\"GSM-CSD\"/><parm name=\"NAME\" value=\"MY ISP CSD\"/><parm name=\"NAP-ADDRESS\" value=\"[phone]\"/><parm name=\"NAP-ADDRTYPE\" value=\"E164\"/><parm name=\"CALLTYPE\" value=\"ANALOG-MODEM\"/><characteristic type=\"NAPAUTHINFO\"><parm name=\"AUTHTYPE\" value=\"PAP\"/><parm name=\"AUTHNAME\" value=\"wwwmmmuser\"/><parm name=\"AUTHSECRET\" value=\"wwwmmmsecret\"/></characteristic><characteristic type=\"VALIDITY\"><parm name=\"COUNTRY\" value=\"228\"/><parm name=\"NETWORK\" value=\"001\"/></characteristic></characteristic></wap-provisioningdoc>";

ditto

Comment 30

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

Attachment: 0001. Refactory existing WAP PUSH handlers. V4

Address comment 28.
Thank you for the tweak!

Comment 31

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

Attachment: 0004. Add test case for CP message. V3

Address comment 29.

Comment 32

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

Comment on attachment 779114 [details] [diff] [review]
0004. Add test case for CP message. V3

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

::: dom/wappush/tests/test_cp_pdu_helper.js
@@ +292,5 @@
> +        "<characteristic type=\"PORT\">" +
> +        "<parm name=\"PORTNBR\" value=\"9203\"/>" +
> +      "</characteristic>" +
> +    "</characteristic>" +
> +  "</characteristic>" +

Now we know here is a unmatched '</characteristic>'.

Comment 33

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

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #32)
> Comment on attachment 779114 [details] [diff] [review]
> 0004. Add test case for CP message. V3
> 
> Review of attachment 779114 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/wappush/tests/test_cp_pdu_helper.js
> @@ +292,5 @@
> > +        "<characteristic type=\"PORT\">" +
> > +        "<parm name=\"PORTNBR\" value=\"9203\"/>" +
> > +      "</characteristic>" +
> > +    "</characteristic>" +
> > +  "</characteristic>" +
> 
> Now we know here is a unmatched '</characteristic>'.

Sorry I missed the indent of "<characteristic type=\"PORT\">"

Comment 34

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

Attachment: 0004. Add test case for CP message. V4

Fix indent per comment 33

Comment 35

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

Try : https://tbpl.mozilla.org/?tree=Try&rev=77dc84b6c46b

Comment 36

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

Attachment: 0004. Add test case for CP message. V4.1

Fix description format in patch summary

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/projects/birch/rev/7a4e174d7dbc
https://hg.mozilla.org/projects/birch/rev/f31f09459b3f
https://hg.mozilla.org/projects/birch/rev/1e4285701fac
https://hg.mozilla.org/projects/birch/rev/e8ee9efc1d3b

Comment 38

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7a4e174d7dbc
https://hg.mozilla.org/mozilla-central/rev/f31f09459b3f
https://hg.mozilla.org/mozilla-central/rev/1e4285701fac
https://hg.mozilla.org/mozilla-central/rev/e8ee9efc1d3b