Closed Bug 1138220 Opened 5 years ago Closed 5 years ago

some headers are not not properly capitalized

Categories

(MailNews Core :: Composition, defect, minor)

defect
Not set
minor

Tracking

(thunderbird38+ fixed, thunderbird39 fixed)

RESOLVED FIXED
Thunderbird 40.0
Tracking Status
thunderbird38 + fixed
thunderbird39 --- fixed

People

(Reporter: mkmelin, Assigned: mkmelin)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

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.
Does it matter? MIME-headers are supposed to be case-insensitive.
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.
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.
Severity: normal → minor
See Also: → 1143569
  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("-");
  }
OS: Linux → All
Hardware: x86_64 → All
Add a few other known headers, and make the default headers names Upper-Dashed-Camel-Cased.
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #8581201 - Flags: review?(Pidgeot18)
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.
Attachment #8581201 - Flags: review?(Pidgeot18) → review-
(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?
Attachment #8581201 - Attachment is obsolete: true
Attachment #8591335 - Flags: review?(Pidgeot18)
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: bugzilla-daemon@mozilla.org\r\n' +
> +      'Subject: [Bug 939557] browsercomps.dll failed to build\r\n'+
> +      'X-Capitalization-Test: should capitalize\r\n'
> +);

Nit: er, extraneous newline.
Attachment #8591335 - Flags: review?(Pidgeot18) → review+
https://hg.mozilla.org/comm-central/rev/3bbad30cccf2 -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 40.0
You need to log in before you can comment on or make changes to this bug.