Closed
Bug 1138220
Opened 9 years ago
Closed 9 years ago
some headers are not not properly capitalized
Categories
(MailNews Core :: Composition, defect)
Tracking
(thunderbird38+ fixed, thunderbird39 fixed)
RESOLVED
FIXED
Thunderbird 40.0
People
(Reporter: mkmelin, Assigned: mkmelin)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
9.43 KB,
patch
|
jcranmer
:
review+
rkent
:
approval-comm-aurora+
rkent
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
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•9 years ago
|
||
Does it matter? MIME-headers are supposed to be case-insensitive.
Comment 2•9 years ago
|
||
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.
Assignee | ||
Comment 3•9 years ago
|
||
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•9 years ago
|
||
https://dxr.mozilla.org/comm-central/source/mailnews/mime/src/mimeJSComponents.js#120
Updated•9 years ago
|
Severity: normal → minor
Comment 5•9 years ago
|
||
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("-"); }
Assignee | ||
Updated•9 years ago
|
OS: Linux → All
Hardware: x86_64 → All
Assignee | ||
Comment 6•9 years ago
|
||
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 7•9 years ago
|
||
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-
Assignee | ||
Comment 8•9 years ago
|
||
(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?
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8581201 -
Attachment is obsolete: true
Attachment #8591335 -
Flags: review?(Pidgeot18)
Comment 10•9 years ago
|
||
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+
Assignee | ||
Comment 11•9 years ago
|
||
https://hg.mozilla.org/comm-central/rev/3bbad30cccf2 -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-thunderbird38:
--- → affected
status-thunderbird39:
--- → affected
tracking-thunderbird38:
--- → +
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 40.0
Comment 12•9 years ago
|
||
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
Attachment #8591335 -
Flags: approval-comm-beta+
Attachment #8591335 -
Flags: approval-comm-aurora+
Updated•9 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•