This project is archived and is in readonly mode.
Fix layout handling on AJAX requests
Reported by Matt Jones | February 23rd, 2009 @ 11:30 AM
(For background, see #1844)
The patch for #1844 makes it impossible to have any layout on an AJAX response, even if you're returning HTML.
The attached patch fixes the same bug as #1844 and enables HTML AJAX responses to be handled correctly.
The key part is in candidate_for_layout, where I had to borrow some of 2.2.2's behavior to get HTML layouts to work correctly:
template_object = self.view_paths.find_template(template, default_template_format)
# this restores the behavior from 2.2.2, where response.template.template_format was reset
# to :html for :js requests with a matching html template.
# see v2.2.2, ActionView::Base, lines 328-330
@real_format = :html if response.template.template_format == :js && template_object.format == "html"
!template_object.exempt_from_layout?
The v2.2.2 code from _pick_template (in ActionView::Base) is:
# from lines 328-330, action_view/base.rb (_pick_template)
elsif template_format == :js && template = self.view_paths["#{template_file_name}.html"]
@template_format = :html
template
In the old code, this carried forward into active_layout and friends. The other notable change in this patch is in find_layout, where a third argument to find_template is used to turn off the HTML fallback (the primary cause of #1844). It should also be noted that find_layout swallows MissingTemplate exceptions for non-HTML types; without that, an action trying to return a .js file without layout would fail, since layouts/application.js.erb typically doesn't exist.
Comments and changes to this ticket
-
DHH February 23rd, 2009 @ 11:32 AM
- Assigned user set to josh
- Milestone cleared.
-
Repository February 24th, 2009 @ 04:42 PM
- State changed from new to resolved
(from [b35562f432d0807517a614a540026c2fc557e2ac]) correctly handle layouts for AJAX requests and regular js files [#2052 state:resolved]
Signed-off-by: Joshua Peek josh@joshpeek.com http://github.com/rails/rails/co...
-
jay March 5th, 2009 @ 09:30 PM
I think this commit broke some previously working code. We have something similar to the following in an action:
if request.xhr? render :template => 'some_template', :layout => 'some_layout' end
Where some_template and some_layout are both html.erb files.
From what I can tell, when ActionView::PathSet#find_template gets called while looking for the layout, the format argument gets set to :html when using the proc described in one of the comments in ticket #1844, while the above code sets the argument to :js. (Both during XHR requests, of course.)
Having the html_fallback argument set to false in ActionController::Layout#find_layout causes the above code to break and not find the appropriate HTML layout, but allows the proc-based layout to go through successfully.
Setting the html_fallback argument to true in find_layout seems to allow both layout methods to work and doesn't break any tests, so would there be any reason not to allow the HTML fallback in cases where the format is :js?
-
Matt Jones March 5th, 2009 @ 10:24 PM
- State changed from resolved to open
@jay - good catch. The breaking test case should have been the one from #1844, but it appears that I messed that one up. It's necessary to turn off html fallback to get correct results in #1844's case:
- :js request
- js template
- regular layouts/application.html.erb
Before this patch (or #1844's), that would wrap the js template in layouts/application.html.erb. Obviously, not a desired behavior...
However, I'm noticing that the explicit layout case doesn't hit candidate_for_layout?, which explains why your example is failing.
Re-opening until I can get a patch together, hopefully later today.
-
Matt Jones March 6th, 2009 @ 01:03 AM
The attached patch includes a test case for jay's scenario, and also makes the test from #1844 actually test what it says. (I'd broken it in the last patch)
The major change is that pick_layout now does html fallback if the layout is specified explicitly. It seems reasonable to expect that a user explicitly specifying a layout knows what they're doing.
There's also a test for xhr requests and render :text with :layout => true; it took me forever to figure out how that worked, and there wasn't an explicit test of what happens. FYI: that case is why the :layout => true case of pick_layout calls candidate_for_layout with a different set of options.
-
Repository March 7th, 2009 @ 07:33 PM
- State changed from open to resolved
(from [87e8b162463f13bd50d27398f020769460a770e3]) fix HTML fallback for explicit templates [#2052 state:resolved]
Signed-off-by: Joshua Peek josh@joshpeek.com http://github.com/rails/rails/co...
-
Repository April 13th, 2009 @ 11:58 PM
(from [906aebceedb95d8caa6db6314bc90f605bdfaf2b]) Bring abstract_controller up to date with rails/master
Resolved all the conflicts since 2.3.0 -> HEAD. Following is a list of commits that could not be applied cleanly or are obviated with the abstract_controller refactor. They all need to be revisited to ensure that fixes made in 2.3 do not reappear in 3.0:
2259ecf368e6a6715966f69216e3ee86bf1a82a7 AR not available * This will be reimplemented with ActionORM or equivalent
06182ea02e92afad579998aa80144588e8865ac3 implicitly rendering a js response should not use the default layout [#1844 state:resolved] * This will be handled generically
893e9eb99504705419ad6edac14d00e71cef5f12 Improve view rendering performance in development mode and reinstate template recompiling in production [#1909 state:resolved] * We will need to reimplement rails-dev-boost on top of the refactor;
the changes here are very implementation specific and cannot be cleanly applied. The following commits are implicated: 199e750d46c04970b5e7684998d09405648ecbd4 3942cb406e1d5db0ac00e03153809cc8dc4cc4db f8ea9f85d4f1e3e6f3b5d895bef6b013aa4b0690 e3b166aab37ddc2fbab030b146eb61713b91bf55 ae9f258e03c9fd5088da12c1c6cd216cc89a01f7 44423126c6f6133a1d9cf1d0832b527e8711d40f
0cb020b4d6d838025859bd60fb8151c8e21b8e84 workaround for picking layouts based on wrong view_paths [#1974 state:resolved] * The specifics of this commit no longer apply. Since it is a two-line
commit, we will reimplement this change.
8c5cc66a831aadb159f3daaffa4208064c30af0e make action_controller/layouts pick templates from the current instance's view_paths instead of the class view_paths [#1974 state:resolved] * This does not apply at all. It should be trivial to apply the feature
to the reimplemented ActionController::Base.
87e8b162463f13bd50d27398f020769460a770e3 fix HTML fallback for explicit templates [#2052 state:resolved] * There were a number of patches related to this that simply compounded
each other. Basically none of them apply cleanly, and the underlying issue needs to be revisited. After discussing the underlying problem with Koz, we will defer these fixes for further discussion.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Tags
Referenced by
- 2052 Fix layout handling on AJAX requests (from [b35562f432d0807517a614a540026c2fc557e2ac]) correct...
- 2052 Fix layout handling on AJAX requests (from [87e8b162463f13bd50d27398f020769460a770e3]) fix HTM...
- 1844 Render behaviour problem 87e8b162463f13bd50d27398f020769460a770e3 fix HTML fallbac...
- 1909 Improve view performance in development and reinstate template reloading in production 87e8b162463f13bd50d27398f020769460a770e3 fix HTML fallbac...
- 1974 Layout picked from wrong view_paths? 87e8b162463f13bd50d27398f020769460a770e3 fix HTML fallbac...
- 1974 Layout picked from wrong view_paths? 87e8b162463f13bd50d27398f020769460a770e3 fix HTML fallbac...
- 2052 Fix layout handling on AJAX requests 87e8b162463f13bd50d27398f020769460a770e3 fix HTML fallbac...