Permalink
Browse files

Do note remove `Content-Type` when `render :body`

`render :body` should just not set the `Content-Type` header. By
removing the header, it breaks the compatibility with other parts.

After this commit, `render :body` will returns `text/html` content type,
sets by default from `ActionDispatch::Response`, and it will preserve
the overridden content type if you override it.

Fixes #14197, #14238

This partially reverts commit 3047376.
  • Loading branch information...
1 parent 332a359 commit c3bca7347675b73cef5564ef51cca7fe4b490a29 @sikachu sikachu committed Mar 1, 2014
@@ -5,8 +5,8 @@ module ActionController
module RackDelegation
extend ActiveSupport::Concern
- delegate :headers, :status=, :location=, :content_type=, :no_content_type=,
- :status, :location, :content_type, :no_content_type, :to => "@_response"
+ delegate :headers, :status=, :location=, :content_type=,
+ :status, :location, :content_type, :to => "@_response"
def dispatch(action, request)
set_response!(request)
@@ -45,9 +45,7 @@ def _render_in_priorities(options)
def _process_format(format, options = {})
super
- if options[:body]
- self.headers.delete "Content-Type"
- elsif options[:plain]
+ if options[:plain]
self.content_type = Mime::TEXT
else
self.content_type ||= format.to_s
@@ -63,8 +63,6 @@ class Response
# content you're giving them, so we need to send that along.
attr_accessor :charset
- attr_accessor :no_content_type # :nodoc:
-
CONTENT_TYPE = "Content-Type".freeze
SET_COOKIE = "Set-Cookie".freeze
LOCATION = "Location".freeze
@@ -305,17 +303,8 @@ def append_charset?
!@sending_file && @charset != false
end
- def remove_content_type!
- headers.delete CONTENT_TYPE
- end
-
def rack_response(status, header)
- if no_content_type
- remove_content_type!
- else
- assign_default_content_type_and_charset!(header)
- end
-
+ assign_default_content_type_and_charset!(header)
handle_conditional_get!
header[SET_COOKIE] = header[SET_COOKIE].join("\n") if header[SET_COOKIE].respond_to?(:join)
@@ -65,6 +65,11 @@ def with_custom_layout
render body: "hello world", layout: "greetings"
end
+ def with_custom_content_type
+ response.headers['Content-Type'] = 'application/json'
+ render body: '["troll","face"]'
+ end
+
def with_ivar_in_layout
@ivar = "hello world"
render body: "hello world", layout: "ivar"
@@ -141,6 +146,13 @@ class RenderBodyTest < Rack::TestCase
assert_status 200
end
+ test "specified content type should not be removed" do
+ get "/render_body/with_layout/with_custom_content_type"
+
+ assert_equal %w{ troll face }, JSON.parse(response.body)
+ assert_equal 'application/json', response.headers['Content-Type']
+ end
+
test "rendering body with layout: false" do
get "/render_body/with_layout/with_layout_false"
@@ -154,22 +166,5 @@ class RenderBodyTest < Rack::TestCase
assert_body "hello world"
assert_status 200
end
-
- test "rendering from minimal controller returns response with no content type" do
- get "/render_body/minimal/index"
-
- assert_header_no_content_type
- end
-
- test "rendering from normal controller returns response with no content type" do
- get "/render_body/simple/index"
-
- assert_header_no_content_type
- end
-
- def assert_header_no_content_type
- assert_not response.headers.has_key?("Content-Type"),
- %(Expect response not to have Content-Type header, got "#{response.headers["Content-Type"]}")
- end
end
end
@@ -235,14 +235,6 @@ def test_response_body_encoding
assert_equal @response.body, body.each.to_a.join
end
end
-
- test "does not add default content-type if Content-Type is none" do
- resp = ActionDispatch::Response.new.tap { |response|
- response.no_content_type = true
- }
-
- assert_not resp.headers.has_key?('Content-Type')
- end
end
class ResponseIntegrationTest < ActionDispatch::IntegrationTest
@@ -17,8 +17,9 @@ module RenderingHelper
# * <tt>:html</tt> - Renders the html safe string passed in out, otherwise
# performs html escape on the string first. Setting the content type as
# <tt>text/html</tt>.
- # * <tt>:body</tt> - Renders the text passed in, and does not set content
- # type in the response.
+ # * <tt>:body</tt> - Renders the text passed in, and inherits the content
+ # type of <tt>text/html</tt> from <tt>ActionDispatch::Response</tt>
+ # object.
#
# If no options hash is passed or :update specified, the default is to render a partial and use the second parameter
# as the locals hash.
@@ -102,11 +102,6 @@ def _render_template(options) #:nodoc:
# Assign the rendered format to lookup context.
def _process_format(format, options = {}) #:nodoc:
super
-
- if options[:body]
- self.no_content_type = true
- end
-
lookup_context.formats = [format.to_sym]
lookup_context.rendered_format = lookup_context.formats.first
end
@@ -304,10 +304,13 @@ type, by using the `:body` option to `render`:
render body: "raw"
```
-TIP: This option should be used only if you explicitly want the content type to
-be unset. Using `:plain` or `:html` might be more appropriate in most of the
+TIP: This option should be used only if you don't care about the content type of
+the response. Using `:plain` or `:html` might be more appropriate in most of the
time.
+NOTE: Unless overriden, your response returned from this render option will be
+`text/html`, as that is the default content type of Action Dispatch response.
+
#### Options for `render`
Calls to the `render` method generally accept four options:

0 comments on commit c3bca73

Please sign in to comment.