Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #9564 (closed defect: fixed)

Opened 1 year ago

Last modified 1 year ago

[PATCH] Default layouts do not take the format into account

Reported by: david Assigned to: core
Priority: normal Milestone:
Component: ActionPack Version: edge
Severity: normal Keywords: rails2
Cc: lifofifo

Description

Consider this setup:

class CommentsController < ApplicationController
  def index
    @comments = Comment.find(:all)
    
    respond_to do |format|
      format.html
      format.iphone { render :content_type => Mime::HTML }
    end
  end
end

If app/views/layouts/comments.html.erb exists, then it's assumed that it'll work for both formats by the layout picking algorithm. But of course it won't, so when you hit the iphone format, you'll get an error from assert_existence_of_template_file(layout).

The problem is that the default_layout is set at inheritance time (by inherited_with_layout) to be format indifferent.Thus, if any layout exists for this action, regardless of the format, it's assumed that all formats for this action will be able to use that layout. Which is not the case.

So we need to change the way default_layout is set and handled to consider the layout. Examine the format of the layout template and only assign the default layout to formats that match that.

Attachments

default_layouts_should_respect_format.patch (2.8 kB) - added by lifofifo on 09/17/07 23:44:56.
Make tests a bit clearer
default_layouts_with_register_alias.patch (1.9 kB) - added by lifofifo on 09/25/07 14:39:15.
render_proper_template.patch (2.8 kB) - added by lifofifo on 09/25/07 16:21:34.
use_proper_layouts.patch (8.4 kB) - added by lifofifo on 09/27/07 21:56:27.

Change History

09/17/07 23:44:56 changed by lifofifo

  • attachment default_layouts_should_respect_format.patch added.

Make tests a bit clearer

09/18/07 09:48:22 changed by manfred

+1, applies and tests run. Code looks good.

09/18/07 21:24:10 changed by josh

+1

09/18/07 23:11:50 changed by david

  • status changed from new to closed.
  • resolution set to fixed.

Fixed by [7514].

09/24/07 23:45:46 changed by david

  • status changed from closed to reopened.
  • resolution deleted.

Updated the tests to deal with the new process of using register_alias and get these failures:

  1) Error:
test_format_with_custom_response_type(MimeControllerTest):
ActionController::MissingTemplate: Missing template respond_to/iphone_with_html_response_type.iphone.erb in view path ./test/controller/../fixtures/
    ./test/controller/../../lib/action_controller/base.rb:1212:in `assert_existence_of_template_file'
    ./test/controller/../../lib/action_controller/base.rb:1045:in `render_for_file'
    ./test/controller/../../lib/action_controller/base.rb:789:in `render_with_no_layout'
    ./test/controller/../../lib/action_controller/layout.rb:247:in `render_without_benchmark'
    ./test/controller/../../lib/action_controller/benchmarking.rb:51:in `render'
    /usr/local/lib/ruby/1.8/benchmark.rb:293:in `measure'
    ./test/controller/../../lib/action_controller/benchmarking.rb:51:in `render'
    ./test/controller/../../lib/action_controller/base.rb:1102:in `perform_action_without_filters'
    ./test/controller/../../lib/action_controller/filters.rb:695:in `call_filters'
    ./test/controller/../../lib/action_controller/filters.rb:687:in `perform_action_without_benchmark'
    ./test/controller/../../lib/action_controller/benchmarking.rb:68:in `perform_action_without_rescue'
    /usr/local/lib/ruby/1.8/benchmark.rb:293:in `measure'
    ./test/controller/../../lib/action_controller/benchmarking.rb:68:in `perform_action_without_rescue'
    ./test/controller/../../lib/action_controller/rescue.rb:175:in `perform_action'
    ./test/controller/../../lib/action_controller/base.rb:509:in `send'
    ./test/controller/../../lib/action_controller/base.rb:509:in `process_without_filters'
    ./test/controller/../../lib/action_controller/filters.rb:683:in `process_without_session_management_support'
    ./test/controller/../../lib/action_controller/session_management.rb:122:in `process_without_test'
    ./test/controller/../../lib/action_controller/test_process.rb:15:in `process'
    ./test/controller/../../lib/action_controller/test_process.rb:399:in `process'
    ./test/controller/../../lib/action_controller/test_process.rb:370:in `get'
    ./test/controller/mime_responds_test.rb:400:in `test_format_with_custom_response_type'
    /usr/local/lib/ruby/gems/1.8/gems/mocha-0.5.5/lib/mocha/test_case_adapter.rb:19:in `__send__'
    /usr/local/lib/ruby/gems/1.8/gems/mocha-0.5.5/lib/mocha/test_case_adapter.rb:19:in `run'

  2) Failure:
test_format_with_custom_response_type_and_request_headers(MimeControllerTest)
    [./test/controller/mime_responds_test.rb:408:in `test_format_with_custom_response_type_and_request_headers'
     /usr/local/lib/ruby/gems/1.8/gems/mocha-0.5.5/lib/mocha/test_case_adapter.rb:19:in `__send__'
     /usr/local/lib/ruby/gems/1.8/gems/mocha-0.5.5/lib/mocha/test_case_adapter.rb:19:in `run']:
<"<html>Hello future from iPhone!</html>"> expected but was
<" ">.

09/25/07 14:39:15 changed by lifofifo

  • attachment default_layouts_with_register_alias.patch added.

09/25/07 14:39:47 changed by lifofifo

  • cc set to lifofifo.

09/25/07 16:21:34 changed by lifofifo

  • attachment render_proper_template.patch added.

09/26/07 12:32:48 changed by mislav

+1

09/26/07 13:41:50 changed by mislav

  • summary changed from Default layouts do not take the format into account to [PATCH] Default layouts do not take the format into account.

I take back that +1 after the discussion on #rails-contrib. Lifo is not sure about the patch, either

09/27/07 21:56:27 changed by lifofifo

  • attachment use_proper_layouts.patch added.

09/27/07 22:06:49 changed by josh

+1

09/28/07 01:23:24 changed by david

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [7661]) Fixed the layout defaults (closes #9564) [lifo]