1138220 - some headers are not not properly capitalized

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Magnus Melin [:mkmelin]]

I notice some of the headers in messages we send out are not properly Capitalized. 

E.g. 
 - x-mozilla-news-host
 - user-agent
 - mime-version

They should of course read User-Agent, MIME-Version etc.

I assume this is fallout from jsmime.

Comment 1

[Masatoshi Kimura [:emk]]

Does it matter? MIME-headers are supposed to be case-insensitive.

Comment 2

[Joshua Cranmer [:jcranmer]]

MIME headers are case-insensitive; jsmime itself should be spitting out the headers for unknown headers (e.g., user-agent) in whatever case you sent it, but I believe we're normalizing to lower case within the compose backend because we don't have an easy case-preserving-but-insensitive map type.

Comment 3

[Magnus Melin [:mkmelin]]

Yes of course they are case-insensitive per spec. That doesn't make it less ugly.

Do you have a ref to where it all happens?
If we don't know, we should normalize to Upper-Cased like most headers would like to be. MIME-Version is a one off, but should be known.

Comment 4

[Joshua Cranmer [:jcranmer]]

https://dxr.mozilla.org/comm-central/source/mailnews/mime/src/mimeJSComponents.js#120

Comment 5

[WADA:World Anti-bad-Duping Agency]

  var Normlize=function (headename) { // assums (typeof headename)=="string"
  var xx=headename.split("-");
  for(var nn=0;nn<xx.length&&0<xx[nn].length;nn++) xx[nn]=xx[nn].substr(0,1).toUpperCase()+xx[nn].substr(1,xx[nn].length-1);
  return a.join("-");
  }

Comment 6

[Magnus Melin [:mkmelin]]

Attachment: bug1138220_headers_capitalization.patch

Add a few other known headers, and make the default headers names Upper-Dashed-Camel-Cased.

Comment 7

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8581201 [details] [diff] [review]
bug1138220_headers_capitalization.patch

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

::: mailnews/mime/jsmime/jsmime.js
@@ +167,5 @@
>  addHeader("Return-Receipt-To", parseAddress, writeAddress);
>  
> +// http://cr.yp.to/proto/replyto.html
> +addHeader("Mail-Reply-To", parseAddress, writeAddress);
> +addHeader("Mail-Followup-To", parseAddress, writeAddress);

These headers need to be added to the test_structured_headers.js and test_structured_header_emitters.js tests.

@@ +211,5 @@
>  
>  // RFC 5322
>  addHeader("Comments", parseUnstructured, writeUnstructured);
>  addHeader("Keywords", parseUnstructured, writeUnstructured);
> +addHeader("Message-ID", parseUnstructured, writeUnstructured);

Please remove this line. Message-ID needs its own separate treatment, and unstructured isn't appropriate.

@@ +2868,5 @@
> +    // Assume it's an unstructured header.
> +    // All-lower-case-names are ugly, so capitalize first letters.
> +    name = name.replace(/(^|-)[a-z]/g, function(match) {
> +      return match.toUpperCase();
> +    });

I feel like this needs test support as well.

Comment 8

[Magnus Melin [:mkmelin]]

(In reply to Joshua Cranmer [:jcranmer] from comment #7)
> > +addHeader("Message-ID", parseUnstructured, writeUnstructured);
> 
> Please remove this line. Message-ID needs its own separate treatment, and
> unstructured isn't appropriate.

Doesn't it end up being treated that anyway at the moment? It's only added to cater for the proper (special) capitalization. So should I just add parseMessageID and writeMessageID functions as placeholders that call parse/writeUnstructured?

Comment 9

[Magnus Melin [:mkmelin]]

Attachment: bug1138220_headers_capitalization.patch

Comment 10

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8591335 [details] [diff] [review]
bug1138220_headers_capitalization.patch

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

I'm still not happy about adding the Message-ID stuff just to say Message-ID instead of Message-Id...

::: mailnews/mime/jsmime/jsmime.js
@@ +217,5 @@
> +}
> +function writeMessageID(value) {
> +  // TODO: Proper parsing support for these headers is currently unsupported).
> +  this.addUnstructured(value);
> +}

Nit: move the Message-ID related headers to their own block after the Date headers. (Message-ID headers will eventually constitute their own kind of headers.)

@@ +1212,5 @@
>   * - Return-Receipt-To
>   * - Sender
>   * - To
> + * - Mail-Reply-To
> + * - Mail-Followup-To

Nit: sort this list alphabetically.

::: mailnews/mime/jsmime/test/test_structured_header_emitters.js
@@ +87,5 @@
>      assert.equal(str,
>        'From: [e-mail]\r\n' +
> +      'Subject: [Bug 939557] browsercomps.dll failed to build\r\n'+
> +      'X-Capitalization-Test: should capitalize\r\n'
> +);

Nit: er, extraneous newline.

Comment 11

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/3bbad30cccf2 -> FIXED

Comment 12

[Kent James (:rkent)]

Comment on attachment 8591335 [details] [diff] [review]
bug1138220_headers_capitalization.patch

[Triage Comment]
http://hg.mozilla.org/releases/comm-aurora/rev/4f1cad9b6bd8
http://hg.mozilla.org/releases/comm-beta/rev/244c73782efb