res.type nukes the default charset #1261

Closed
glenjamin opened this Issue Aug 1, 2012 · 6 comments

Projects

None yet

4 participants

@glenjamin

eg.

res.type('text');
res.send("Some text");

Produces Content-Type: text/plain with no charset

It feels like it'd be more helpful for the shorthand version of type() to use the value of res.charset

res.type('text');
res.send("Some text");
// Content-Type => text/plain; charset=utf-8

I think this just means moving the default setting higher up in send?

@tj
Member
tj commented Aug 1, 2012

yeah sounds buggy to me i'll look into that in a minute

@tj
Member
tj commented Aug 1, 2012

hmm might not be a great solution for this right now, res.charset is a Connect thing actually, and you have to set it before doing res.setHeader with a Content-Type, but we can see about using the "header" event that Connect adds to add this in

@glenjamin

How about changing this:

res.contentType =
res.type = function(type){
  return this.set('Content-Type', ~type.indexOf('/')
    ? type
    : mime.lookup(type));
};

To something like this:

res.contentType =
res.type = function(type){
  if (!~type.indexOf('/')) {
    this.charset = this.charset || 'utf-8';
    type = mime.lookup(type)
  }
  return this.set('Content-Type', type);
};
@tj
Member
tj commented Aug 4, 2012

the problem is that it has to work for a regular res.setHeader content-type as well. I added a "header" event in connect as well so we can probably use that, I'll try that soon if I dont forget, then just check charset and apply it that way

@jonathanong jonathanong was assigned Dec 4, 2013
@jonathanong
Member

btw we got to bake this into express in 4.0 like koa does. i'm removing all node patches in connect for v3

@defunctzombie

Master no longer depends on connect. If possible I would like to remove this header event if nothing in the codebase uses it now. I believe this is no longer an issue in current master and if it is, please comment here with a failing test.

@jonathanong jonathanong was unassigned by glenjamin Jul 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment