Skip to content
This repository has been archived by the owner on Jan 24, 2021. It is now read-only.

Adds charset=utf8 to a json response Content-Type #1089

Merged
merged 6 commits into from Jun 21, 2013

Conversation

albertjan
Copy link
Contributor

No description provided.

@grumpydev
Copy link
Member

Is there any downside/breaking issues with making this the default? Feels odd not making it more configurable/overridable.

@albertjan
Copy link
Contributor Author

I'm all for making it configurable. Should i put it in a static property on the JsonResponse? But I would make this the default though since almost all json serialisers serialise with UTF8.

@thecodejunkie
Copy link
Member

The only breaking change I can see is if there is someone that explicitly string matches against the content type in something like an after-hook

@thecodejunkie
Copy link
Member

@albertjan @grumpydev I know you guys talked a bit more about the "making it more configurable/overridable" part. Did you reach a conclusions?

@prabirshrestha
Copy link
Contributor

It could be a part of JsonSettings.

https://github.com/NancyFx/Nancy/blob/master/src/Nancy/Json/JsonSettings.cs

public static string DefaultCharset { get; set; }

Jsonp call could also use this. https://github.com/NancyFx/Nancy/blob/master/src/Nancy/Jsonp.cs#L59
Currently it uses the following code context.Response.ContentType = "application/javascript"; or add JsonpSettings.

@albertjan
Copy link
Contributor Author

@thecodejunkie, @grumpydev you concur with @prabirshrestha sugestion?

@albertjan
Copy link
Contributor Author

Will do some reformatting tomorrow at work "Xamarin Studio" doesn't play nice.

@grumpydev
Copy link
Member

Some weird formatting and spacing issues in JsonResponse.cs (brackets look out of line, spaces before opening method parens)

@thecodejunkie
Copy link
Member

@albertjan This is breaking on mono, see the Travis CI log please. we'd like to get this into 0.18 so the sooner the better :neckbeard:

@albertjan
Copy link
Contributor Author

@thecodejunkie no there isn't :) @grumpydev told me this test is a rogue test that will sometimes break on mono builds.

grumpydev added a commit that referenced this pull request Jun 21, 2013
Adds charset=utf8 to a json response Content-Type
@grumpydev grumpydev merged commit 83688d5 into NancyFx:master Jun 21, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants