From 6c2d974e152850b533dafc6654d0df7bddfbd4bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 9 May 2010 12:52:26 +0300 Subject: Use annoted source code in Template:Error to avoid special cases in the show exceptions middleware. --- actionpack/lib/action_dispatch/middleware/show_exceptions.rb | 12 ++++-------- actionpack/lib/action_view/template/error.rb | 5 ++--- actionpack/test/template/render_test.rb | 1 + 3 files changed, 7 insertions(+), 11 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 12a93d6a24..2dd2ec9fe9 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -133,14 +133,10 @@ module ActionDispatch return unless logger ActiveSupport::Deprecation.silence do - if ActionView::Template::Error === exception - logger.fatal(exception.to_s) - else - logger.fatal( - "\n#{exception.class} (#{exception.message}):\n " + - clean_backtrace(exception).join("\n ") + "\n\n" - ) - end + message = "\n#{exception.class} (#{exception.message}):\n" + message << exception.annoted_source_code if exception.respond_to?(:annoted_source_code) + message << exception.backtrace.join("\n ") + logger.fatal("#{message}\n\n") end end diff --git a/actionpack/lib/action_view/template/error.rb b/actionpack/lib/action_view/template/error.rb index a947d746e3..6866eabf77 100644 --- a/actionpack/lib/action_view/template/error.rb +++ b/actionpack/lib/action_view/template/error.rb @@ -84,9 +84,8 @@ module ActionView end end - def to_s - "\n#{self.class} (#{message}) #{source_location}:\n\n" + - "#{source_extract(4)}\n #{backtrace.join("\n ")}\n\n" + def annoted_source_code + source_extract(4) end private diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index c9a50da418..d0212024ae 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -111,6 +111,7 @@ module RenderTestCases assert_match %r!method.*doesnt_exist!, e.message assert_equal "", e.sub_template_message assert_equal "1", e.line_number + assert_equal "1: <%= doesnt_exist %>", e.annoted_source_code.strip assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name end -- cgit v1.2.3 From 446b0ffe1c804f4925867d785b9709d21105c707 Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Sun, 9 May 2010 17:14:48 +0300 Subject: corrected error message in session/cookie_store [#4546 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_dispatch/middleware/session/cookie_store.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index 88ba941676..7114f42003 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -177,7 +177,7 @@ module ActionDispatch if key.blank? raise ArgumentError, 'A key is required to write a ' + 'cookie containing the session data. Use ' + - 'config.action_controller.session_store :cookie_store, { :key => ' + + 'config.session_store :cookie_store, { :key => ' + '"_myapp_session" } in config/application.rb' end end -- cgit v1.2.3 From 6e69b42b21d8e76c4d87b6fbc4222f55d3b11a06 Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Tue, 20 Apr 2010 23:16:18 -0500 Subject: Let label helpers accept blocks. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_view/helpers/form_helper.rb | 45 ++++++++++++++-------- .../lib/action_view/helpers/form_tag_helper.rb | 13 +++++-- actionpack/test/template/form_helper_test.rb | 10 +++-- actionpack/test/template/form_tag_helper_test.rb | 9 +++++ 4 files changed, 56 insertions(+), 21 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 6e26ae6c29..557f8454be 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -573,8 +573,19 @@ module ActionView # label(:post, :privacy, "Public Post", :value => "public") # # => # - def label(object_name, method, text = nil, options = {}) - InstanceTag.new(object_name, method, self, options.delete(:object)).to_label_tag(text, options) + # label(:post, :terms) do + # 'Accept Terms.' + # end + def label(object_name, method, content_or_options_with_block = nil, options = nil, &block) + if block_given? + options = content_or_options_with_block if content_or_options_with_block.is_a?(Hash) + text = nil + else + text = content_or_options_with_block + end + + options ||= {} + InstanceTag.new(object_name, method, self, options.delete(:object)).to_label_tag(text, options, &block) end # Returns an input tag of the "text" type tailored for accessing a specified attribute (identified by +method+) on an object @@ -823,7 +834,7 @@ module ActionView module InstanceTagMethods #:nodoc: extend ActiveSupport::Concern - include Helpers::TagHelper, Helpers::FormTagHelper + include Helpers::CaptureHelper, Context, Helpers::TagHelper, Helpers::FormTagHelper attr_reader :method_name, :object_name @@ -844,7 +855,7 @@ module ActionView end end - def to_label_tag(text = nil, options = {}) + def to_label_tag(text = nil, options = {}, &block) options = options.stringify_keys tag_value = options.delete("value") name_and_id = options.dup @@ -853,19 +864,23 @@ module ActionView options.delete("index") options["for"] ||= name_and_id["id"] - content = if text.blank? - I18n.t("helpers.label.#{object_name}.#{method_name}", :default => "").presence + if block_given? + label_tag(name_and_id["id"], options, &block) else - text.to_s - end + content = if text.blank? + I18n.t("helpers.label.#{object_name}.#{method_name}", :default => "").presence + else + text.to_s + end - content ||= if object && object.class.respond_to?(:human_attribute_name) - object.class.human_attribute_name(method_name) - end + content ||= if object && object.class.respond_to?(:human_attribute_name) + object.class.human_attribute_name(method_name) + end - content ||= method_name.humanize + content ||= method_name.humanize - label_tag(name_and_id["id"], content, options) + label_tag(name_and_id["id"], content, options) + end end def to_input_field_tag(field_type, options = {}) @@ -1137,8 +1152,8 @@ module ActionView @template.fields_for(name, *args, &block) end - def label(method, text = nil, options = {}) - @template.label(@object_name, method, text, objectify_options(options)) + def label(method, text = nil, options = {}, &block) + @template.label(@object_name, method, text, objectify_options(options), &block) end def check_box(method, options = {}, checked_value = "1", unchecked_value = "0") diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb index b840f77fb5..9d15805d46 100644 --- a/actionpack/lib/action_view/helpers/form_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb @@ -142,7 +142,7 @@ module ActionView tag :input, { "type" => "text", "name" => name, "id" => sanitize_to_id(name), "value" => value }.update(options.stringify_keys) end - # Creates a label field + # Creates a label element. Accepts a block. # # ==== Options # * Creates standard HTML attributes for the tag. @@ -156,8 +156,15 @@ module ActionView # # label_tag 'name', nil, :class => 'small_label' # # => - def label_tag(name, text = nil, options = {}) - content_tag :label, text || name.to_s.humanize, { "for" => sanitize_to_id(name) }.update(options.stringify_keys) + def label_tag(name = nil, content_or_options_with_block = nil, options = nil, &block) + if block_given? + options = content_or_options_with_block if content_or_options_with_block.is_a?(Hash) + end + + options ||= {} + options.stringify_keys! + options["for"] = sanitize_to_id(name) unless name.blank? || options.has_key?("for") + content_tag :label, content_or_options_with_block || name.to_s.humanize, options, &block end # Creates a hidden form input field used to transmit data that would be lost due to HTTP's statelessness or diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index 2234fbece9..da2adc9869 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -139,6 +139,10 @@ class FormHelperTest < ActionView::TestCase assert_dom_equal('', label("post", "title", "The title goes here", :value => "great title")) end + def test_label_with_block + assert_dom_equal('', label(:post, :title) { "The title, please:" }) + end + def test_text_field assert_dom_equal( '', text_field("post", "title") @@ -448,7 +452,7 @@ class FormHelperTest < ActionView::TestCase def test_form_for assert_deprecated do form_for(:post, @post, :html => { :id => 'create-post' }) do |f| - concat f.label(:title) + concat f.label(:title) { "The Title" } concat f.text_field(:title) concat f.text_area(:body) concat f.check_box(:secret) @@ -458,7 +462,7 @@ class FormHelperTest < ActionView::TestCase expected = "
" + - "" + + "" + "" + "" + "" + @@ -485,7 +489,7 @@ class FormHelperTest < ActionView::TestCase "" + "" + "" + - "" + + "" + "
" assert_dom_equal expected, output_buffer diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb index abb0e1df93..1e116c041f 100644 --- a/actionpack/test/template/form_tag_helper_test.rb +++ b/actionpack/test/template/form_tag_helper_test.rb @@ -288,6 +288,15 @@ class FormTagHelperTest < ActionView::TestCase assert_match VALID_HTML_ID, label_elem['for'] end + def test_label_tag_with_block + assert_dom_equal('', label_tag { "Blocked" }) + end + + def test_label_tag_with_block_and_argument + output = label_tag("clock") { "Grandfather" } + assert_dom_equal('', output) + end + def test_boolean_options assert_dom_equal %(), check_box_tag("admin", 1, true, 'disabled' => true, :readonly => "yes") assert_dom_equal %(), check_box_tag("admin", 1, true, :disabled => false, :readonly => nil) -- cgit v1.2.3 From d18a2742e01d195eb2d228207062aff49f7eb854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 15 May 2010 09:08:40 +0200 Subject: Improve previous patch a bit [#3645 state:resolved] --- actionpack/lib/action_view/helpers/form_helper.rb | 6 +++--- actionpack/lib/action_view/helpers/form_tag_helper.rb | 9 +++------ actionpack/test/template/form_tag_helper_test.rb | 5 +++++ 3 files changed, 11 insertions(+), 9 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 557f8454be..932e9e2f95 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -576,12 +576,12 @@ module ActionView # label(:post, :terms) do # 'Accept Terms.' # end - def label(object_name, method, content_or_options_with_block = nil, options = nil, &block) + def label(object_name, method, content_or_options = nil, options = nil, &block) if block_given? - options = content_or_options_with_block if content_or_options_with_block.is_a?(Hash) + options = content_or_options if content_or_options.is_a?(Hash) text = nil else - text = content_or_options_with_block + text = content_or_options end options ||= {} diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb index 9d15805d46..2a3f826c15 100644 --- a/actionpack/lib/action_view/helpers/form_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb @@ -156,15 +156,12 @@ module ActionView # # label_tag 'name', nil, :class => 'small_label' # # => - def label_tag(name = nil, content_or_options_with_block = nil, options = nil, &block) - if block_given? - options = content_or_options_with_block if content_or_options_with_block.is_a?(Hash) - end - + def label_tag(name = nil, content_or_options = nil, options = nil, &block) + options = content_or_options if block_given? && content_or_options.is_a?(Hash) options ||= {} options.stringify_keys! options["for"] = sanitize_to_id(name) unless name.blank? || options.has_key?("for") - content_tag :label, content_or_options_with_block || name.to_s.humanize, options, &block + content_tag :label, content_or_options || name.to_s.humanize, options, &block end # Creates a hidden form input field used to transmit data that would be lost due to HTTP's statelessness or diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb index 1e116c041f..1c095b621e 100644 --- a/actionpack/test/template/form_tag_helper_test.rb +++ b/actionpack/test/template/form_tag_helper_test.rb @@ -297,6 +297,11 @@ class FormTagHelperTest < ActionView::TestCase assert_dom_equal('', output) end + def test_label_tag_with_block_and_argument_and_options + output = label_tag("clock", :id => "label_clock") { "Grandfather" } + assert_dom_equal('', output) + end + def test_boolean_options assert_dom_equal %(), check_box_tag("admin", 1, true, 'disabled' => true, :readonly => "yes") assert_dom_equal %(), check_box_tag("admin", 1, true, :disabled => false, :readonly => nil) -- cgit v1.2.3 From 9869ee77cdbf8f0e9c7eb6e33738d0aa8f5dc70a Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Sat, 15 May 2010 09:42:02 +0200 Subject: Accept :alt => nil on image_tag [#4558 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_view/helpers/asset_tag_helper.rb | 2 +- actionpack/test/template/asset_tag_helper_test.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 8731ed0ef3..e1fbc118d5 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -620,7 +620,7 @@ module ActionView options.symbolize_keys! src = options[:src] = path_to_image(source) - options[:alt] ||= File.basename(src, '.*').capitalize + options[:alt] = options.fetch(:alt){ File.basename(src, '.*').capitalize } if size = options.delete(:size) options[:width], options[:height] = size.split("x") if size =~ %r{^\d+x\d+$} diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 124bf734ac..b6a6f52876 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -154,7 +154,8 @@ class AssetTagHelperTest < ActionView::TestCase %(image_tag(".pdf.png")) => %(.pdf), %(image_tag("http://www.rubyonrails.com/images/rails.png")) => %(Rails), %(image_tag("mouse.png", :mouseover => "/images/mouse_over.png")) => %(Mouse), - %(image_tag("mouse.png", :mouseover => image_path("mouse_over.png"))) => %(Mouse) + %(image_tag("mouse.png", :mouseover => image_path("mouse_over.png"))) => %(Mouse), + %(image_tag("mouse.png", :alt => nil)) => %() } FaviconLinkToTag = { -- cgit v1.2.3 From cdf700147c5b3ec5e4d1e7de1c2d08134568c2d1 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Sat, 15 May 2010 13:26:33 +0200 Subject: fix assert_select messages to its declaration behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- .../lib/action_dispatch/testing/assertions/selector.rb | 16 +++++++++++----- actionpack/test/controller/assert_select_test.rb | 16 +++++++++++++--- 2 files changed, 24 insertions(+), 8 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/testing/assertions/selector.rb b/actionpack/lib/action_dispatch/testing/assertions/selector.rb index a6b1126e2b..9deabf5b3c 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/selector.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/selector.rb @@ -298,10 +298,14 @@ module ActionDispatch # found one but expecting two. message ||= content_mismatch if matches.empty? # Test minimum/maximum occurrence. - min, max = equals[:minimum], equals[:maximum] - message = message || %(Expected #{count_description(min, max)} matching "#{selector.to_s}", found #{matches.size}.) - assert matches.size >= min, message if min - assert matches.size <= max, message if max + min, max, count = equals[:minimum], equals[:maximum], equals[:count] + message = message || %(Expected #{count_description(min, max, count)} matching "#{selector.to_s}", found #{matches.size}.) + if count + assert matches.size == count, message + else + assert matches.size >= min, message if min + assert matches.size <= max, message if max + end # If a block is given call that block. Set @selected to allow # nested assert_select, which can be nested several levels deep. @@ -318,11 +322,13 @@ module ActionDispatch matches end - def count_description(min, max) #:nodoc: + def count_description(min, max, count) #:nodoc: pluralize = lambda {|word, quantity| word << (quantity == 1 ? '' : 's')} if min && max && (max != min) "between #{min} and #{max} elements" + elsif min && max && max == min && count + "exactly #{count} #{pluralize['element', min]}" elsif min && !(min == 1 && max == 1) "at least #{min} #{pluralize['element', min]}" elsif max diff --git a/actionpack/test/controller/assert_select_test.rb b/actionpack/test/controller/assert_select_test.rb index 4ef6fa4000..f1254abcf7 100644 --- a/actionpack/test/controller/assert_select_test.rb +++ b/actionpack/test/controller/assert_select_test.rb @@ -80,10 +80,15 @@ class AssertSelectTest < ActionController::TestCase def test_assert_select render_html %Q{
} assert_select "div", 2 - assert_failure(/Expected at least 3 elements matching \"div\", found 2/) { assert_select "div", 3 } assert_failure(/Expected at least 1 element matching \"p\", found 0/) { assert_select "p" } end + def test_equality_integer + render_html %Q{
} + assert_failure(/Expected exactly 3 elements matching \"div\", found 2/) { assert_select "div", 3 } + assert_failure(/Expected exactly 0 elements matching \"div\", found 2/) { assert_select "div", 0 } + end + def test_equality_true_false render_html %Q{
} assert_nothing_raised { assert_select "div" } @@ -94,6 +99,11 @@ class AssertSelectTest < ActionController::TestCase assert_nothing_raised { assert_select "p", false } end + def test_equality_false_message + render_html %Q{
} + assert_failure(/Expected exactly 0 elements matching \"div\", found 2/) { assert_select "div", false } + end + def test_equality_string_and_regexp render_html %Q{
foo
foo
} assert_nothing_raised { assert_select "div", "foo" } @@ -128,7 +138,7 @@ class AssertSelectTest < ActionController::TestCase def test_counts render_html %Q{
foo
foo
} assert_nothing_raised { assert_select "div", 2 } - assert_failure(/Expected at least 3 elements matching \"div\", found 2/) do + assert_failure(/Expected exactly 3 elements matching \"div\", found 2/) do assert_select "div", 3 end assert_nothing_raised { assert_select "div", 1..2 } @@ -136,7 +146,7 @@ class AssertSelectTest < ActionController::TestCase assert_select "div", 3..4 end assert_nothing_raised { assert_select "div", :count=>2 } - assert_failure(/Expected at least 3 elements matching \"div\", found 2/) do + assert_failure(/Expected exactly 3 elements matching \"div\", found 2/) do assert_select "div", :count=>3 end assert_nothing_raised { assert_select "div", :minimum=>1 } -- cgit v1.2.3 From e807476d31fd15fbf8808d739b0afada4faeece9 Mon Sep 17 00:00:00 2001 From: Martin Date: Thu, 15 Apr 2010 18:59:24 +0200 Subject: added convenience methods #notice and #alert to flash.now MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_dispatch/middleware/flash.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 99b36366d6..adde183cdb 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -49,6 +49,16 @@ module ActionDispatch def [](k) @flash[k] end + + # Convenience accessor for flash.now[:alert]= + def alert=(message) + self[:alert] = message + end + + # Convenience accessor for flash.now[:notice]= + def notice=(message) + self[:notice] = message + end end class FlashHash < Hash -- cgit v1.2.3 From 2d84f24af5cb0854a316929fa0d0720773be2162 Mon Sep 17 00:00:00 2001 From: Anil Wadghule Date: Sat, 15 May 2010 20:18:31 +0530 Subject: Add tests for convenience methods #notice and #alert to flash.now [#4369 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/test/controller/flash_test.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'actionpack') diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb index 3c651ebebc..c662ce264b 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -81,6 +81,16 @@ class FlashTest < ActionController::TestCase redirect_to '/somewhere', :notice => "Good luck in the somewheres!" end + def render_with_flash_now_alert + flash.now.alert = "Beware the nowheres now!" + render :inline => "hello" + end + + def render_with_flash_now_notice + flash.now.notice = "Good luck in the somewheres now!" + render :inline => "hello" + end + def redirect_with_other_flashes redirect_to '/wonderland', :flash => { :joyride => "Horses!" } end @@ -183,6 +193,16 @@ class FlashTest < ActionController::TestCase assert_equal "Good luck in the somewheres!", @controller.send(:flash)[:notice] end + def test_render_with_flash_now_alert + get :render_with_flash_now_alert + assert_equal "Beware the nowheres now!", @controller.send(:flash)[:alert] + end + + def test_render_with_flash_now_notice + get :render_with_flash_now_notice + assert_equal "Good luck in the somewheres now!", @controller.send(:flash)[:notice] + end + def test_redirect_to_with_other_flashes get :redirect_with_other_flashes assert_equal "Horses!", @controller.send(:flash)[:joyride] -- cgit v1.2.3 From fa99de0bd054576336c940ca78f3d1b35b6e490e Mon Sep 17 00:00:00 2001 From: Jeff Kreeftmeijer Date: Sat, 15 May 2010 13:25:48 +0200 Subject: partial counters with :as [#2804 state:resolved] Signed-off-by: Jeremy Kemper --- actionpack/lib/action_view/render/partials.rb | 1 + actionpack/test/controller/render_test.rb | 9 +++++++++ actionpack/test/fixtures/test/_customer_counter_with_as.erb | 1 + 3 files changed, 11 insertions(+) create mode 100644 actionpack/test/fixtures/test/_customer_counter_with_as.erb (limited to 'actionpack') diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 974345633c..6160a95e20 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -248,6 +248,7 @@ module ActionView @collection.each do |object| locals[counter_name] += 1 + locals["#{as.to_s}_counter".to_sym] = locals[counter_name] locals[as] = object segments << template.render(@view, locals) diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 2f3997518f..180d5e1a65 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -547,6 +547,10 @@ class TestController < ActionController::Base render :partial => "customer_counter", :collection => [ Customer.new("david"), Customer.new("mary") ] end + def partial_collection_with_as_and_counter + render :partial => "customer_counter_with_as", :collection => [ Customer.new("david"), Customer.new("mary") ], :as => :client + end + def partial_collection_with_locals render :partial => "customer_greeting", :collection => [ Customer.new("david"), Customer.new("mary") ], :locals => { :greeting => "Bonjour" } end @@ -1242,6 +1246,11 @@ class RenderTest < ActionController::TestCase assert_equal "david0mary1", @response.body end + def test_partial_collection_with_as_and_counter + get :partial_collection_with_as_and_counter + assert_equal "david0mary1", @response.body + end + def test_partial_collection_with_locals get :partial_collection_with_locals assert_equal "Bonjour: davidBonjour: mary", @response.body diff --git a/actionpack/test/fixtures/test/_customer_counter_with_as.erb b/actionpack/test/fixtures/test/_customer_counter_with_as.erb new file mode 100644 index 0000000000..1241eb604d --- /dev/null +++ b/actionpack/test/fixtures/test/_customer_counter_with_as.erb @@ -0,0 +1 @@ +<%= client.name %><%= client_counter %> \ No newline at end of file -- cgit v1.2.3 From da90fe94662b338a2430e99ff0d3298059a28a51 Mon Sep 17 00:00:00 2001 From: Jeff Kreeftmeijer Date: Sat, 15 May 2010 20:07:13 +0200 Subject: make sure `as` is set before trying to build an #{as}_counter. [#2804 state:resolved] Signed-off-by: Jeremy Kemper --- actionpack/lib/action_view/render/partials.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 6160a95e20..5d266c46bf 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -248,7 +248,7 @@ module ActionView @collection.each do |object| locals[counter_name] += 1 - locals["#{as.to_s}_counter".to_sym] = locals[counter_name] + locals["#{as.to_s}_counter".to_sym] = locals[counter_name] if as locals[as] = object segments << template.render(@view, locals) -- cgit v1.2.3 From c5537c1158c91e3244d24751b1e86290b136dc09 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 15 May 2010 11:25:56 -0700 Subject: Ruby 1.9: fix invalid rack response in test --- actionpack/test/dispatch/request/xml_params_parsing_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/test/dispatch/request/xml_params_parsing_test.rb b/actionpack/test/dispatch/request/xml_params_parsing_test.rb index f2ce2c5b93..d44c642420 100644 --- a/actionpack/test/dispatch/request/xml_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/xml_params_parsing_test.rb @@ -21,7 +21,7 @@ class XmlParamsParsingTest < ActionController::IntegrationTest def call(env) bar = env['action_dispatch.request.request_parameters']['foo'] result = "#{bar}" - [200, {"Content-Type" => "application/xml", "Content-Length" => result.length.to_s}, result] + [200, {"Content-Type" => "application/xml", "Content-Length" => result.length.to_s}, [result]] end end req = Rack::MockRequest.new(ActionDispatch::ParamsParser.new(Linted.new)) -- cgit v1.2.3 From f055bc05d515b80c89b99b775546b954f270bc5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 15 May 2010 21:55:03 +0200 Subject: Optimize the code added in fa99de0bd054576336c9 --- actionpack/lib/action_view/render/partials.rb | 28 ++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 5d266c46bf..85f67d4f14 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -241,16 +241,21 @@ module ActionView end def collection_with_template(template = @template) - segments, locals, as, template = [], @locals, @options[:as] || @template.variable_name, @template + segments, locals, template = [], @locals, @template - counter_name = template.counter_name - locals[counter_name] = -1 + if @options[:as] + as = @options[:as] + counter = "#{as}_counter".to_sym + else + as = template.variable_name + counter = template.counter_name + end + + locals[counter] = -1 @collection.each do |object| - locals[counter_name] += 1 - locals["#{as.to_s}_counter".to_sym] = locals[counter_name] if as + locals[counter] += 1 locals[as] = object - segments << template.render(@view, locals) end @@ -258,13 +263,18 @@ module ActionView end def collection_without_template(collection_paths = @collection_paths) - segments, locals, as = [], @locals, @options[:as] - index, template = -1, nil + segments, locals = [], @locals + index, template = -1, nil + + if @options[:as] + as = @options[:as] + counter = "#{as}_counter" + end @collection.each_with_index do |object, i| template = find_template(collection_paths[i]) - locals[template.counter_name] = (index += 1) locals[as || template.variable_name] = object + locals[counter || template.counter_name] = (index += 1) segments << template.render(@view, locals) end -- cgit v1.2.3 From 6617d0189377a2f820c8f948589bb2d4a91155af Mon Sep 17 00:00:00 2001 From: Jeff Dean Date: Sat, 15 May 2010 16:22:35 -0400 Subject: Sending :id => nil to form helpers now properly omits the "id" html element [#4559 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_view/helpers/form_helper.rb | 16 +++- actionpack/test/template/form_helper_test.rb | 107 ++++++++++++++++++++++ 2 files changed, 118 insertions(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 932e9e2f95..414a5d4cd9 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -859,7 +859,13 @@ module ActionView options = options.stringify_keys tag_value = options.delete("value") name_and_id = options.dup - name_and_id["id"] = name_and_id["for"] + + if name_and_id["for"] + name_and_id["id"] = name_and_id["for"] + else + name_and_id.delete("id") + end + add_default_name_and_id_for_value(tag_value, name_and_id) options.delete("index") options["for"] ||= name_and_id["id"] @@ -1027,7 +1033,7 @@ module ActionView pretty_tag_value = tag_value.to_s.gsub(/\s/, "_").gsub(/\W/, "").downcase specified_id = options["id"] add_default_name_and_id(options) - options["id"] += "_#{pretty_tag_value}" unless specified_id + options["id"] += "_#{pretty_tag_value}" if specified_id.blank? && options["id"].present? else add_default_name_and_id(options) end @@ -1036,14 +1042,14 @@ module ActionView def add_default_name_and_id(options) if options.has_key?("index") options["name"] ||= tag_name_with_index(options["index"]) - options["id"] ||= tag_id_with_index(options["index"]) + options["id"] = options.fetch("id", tag_id_with_index(options["index"])) options.delete("index") elsif defined?(@auto_index) options["name"] ||= tag_name_with_index(@auto_index) - options["id"] ||= tag_id_with_index(@auto_index) + options["id"] = options.fetch("id", tag_id_with_index(@auto_index)) else options["name"] ||= tag_name + (options.has_key?('multiple') ? '[]' : '') - options["id"] ||= tag_id + options["id"] = options.fetch("id", tag_id) end end diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index da2adc9869..3893d152a2 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -423,6 +423,90 @@ class FormHelperTest < ActionView::TestCase check_box("post", "secret", :id => "i mean it") end + def test_nil_id + assert_dom_equal( + '', text_field("post", "title", "id" => nil) + ) + assert_dom_equal( + '', + text_area("post", "body", "id" => nil) + ) + assert_dom_equal( + '', + check_box("post", "secret", "id" => nil) + ) + assert_dom_equal( + '', + radio_button("post", "secret", "0", "id" => nil) + ) + assert_dom_equal( + '', + select("post", "secret", [], {}, "id" => nil) + ) + assert_dom_equal text_field("post", "title", "id" => nil), + text_field("post", "title", :id => nil) + assert_dom_equal text_area("post", "body", "id" => nil), + text_area("post", "body", :id => nil) + assert_dom_equal check_box("post", "secret", "id" => nil), + check_box("post", "secret", :id => nil) + assert_dom_equal radio_button("post", "secret", "id" => nil), + radio_button("post", "secret", :id => nil) + end + + def test_index + assert_dom_equal( + '', + text_field("post", "title", "index" => 5) + ) + assert_dom_equal( + '', + text_area("post", "body", "index" => 5) + ) + assert_dom_equal( + '', + check_box("post", "secret", "index" => 5) + ) + assert_dom_equal( + text_field("post", "title", "index" => 5), + text_field("post", "title", "index" => 5) + ) + assert_dom_equal( + text_area("post", "body", "index" => 5), + text_area("post", "body", "index" => 5) + ) + assert_dom_equal( + check_box("post", "secret", "index" => 5), + check_box("post", "secret", "index" => 5) + ) + end + + def test_index_with_nil_id + assert_dom_equal( + '', + text_field("post", "title", "index" => 5, 'id' => nil) + ) + assert_dom_equal( + '', + text_area("post", "body", "index" => 5, 'id' => nil) + ) + assert_dom_equal( + '', + check_box("post", "secret", "index" => 5, 'id' => nil) + ) + assert_dom_equal( + text_field("post", "title", "index" => 5, 'id' => nil), + text_field("post", "title", :index => 5, :id => nil) + ) + assert_dom_equal( + text_area("post", "body", "index" => 5, 'id' => nil), + text_area("post", "body", :index => 5, :id => nil) + ) + assert_dom_equal( + check_box("post", "secret", "index" => 5, 'id' => nil), + check_box("post", "secret", :index => 5, :id => nil) + ) + end + def test_auto_index pid = @post.id assert_dom_equal( @@ -449,6 +533,29 @@ class FormHelperTest < ActionView::TestCase ) end + def test_auto_index_with_nil_id + pid = @post.id + assert_dom_equal( + "", + text_field("post[]","title", :id => nil) + ) + assert_dom_equal( + "", + text_area("post[]", "body", :id => nil) + ) + assert_dom_equal( + "", + check_box("post[]", "secret", :id => nil) + ) + assert_dom_equal( +"", + radio_button("post[]", "title", "Hello World", :id => nil) + ) + assert_dom_equal("", + radio_button("post[]", "title", "Goodbye World", :id => nil) + ) + end + def test_form_for assert_deprecated do form_for(:post, @post, :html => { :id => 'create-post' }) do |f| -- cgit v1.2.3 From fc2480a277fd4753aebf68dcf8787d24e00bd057 Mon Sep 17 00:00:00 2001 From: rohit Date: Sun, 16 May 2010 13:04:46 +0530 Subject: Fixed 1 failure and 2 errors in ActionPack testsuite [#4613 state:commited] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/test/controller/assert_select_test.rb | 6 +++--- actionpack/test/controller/render_test.rb | 4 ++-- actionpack/test/template/capture_helper_test.rb | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/assert_select_test.rb b/actionpack/test/controller/assert_select_test.rb index f1254abcf7..7012c4c9b0 100644 --- a/actionpack/test/controller/assert_select_test.rb +++ b/actionpack/test/controller/assert_select_test.rb @@ -211,7 +211,7 @@ class AssertSelectTest < ActionController::TestCase assert_nothing_raised { assert_select "div", "foo" } assert_nothing_raised { assert_select "div", "bar" } assert_nothing_raised { assert_select "div", /\w*/ } - assert_nothing_raised { assert_select "div", /\w*/, :count=>2 } + assert_nothing_raised { assert_select "div", :text => /\w*/, :count=>2 } assert_raise(Assertion) { assert_select "div", :text=>"foo", :count=>2 } assert_nothing_raised { assert_select "div", :html=>"bar" } assert_nothing_raised { assert_select "div", :html=>"bar" } @@ -276,8 +276,8 @@ class AssertSelectTest < ActionController::TestCase def test_css_select render_html %Q{
} - assert 2, css_select("div").size - assert 0, css_select("p").size + assert_equal 2, css_select("div").size + assert_equal 0, css_select("p").size end def test_nested_css_select diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 180d5e1a65..52049f2a8a 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -1388,7 +1388,7 @@ class EtagRenderTest < ActionController::TestCase def test_render_against_etag_request_should_have_no_content_length_when_match @request.if_none_match = etag_for("hello david") get :render_hello_world_from_variable - assert !@response.headers.has_key?("Content-Length"), @response.headers['Content-Length'] + assert !@response.headers.has_key?("Content-Length") end def test_render_against_etag_request_should_200_when_no_match @@ -1524,4 +1524,4 @@ class LastModifiedRenderTest < ActionController::TestCase get :conditional_hello_with_bangs assert_response :success end -end \ No newline at end of file +end diff --git a/actionpack/test/template/capture_helper_test.rb b/actionpack/test/template/capture_helper_test.rb index bf541c17d3..9f3d68a639 100644 --- a/actionpack/test/template/capture_helper_test.rb +++ b/actionpack/test/template/capture_helper_test.rb @@ -74,7 +74,7 @@ class CaptureHelperTest < ActionView::TestCase @av.output_buffer.force_encoding(alt_encoding) @av.with_output_buffer do - assert alt_encoding, @av.output_buffer.encoding + assert_equal alt_encoding, @av.output_buffer.encoding end end end -- cgit v1.2.3 From 2dc1402417242784a738321e7edd521f8ec7ac83 Mon Sep 17 00:00:00 2001 From: pleax Date: Sat, 15 May 2010 13:53:59 +0400 Subject: added support for html attributes in options_for_select [#2165] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- .../lib/action_view/helpers/form_options_helper.rb | 27 ++++++++++- .../test/template/form_options_helper_test.rb | 56 ++++++++++++++++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index 8f8db548c3..fe71d2cdf7 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -270,6 +270,15 @@ module ActionView # options_for_select([ "VISA", "MasterCard", "Discover" ], ["VISA", "Discover"]) # \n\n # + # You can optionally provide html attributes as the last element of the array. + # + # Examples: + # options_for_select([ "Denmark", ["USA", {:class=>'bold'}], "Sweden" ], ["USA", "Sweden"]) + # \n\n + # + # options_for_select([["Dollar", "$", {:class=>"bold"}], ["Kroner", "DKK", {:onclick => "alert('HI');"}]]) + # \n + # # If you wish to specify disabled option tags, set +selected+ to be a hash, with :disabled being either a value # or array of values to be disabled. In this case, you can use :selected to specify selected option tags. # @@ -291,10 +300,11 @@ module ActionView selected, disabled = extract_selected_and_disabled(selected) options_for_select = container.inject([]) do |options, element| + html_attributes = option_html_attributes(element) text, value = option_text_and_value(element) selected_attribute = ' selected="selected"' if option_value_selected?(value, selected) disabled_attribute = ' disabled="disabled"' if disabled && option_value_selected?(value, disabled) - options << %() + options << %() end options_for_select.join("\n").html_safe @@ -486,9 +496,22 @@ module ActionView end private + def option_html_attributes(element) + return "" unless Array === element + html_attributes = [] + element.select { |e| Hash === e }.reduce({}, :merge).each do |k, v| + html_attributes << " #{k}=\"#{html_escape(v.to_s)}\"" + end + html_attributes.join + end + def option_text_and_value(option) # Options are [text, value] pairs or strings used for both. - if !option.is_a?(String) and option.respond_to?(:first) and option.respond_to?(:last) + case + when Array === option + option = option.reject { |e| Hash === e } + [option.first, option.last] + when !option.is_a?(String) && option.respond_to?(:first) && option.respond_to?(:last) [option.first, option.last] else [option, option] diff --git a/actionpack/test/template/form_options_helper_test.rb b/actionpack/test/template/form_options_helper_test.rb index 98503c32fd..19b73aa810 100644 --- a/actionpack/test/template/form_options_helper_test.rb +++ b/actionpack/test/template/form_options_helper_test.rb @@ -767,6 +767,62 @@ class FormOptionsHelperTest < ActionView::TestCase html end + def test_options_for_select_with_element_attributes + assert_dom_equal( + "\n\n\n", + options_for_select([ [ "", { :class => 'bold' } ], [ "USA", { :onclick => "alert('Hello World');" } ], [ "Sweden" ], "Germany" ]) + ) + end + + def test_options_for_select_with_element_attributes_and_selection + assert_dom_equal( + "\n\n", + options_for_select([ "", [ "USA", { :class => 'bold' } ], "Sweden" ], "USA") + ) + end + + def test_options_for_select_with_element_attributes_and_selection_array + assert_dom_equal( + "\n\n", + options_for_select([ "", [ "USA", { :class => 'bold' } ], "Sweden" ], [ "USA", "Sweden" ]) + ) + end + + def test_option_html_attributes_from_without_hash + assert_dom_equal( + "", + option_html_attributes([ 'foo', 'bar' ]) + ) + end + + def test_option_html_attributes_with_single_element_hash + assert_dom_equal( + " class=\"fancy\"", + option_html_attributes([ 'foo', 'bar', { :class => 'fancy' } ]) + ) + end + + def test_option_html_attributes_with_multiple_element_hash + assert_dom_equal( + " class=\"fancy\" onclick=\"alert('Hello World');\"", + option_html_attributes([ 'foo', 'bar', { :class => 'fancy', 'onclick' => "alert('Hello World');" } ]) + ) + end + + def test_option_html_attributes_with_multiple_hashes + assert_dom_equal( + " class=\"fancy\" onclick=\"alert('Hello World');\"", + option_html_attributes([ 'foo', 'bar', { :class => 'fancy' }, { 'onclick' => "alert('Hello World');" } ]) + ) + end + + def test_option_html_attributes_with_special_characters + assert_dom_equal( + " onclick=\"alert("<code>")\"", + option_html_attributes([ 'foo', 'bar', { :onclick => %(alert("")) } ]) + ) + end + def test_grouped_collection_select @continents = [ Continent.new("", [Country.new("", ""), Country.new("so", "Somalia")] ), -- cgit v1.2.3 From 4ea48f2a9880508a4967be011345fc05570f44c4 Mon Sep 17 00:00:00 2001 From: Hussein Morsy Date: Sun, 16 May 2010 14:50:08 +0200 Subject: Fixed 1 failure in ActionPack testsuite [#4613 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/test/template/form_helper_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index 3893d152a2..d1e1338a17 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -449,8 +449,8 @@ class FormHelperTest < ActionView::TestCase text_area("post", "body", :id => nil) assert_dom_equal check_box("post", "secret", "id" => nil), check_box("post", "secret", :id => nil) - assert_dom_equal radio_button("post", "secret", "id" => nil), - radio_button("post", "secret", :id => nil) + assert_dom_equal radio_button("post", "secret", "0", "id" => nil), + radio_button("post", "secret", "0", :id => nil) end def test_index -- cgit v1.2.3 From f58bdae1f716c71202546c5a40a951b5fc54a591 Mon Sep 17 00:00:00 2001 From: Simon Jefford Date: Sun, 16 May 2010 01:22:30 +0100 Subject: Check blocks are not incorrectly detected when compiling erubis templates [#4575 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_view/template/handlers/erb.rb | 2 +- actionpack/test/controller/capture_test.rb | 5 +++++ actionpack/test/fixtures/test/proper_block_detection.erb | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 actionpack/test/fixtures/test/proper_block_detection.erb (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template/handlers/erb.rb b/actionpack/lib/action_view/template/handlers/erb.rb index 705c2bf82e..237746437a 100644 --- a/actionpack/lib/action_view/template/handlers/erb.rb +++ b/actionpack/lib/action_view/template/handlers/erb.rb @@ -28,7 +28,7 @@ module ActionView src << "@output_buffer.safe_concat('" << escape_text(text) << "');" end - BLOCK_EXPR = /(do|\{)(\s*\|[^|]*\|)?\s*\Z/ + BLOCK_EXPR = /\s+(do|\{)(\s*\|[^|]*\|)?\s*\Z/ def add_expr_literal(src, code) if code =~ BLOCK_EXPR diff --git a/actionpack/test/controller/capture_test.rb b/actionpack/test/controller/capture_test.rb index 06a5af6b32..d1dbd535c4 100644 --- a/actionpack/test/controller/capture_test.rb +++ b/actionpack/test/controller/capture_test.rb @@ -61,6 +61,11 @@ class CaptureTest < ActionController::TestCase assert_equal expected_content_for_output, @response.body end + def test_proper_block_detection + @todo = "some todo" + get :proper_block_detection + end + private def expected_content_for_output "Putting stuff in the title!\n\nGreat stuff!" diff --git a/actionpack/test/fixtures/test/proper_block_detection.erb b/actionpack/test/fixtures/test/proper_block_detection.erb new file mode 100644 index 0000000000..23564dbcee --- /dev/null +++ b/actionpack/test/fixtures/test/proper_block_detection.erb @@ -0,0 +1 @@ +<%= @todo %> -- cgit v1.2.3 From d61dbce48221b270a5e2d1d35520873c5fdd9677 Mon Sep 17 00:00:00 2001 From: Rizwan Reza Date: Sun, 16 May 2010 20:10:16 +0430 Subject: Take out stale tasks from Actionpack's Rakefile [#4619 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/Rakefile | 19 ------------------- 1 file changed, 19 deletions(-) (limited to 'actionpack') diff --git a/actionpack/Rakefile b/actionpack/Rakefile index b9ace8658a..f3bd7dfc10 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -88,23 +88,4 @@ task :lines do end puts "Total: Lines #{total_lines}, LOC #{total_codelines}" -end - -# Publishing ------------------------------------------------------ - -task :update_scriptaculous do - for js in %w( controls dragdrop effects ) - system("svn export --force http://dev.rubyonrails.org/svn/rails/spinoffs/scriptaculous/src/#{js}.js #{File.dirname(__FILE__)}/lib/action_view/helpers/javascripts/#{js}.js") - end -end - -desc "Updates actionpack to the latest version of the javascript spinoffs" -task :update_js => [ :update_scriptaculous ] - -# Publishing ------------------------------------------------------ - -desc "Publish the API documentation" -task :pdoc => [:rdoc] do - require 'rake/contrib/sshpublisher' - Rake::SshDirPublisher.new("wrath.rubyonrails.org", "public_html/ap", "doc").upload end \ No newline at end of file -- cgit v1.2.3 From af0d1a88157942c6e6398dbf73891cff1e152405 Mon Sep 17 00:00:00 2001 From: wycats Date: Sat, 15 May 2010 03:47:24 -0700 Subject: Initial work to improve the state of encodings for templates --- actionpack/lib/action_view.rb | 2 + actionpack/lib/action_view/template.rb | 6 ++ .../lib/action_view/template/handlers/erb.rb | 98 ++++++++++++---------- 3 files changed, 61 insertions(+), 45 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 5555217ee2..5e3b2ec51b 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -58,6 +58,8 @@ module ActionView end autoload :TestCase, 'action_view/test_case' + + ENCODING_FLAG = "#.*coding[:=]\s*(\S+)[ \t]*" end require 'active_support/i18n' diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index a1a970e2d2..ce249e2a96 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -27,6 +27,12 @@ module ActionView end def initialize(source, identifier, handler, details) + if source.encoding_aware? && source =~ %r{\A#{ENCODING_FLAG}} + # don't snip off the \n to preserve line numbers + source.sub!(/\A[^\n]*/, '') + source.force_encoding($1).encode + end + @source = source @identifier = identifier @handler = handler diff --git a/actionpack/lib/action_view/template/handlers/erb.rb b/actionpack/lib/action_view/template/handlers/erb.rb index 237746437a..17652d6d1f 100644 --- a/actionpack/lib/action_view/template/handlers/erb.rb +++ b/actionpack/lib/action_view/template/handlers/erb.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/class/attribute_accessors' require 'active_support/core_ext/string/output_safety' +require "action_view/template" require 'erubis' module ActionView @@ -17,65 +18,72 @@ module ActionView end end - module Template::Handlers - class Erubis < ::Erubis::Eruby - def add_preamble(src) - src << "@output_buffer = ActionView::OutputBuffer.new;" - end + class Template + module Handlers + class Erubis < ::Erubis::Eruby + def add_preamble(src) + src << "@output_buffer = ActionView::OutputBuffer.new;" + end - def add_text(src, text) - return if text.empty? - src << "@output_buffer.safe_concat('" << escape_text(text) << "');" - end + def add_text(src, text) + return if text.empty? + src << "@output_buffer.safe_concat('" << escape_text(text) << "');" + end - BLOCK_EXPR = /\s+(do|\{)(\s*\|[^|]*\|)?\s*\Z/ + BLOCK_EXPR = /\s+(do|\{)(\s*\|[^|]*\|)?\s*\Z/ - def add_expr_literal(src, code) - if code =~ BLOCK_EXPR - src << '@output_buffer.append= ' << code - else - src << '@output_buffer.append= (' << code << ');' + def add_expr_literal(src, code) + if code =~ BLOCK_EXPR + src << '@output_buffer.append= ' << code + else + src << '@output_buffer.append= (' << code << ');' + end end - end - def add_stmt(src, code) - if code =~ BLOCK_EXPR - src << '@output_buffer.append_if_string= ' << code - else - super + def add_stmt(src, code) + if code =~ BLOCK_EXPR + src << '@output_buffer.append_if_string= ' << code + else + super + end end - end - def add_expr_escaped(src, code) - src << '@output_buffer.append= ' << escaped_expr(code) << ';' - end + def add_expr_escaped(src, code) + src << '@output_buffer.append= ' << escaped_expr(code) << ';' + end - def add_postamble(src) - src << '@output_buffer.to_s' + def add_postamble(src) + src << '@output_buffer.to_s' + end end - end - class ERB < Template::Handler - include Compilable + class ERB < Handler + include Compilable - ## - # :singleton-method: - # Specify trim mode for the ERB compiler. Defaults to '-'. - # See ERb documentation for suitable values. - cattr_accessor :erb_trim_mode - self.erb_trim_mode = '-' + ## + # :singleton-method: + # Specify trim mode for the ERB compiler. Defaults to '-'. + # See ERb documentation for suitable values. + cattr_accessor :erb_trim_mode + self.erb_trim_mode = '-' - self.default_format = Mime::HTML + self.default_format = Mime::HTML - cattr_accessor :erb_implementation - self.erb_implementation = Erubis + cattr_accessor :erb_implementation + self.erb_implementation = Erubis - def compile(template) - source = template.source.gsub(/\A(<%(#.*coding[:=]\s*(\S+)\s*)-?%>)\s*\n?/, '') - erb = "<% __in_erb_template=true %>#{source}" - result = self.class.erb_implementation.new(erb, :trim=>(self.class.erb_trim_mode == "-")).src - result = "#{$2}\n#{result}" if $2 - result + ENCODING_TAG = Regexp.new("\A(<%#{ENCODING_FLAG}-?%>)[ \t]*") + + def compile(template) + erb = template.source.gsub(ENCODING_TAG, '') + result = self.class.erb_implementation.new( + erb, + :trim => (self.class.erb_trim_mode == "-") + ).src + + result = "#{$2}\n#{result}" if $2 + result + end end end end -- cgit v1.2.3 From 64d109e3539ad600f58536d3ecabd2f87b67fd1c Mon Sep 17 00:00:00 2001 From: wycats Date: Sun, 16 May 2010 10:25:55 +0400 Subject: Significantly improved internal encoding heuristics and support. * Default Encoding.default_internal to UTF-8 * Eliminated the use of file-wide magic comments to coerce code evaluated inside the file * Read templates as BINARY, use default_external or template-wide magic comments inside the Template to set the initial encoding * This means that template handlers in Ruby 1.9 will receive Strings encoded in default_internal (UTF-8 by default) * Create a better Exception for encoding issues, and use it when the template source has bytes that are not compatible with the specified encoding * Allow template handlers to opt-into handling BINARY. If they do so, they need to do some of their own manual encoding work * Added a "Configuration Gotchas" section to the intro Rails Guide instructing users to use UTF-8 for everything * Use config.encoding= in Ruby 1.8, and raise if a value that is an invalid $KCODE value is used Also: * Fixed a few tests that were assert() rather than assert_equal() and were caught by Minitest requiring a String for the message * Fixed a test where an assert_select was misformed, also caught by Minitest being more restrictive * Fixed a test where a Rack response was returning a String rather than an Enumerable --- actionpack/lib/action_view.rb | 6 +- actionpack/lib/action_view/template.rb | 201 +++++++++++++++++---- actionpack/lib/action_view/template/error.rb | 18 ++ .../lib/action_view/template/handlers/erb.rb | 45 ++++- actionpack/lib/action_view/template/resolver.rb | 5 +- actionpack/test/abstract_unit.rb | 4 + actionpack/test/controller/assert_select_test.rb | 4 +- actionpack/test/controller/capture_test.rb | 2 +- actionpack/test/controller/render_test.rb | 4 +- actionpack/test/fixtures/test/content_for.erb | 3 +- .../fixtures/test/content_for_concatenated.erb | 2 +- .../fixtures/test/content_for_with_parameter.erb | 2 +- .../test/non_erb_block_content_for.builder | 2 +- actionpack/test/template/render_test.rb | 10 +- actionpack/test/template/template_test.rb | 128 +++++++++++++ 15 files changed, 384 insertions(+), 52 deletions(-) create mode 100644 actionpack/test/template/template_test.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 5e3b2ec51b..9f56cca869 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -51,7 +51,9 @@ module ActionView autoload :MissingTemplate, 'action_view/template/error' autoload :ActionViewError, 'action_view/template/error' - autoload :TemplateError, 'action_view/template/error' + autoload :EncodingError, 'action_view/template/error' + autoload :TemplateError, 'action_view/template/error' + autoload :WrongEncodingError, 'action_view/template/error' autoload :TemplateHandler, 'action_view/template' autoload :TemplateHandlers, 'action_view/template' @@ -59,7 +61,7 @@ module ActionView autoload :TestCase, 'action_view/test_case' - ENCODING_FLAG = "#.*coding[:=]\s*(\S+)[ \t]*" + ENCODING_FLAG = '#.*coding[:=]\s*(\S+)[ \t]*' end require 'active_support/i18n' diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index ce249e2a96..5d8ac6b115 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -1,12 +1,89 @@ -# encoding: utf-8 -# This is so that templates compiled in this file are UTF-8 require 'active_support/core_ext/array/wrap' require 'active_support/core_ext/object/blank' +require 'active_support/core_ext/kernel/singleton_class' module ActionView class Template extend ActiveSupport::Autoload + # === Encodings in ActionView::Template + # + # ActionView::Template is one of a few sources of potential + # encoding issues in Rails. This is because the source for + # templates are usually read from disk, and Ruby (like most + # encoding-aware programming languages) assumes that the + # String retrieved through File IO is encoded in the + # default_external encoding. In Rails, the default + # default_external encoding is UTF-8. + # + # As a result, if a user saves their template as ISO-8859-1 + # (for instance, using a non-Unicode-aware text editor), + # and uses characters outside of the ASCII range, their + # users will see diamonds with question marks in them in + # the browser. + # + # To mitigate this problem, we use a few strategies: + # 1. If the source is not valid UTF-8, we raise an exception + # when the template is compiled to alert the user + # to the problem. + # 2. The user can specify the encoding using Ruby-style + # encoding comments in any template engine. If such + # a comment is supplied, Rails will apply that encoding + # to the resulting compiled source returned by the + # template handler. + # 3. In all cases, we transcode the resulting String to + # the default_internal encoding (which defaults + # to UTF-8). + # + # This means that other parts of Rails can always assume + # that templates are encoded in UTF-8, even if the original + # source of the template was not UTF-8. + # + # From a user's perspective, the easiest thing to do is + # to save your templates as UTF-8. If you do this, you + # do not need to do anything else for things to "just work". + # + # === Instructions for template handlers + # + # The easiest thing for you to do is to simply ignore + # encodings. Rails will hand you the template source + # as the default_internal (generally UTF-8), raising + # an exception for the user before sending the template + # to you if it could not determine the original encoding. + # + # For the greatest simplicity, you can support only + # UTF-8 as the default_internal. This means + # that from the perspective of your handler, the + # entire pipeline is just UTF-8. + # + # === Advanced: Handlers with alternate metadata sources + # + # If you want to provide an alternate mechanism for + # specifying encodings (like ERB does via <%# encoding: ... %>), + # you may indicate that you are willing to accept + # BINARY data by implementing self.accepts_binary? + # on your handler. + # + # If you do, Rails will not raise an exception if + # the template's encoding could not be determined, + # assuming that you have another mechanism for + # making the determination. + # + # In this case, make sure you return a String from + # your handler encoded in the default_internal. Since + # you are handling out-of-band metadata, you are + # also responsible for alerting the user to any + # problems with converting the user's data to + # the default_internal. + # + # To do so, simply raise the raise WrongEncodingError + # as follows: + # + # raise WrongEncodingError.new( + # problematic_string, + # expected_encoding + # ) + eager_autoload do autoload :Error autoload :Handler @@ -16,26 +93,22 @@ module ActionView extend Template::Handlers - attr_reader :source, :identifier, :handler, :virtual_path, :formats + attr_reader :source, :identifier, :handler, :virtual_path, :formats, + :original_encoding - Finalizer = proc do |method_name| + Finalizer = proc do |method_name, mod| proc do - ActionView::CompiledTemplates.module_eval do + mod.module_eval do remove_possible_method method_name end end end def initialize(source, identifier, handler, details) - if source.encoding_aware? && source =~ %r{\A#{ENCODING_FLAG}} - # don't snip off the \n to preserve line numbers - source.sub!(/\A[^\n]*/, '') - source.force_encoding($1).encode - end - - @source = source - @identifier = identifier - @handler = handler + @source = source + @identifier = identifier + @handler = handler + @original_encoding = nil @virtual_path = details[:virtual_path] @method_names = {} @@ -48,7 +121,13 @@ module ActionView # Notice that we use a bang in this instrumentation because you don't want to # consume this in production. This is only slow if it's being listened to. ActiveSupport::Notifications.instrument("!render_template.action_view", :virtual_path => @virtual_path) do - method_name = compile(locals, view) + if view.is_a?(ActionView::CompiledTemplates) + mod = ActionView::CompiledTemplates + else + mod = view.singleton_class + end + + method_name = compile(locals, view, mod) view.send(method_name, locals, &block) end rescue Exception => e @@ -56,7 +135,7 @@ module ActionView e.sub_template_of(self) raise e else - raise Template::Error.new(self, view.assigns, e) + raise Template::Error.new(self, view.respond_to?(:assigns) ? view.assigns : {}, e) end end @@ -81,37 +160,97 @@ module ActionView end private - def compile(locals, view) + # Among other things, this method is responsible for properly setting + # the encoding of the source. Until this point, we assume that the + # source is BINARY data. If no additional information is supplied, + # we assume the encoding is the same as Encoding.default_external. + # + # The user can also specify the encoding via a comment on the first + # line of the template (# encoding: NAME-OF-ENCODING). This will work + # with any template engine, as we process out the encoding comment + # before passing the source on to the template engine, leaving a + # blank line in its stead. + # + # Note that after we figure out the correct encoding, we then + # encode the source into Encoding.default_internal. In general, + # this means that templates will be UTF-8 inside of Rails, + # regardless of the original source encoding. + def compile(locals, view, mod) method_name = build_method_name(locals) return method_name if view.respond_to?(method_name) locals_code = locals.keys.map! { |key| "#{key} = local_assigns[:#{key}];" }.join - code = @handler.call(self) - if code.sub!(/\A(#.*coding.*)\n/, '') - encoding_comment = $1 - elsif defined?(Encoding) && Encoding.respond_to?(:default_external) - encoding_comment = "#coding:#{Encoding.default_external}" + if source.encoding_aware? + if source.sub!(/\A#{ENCODING_FLAG}/, '') + encoding = $1 + else + encoding = Encoding.default_external + end + + # Tag the source with the default external encoding + # or the encoding specified in the file + source.force_encoding(encoding) + + # If the original encoding is BINARY, the actual + # encoding is either stored out-of-band (such as + # in ERB <%# %> style magic comments) or missing. + # This is also true if the original encoding is + # something other than BINARY, but it's invalid. + if source.encoding != Encoding::BINARY && source.valid_encoding? + source.encode! + # If the assumed encoding is incorrect, check to + # see whether the handler accepts BINARY. If it + # does, it has another mechanism for determining + # the true encoding of the String. + elsif @handler.respond_to?(:accepts_binary?) && @handler.accepts_binary? + source.force_encoding(Encoding::BINARY) + # If the handler does not accept BINARY, the + # assumed encoding (either the default_external, + # or the explicit encoding specified by the user) + # is incorrect. We raise an exception here. + else + raise WrongEncodingError.new(source, encoding) + end + + # Don't validate the encoding yet -- the handler + # may treat the String as raw bytes and extract + # the encoding some other way end + code = @handler.call(self) + source = <<-end_src def #{method_name}(local_assigns) - _old_virtual_path, @_virtual_path = @_virtual_path, #{@virtual_path.inspect};_old_output_buffer = output_buffer;#{locals_code};#{code} + _old_virtual_path, @_virtual_path = @_virtual_path, #{@virtual_path.inspect};_old_output_buffer = @output_buffer;#{locals_code};#{code} ensure - @_virtual_path, self.output_buffer = _old_virtual_path, _old_output_buffer + @_virtual_path, @output_buffer = _old_virtual_path, _old_output_buffer end end_src - if encoding_comment - source = "#{encoding_comment}\n#{source}" - line = -1 - else - line = 0 + if source.encoding_aware? + # Handlers should return their source Strings in either the + # default_internal or BINARY. If the handler returns a BINARY + # String, we assume its encoding is the one we determined + # earlier, and encode the resulting source in the default_internal. + if source.encoding == Encoding::BINARY + source.force_encoding(Encoding.default_internal) + end + + # In case we get back a String from a handler that is not in + # BINARY or the default_internal, encode it to the default_internal + source.encode! + + # Now, validate that the source we got back from the template + # handler is valid in the default_internal + unless source.valid_encoding? + raise WrongEncodingError.new(@source, Encoding.default_internal) + end end begin - ActionView::CompiledTemplates.module_eval(source, identifier, line) - ObjectSpace.define_finalizer(self, Finalizer[method_name]) + mod.module_eval(source, identifier, 0) + ObjectSpace.define_finalizer(self, Finalizer[method_name, mod]) method_name rescue Exception => e # errors from template code diff --git a/actionpack/lib/action_view/template/error.rb b/actionpack/lib/action_view/template/error.rb index 6866eabf77..d3a53d2147 100644 --- a/actionpack/lib/action_view/template/error.rb +++ b/actionpack/lib/action_view/template/error.rb @@ -4,6 +4,24 @@ module ActionView class ActionViewError < StandardError #:nodoc: end + class EncodingError < StandardError #:nodoc: + end + + class WrongEncodingError < EncodingError #:nodoc: + def initialize(string, encoding) + @string, @encoding = string, encoding + end + + def message + "Your template was not saved as valid #{@encoding}. Please " \ + "either specify #{@encoding} as the encoding for your template " \ + "in your text editor, or mark the template with its " \ + "encoding by inserting the following as the first line " \ + "of the template:\n\n# encoding: .\n\n" \ + "The source of your template was:\n\n#{@string}" + end + end + class MissingTemplate < ActionViewError #:nodoc: attr_reader :path diff --git a/actionpack/lib/action_view/template/handlers/erb.rb b/actionpack/lib/action_view/template/handlers/erb.rb index 17652d6d1f..bbf012ab15 100644 --- a/actionpack/lib/action_view/template/handlers/erb.rb +++ b/actionpack/lib/action_view/template/handlers/erb.rb @@ -5,6 +5,11 @@ require 'erubis' module ActionView class OutputBuffer < ActiveSupport::SafeBuffer + def initialize(*) + super + encode! + end + def <<(value) super(value.to_s) end @@ -72,16 +77,50 @@ module ActionView cattr_accessor :erb_implementation self.erb_implementation = Erubis - ENCODING_TAG = Regexp.new("\A(<%#{ENCODING_FLAG}-?%>)[ \t]*") + ENCODING_TAG = Regexp.new("\\A(<%#{ENCODING_FLAG}-?%>)[ \\t]*") + + def self.accepts_binary? + true + end def compile(template) - erb = template.source.gsub(ENCODING_TAG, '') + if template.source.encoding_aware? + # Even though Rails has given us a String tagged with the + # default_internal encoding (likely UTF-8), it is possible + # that the String is actually encoded using a different + # encoding, specified via an ERB magic comment. If the + # String is not actually UTF-8, the regular expression + # engine will (correctly) raise an exception. For now, + # we'll reset the String to BINARY so we can run regular + # expressions against it + template_source = template.source.dup.force_encoding("BINARY") + + # Erubis does not have direct support for encodings. + # As a result, we will extract the ERB-style magic + # comment, give the String to Erubis as BINARY data, + # and then tag the resulting String with the extracted + # encoding later + erb = template_source.gsub(ENCODING_TAG, '') + encoding = $2 + + if !encoding && (template.source.encoding == Encoding::BINARY) + raise WrongEncodingError.new(template_source, Encoding.default_external) + end + end + result = self.class.erb_implementation.new( erb, :trim => (self.class.erb_trim_mode == "-") ).src - result = "#{$2}\n#{result}" if $2 + # If an encoding tag was found, tag the String + # we're returning with that encoding. Otherwise, + # return a BINARY String, which is what ERB + # returns. Note that if a magic comment was + # not specified, we will return the data to + # Rails as BINARY, which will then use its + # own encoding logic to create a UTF-8 String. + result = "\n#{result}".force_encoding(encoding).encode if encoding result end end diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index a223b3a55f..ef44925951 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -70,7 +70,10 @@ module ActionView Dir[query].reject { |p| File.directory?(p) }.map do |p| handler, format = extract_handler_and_format(p, formats) - Template.new(File.read(p), File.expand_path(p), handler, + + contents = File.open(p, "rb") {|io| io.read } + + Template.new(contents, File.expand_path(p), handler, :virtual_path => path, :format => format) end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 89ba0990f1..479e62b23d 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -12,6 +12,10 @@ $:.unshift(File.dirname(__FILE__) + '/fixtures/alternate_helpers') ENV['TMPDIR'] = File.join(File.dirname(__FILE__), 'tmp') +if defined?(Encoding.default_internal) + Encoding.default_internal = "UTF-8" +end + require 'test/unit' require 'abstract_controller' require 'action_controller' diff --git a/actionpack/test/controller/assert_select_test.rb b/actionpack/test/controller/assert_select_test.rb index 7012c4c9b0..4f8ad23174 100644 --- a/actionpack/test/controller/assert_select_test.rb +++ b/actionpack/test/controller/assert_select_test.rb @@ -212,12 +212,12 @@ class AssertSelectTest < ActionController::TestCase assert_nothing_raised { assert_select "div", "bar" } assert_nothing_raised { assert_select "div", /\w*/ } assert_nothing_raised { assert_select "div", :text => /\w*/, :count=>2 } - assert_raise(Assertion) { assert_select "div", :text=>"foo", :count=>2 } + assert_raise(Assertion) { assert_select "div", :text=>"foo", :count=>2 } assert_nothing_raised { assert_select "div", :html=>"bar" } assert_nothing_raised { assert_select "div", :html=>"bar" } assert_nothing_raised { assert_select "div", :html=>/\w*/ } assert_nothing_raised { assert_select "div", :html=>/\w*/, :count=>2 } - assert_raise(Assertion) { assert_select "div", :html=>"foo", :count=>2 } + assert_raise(Assertion) { assert_select "div", :html=>"foo", :count=>2 } end end diff --git a/actionpack/test/controller/capture_test.rb b/actionpack/test/controller/capture_test.rb index d1dbd535c4..47253f22b8 100644 --- a/actionpack/test/controller/capture_test.rb +++ b/actionpack/test/controller/capture_test.rb @@ -68,6 +68,6 @@ class CaptureTest < ActionController::TestCase private def expected_content_for_output - "Putting stuff in the title!\n\nGreat stuff!" + "Putting stuff in the title!\nGreat stuff!" end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 52049f2a8a..2b1f2a27df 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -1079,7 +1079,7 @@ class RenderTest < ActionController::TestCase def test_action_talk_to_layout get :action_talk_to_layout - assert_equal "Talking to the layout\n\nAction was here!", @response.body + assert_equal "Talking to the layout\nAction was here!", @response.body end # :addressed: @@ -1096,7 +1096,7 @@ class RenderTest < ActionController::TestCase def test_yield_content_for assert_not_deprecated { get :yield_content_for } - assert_equal "Putting stuff in the title!\n\nGreat stuff!\n", @response.body + assert_equal "Putting stuff in the title!\nGreat stuff!\n", @response.body end def test_overwritting_rendering_relative_file_with_extension diff --git a/actionpack/test/fixtures/test/content_for.erb b/actionpack/test/fixtures/test/content_for.erb index 0e47ca8c3d..1fb829f54c 100644 --- a/actionpack/test/fixtures/test/content_for.erb +++ b/actionpack/test/fixtures/test/content_for.erb @@ -1,2 +1 @@ -<% content_for :title do %>Putting stuff in the title!<% end %> -Great stuff! \ No newline at end of file +<% content_for :title do -%>Putting stuff in the title!<% end -%>Great stuff! \ No newline at end of file diff --git a/actionpack/test/fixtures/test/content_for_concatenated.erb b/actionpack/test/fixtures/test/content_for_concatenated.erb index fb6b4b05d7..e65f629574 100644 --- a/actionpack/test/fixtures/test/content_for_concatenated.erb +++ b/actionpack/test/fixtures/test/content_for_concatenated.erb @@ -1,3 +1,3 @@ <% content_for :title, "Putting stuff " - content_for :title, "in the title!" %> + content_for :title, "in the title!" -%> Great stuff! \ No newline at end of file diff --git a/actionpack/test/fixtures/test/content_for_with_parameter.erb b/actionpack/test/fixtures/test/content_for_with_parameter.erb index 57aecbac05..aeb6f73ce0 100644 --- a/actionpack/test/fixtures/test/content_for_with_parameter.erb +++ b/actionpack/test/fixtures/test/content_for_with_parameter.erb @@ -1,2 +1,2 @@ -<% content_for :title, "Putting stuff in the title!" %> +<% content_for :title, "Putting stuff in the title!" -%> Great stuff! \ No newline at end of file diff --git a/actionpack/test/fixtures/test/non_erb_block_content_for.builder b/actionpack/test/fixtures/test/non_erb_block_content_for.builder index a94643561c..d539a425a4 100644 --- a/actionpack/test/fixtures/test/non_erb_block_content_for.builder +++ b/actionpack/test/fixtures/test/non_erb_block_content_for.builder @@ -1,4 +1,4 @@ content_for :title do 'Putting stuff in the title!' end -xml << "\nGreat stuff!" +xml << "Great stuff!" diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index d0212024ae..aca96e0a24 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -232,13 +232,13 @@ module RenderTestCases # TODO: Move to deprecated_tests.rb def test_render_with_nested_layout_deprecated assert_deprecated do - assert_equal %(title\n\n\n
column
\n
content
\n), + assert_equal %(title\n\n
column
\n
content
\n), @view.render(:file => "test/deprecated_nested_layout.erb", :layout => "layouts/yield") end end def test_render_with_nested_layout - assert_equal %(title\n\n\n
column
\n
content
\n), + assert_equal %(title\n\n
column
\n
content
\n), @view.render(:file => "test/nested_layout.erb", :layout => "layouts/yield") end @@ -284,7 +284,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase with_external_encoding Encoding::ASCII_8BIT do result = @view.render(:file => "test/utf8_magic.html.erb", :layouts => "layouts/yield") assert_equal Encoding::UTF_8, result.encoding - assert_equal "Русский текст\n\nUTF-8\nUTF-8\nUTF-8\n", result + assert_equal "\nРусский \nтекст\n\nUTF-8\nUTF-8\nUTF-8\n", result end end @@ -302,7 +302,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase result = @view.render(:file => "test/utf8.html.erb", :layouts => "layouts/yield") flunk 'Should have raised incompatible encoding error' rescue ActionView::Template::Error => error - assert_match 'invalid byte sequence in Shift_JIS', error.original_exception.message + assert_match 'Your template was not saved as valid Shift_JIS', error.original_exception.message end end end @@ -313,7 +313,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase result = @view.render(:file => "test/utf8_magic_with_bare_partial.html.erb", :layouts => "layouts/yield") flunk 'Should have raised incompatible encoding error' rescue ActionView::Template::Error => error - assert_match 'invalid byte sequence in Shift_JIS', error.original_exception.message + assert_match 'Your template was not saved as valid Shift_JIS', error.original_exception.message end end end diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb new file mode 100644 index 0000000000..c4a65d84fc --- /dev/null +++ b/actionpack/test/template/template_test.rb @@ -0,0 +1,128 @@ +require "abstract_unit" + +# These are the normal settings that will be set up by Railties +# TODO: Have these tests support other combinations of these values +Encoding.default_internal = "UTF-8" +Encoding.default_external = "UTF-8" + +class TestERBTemplate < ActiveSupport::TestCase + ERBHandler = ActionView::Template::Handlers::ERB + + class Context + def initialize + @output_buffer = "original" + end + + def hello + "Hello" + end + + def partial + ActionView::Template.new( + "<%= @_virtual_path %>", + "partial", + ERBHandler, + :virtual_path => "partial" + ) + end + + def logger + require "logger" + Logger.new(STDERR) + end + + def my_buffer + @output_buffer + end + end + + def new_template(body = "<%= hello %>", handler = ERBHandler, details = {}) + ActionView::Template.new(body, "hello template", ERBHandler, {:virtual_path => "hello"}) + end + + def render(locals = {}) + @template.render(@obj, locals) + end + + def setup + @obj = Context.new + end + + def test_basic_template + @template = new_template + assert_equal "Hello", render + end + + def test_locals + @template = new_template("<%= my_local %>") + assert_equal "I'm a local", render(:my_local => "I'm a local") + end + + def test_restores_buffer + @template = new_template + assert_equal "Hello", render + assert_equal "original", @obj.my_buffer + end + + def test_virtual_path + @template = new_template("<%= @_virtual_path %>" \ + "<%= partial.render(self, {}) %>" \ + "<%= @_virtual_path %>") + assert_equal "hellopartialhello", render + end + + if "ruby".encoding_aware? + def test_resulting_string_is_utf8 + @template = new_template + assert_equal Encoding::UTF_8, render.encoding + end + + def test_no_magic_comment_word_with_utf_8 + @template = new_template("hello \u{fc}mlat") + assert_equal Encoding::UTF_8, render.encoding + assert_equal "hello \u{fc}mlat", render + end + + # This test ensures that if the default_external + # is set to something other than UTF-8, we don't + # get any errors and get back a UTF-8 String. + def test_default_external_works + Encoding.default_external = "ISO-8859-1" + @template = new_template("hello \xFCmlat") + assert_equal Encoding::UTF_8, render.encoding + assert_equal "hello \u{fc}mlat", render + ensure + Encoding.default_external = "UTF-8" + end + + def test_encoding_can_be_specified_with_magic_comment + @template = new_template("# encoding: ISO-8859-1\nhello \xFCmlat") + assert_equal Encoding::UTF_8, render.encoding + assert_equal "\nhello \u{fc}mlat", render + end + + # TODO: This is currently handled inside ERB. The case of explicitly + # lying about encodings via the normal Rails API should be handled + # inside Rails. + def test_lying_with_magic_comment + assert_raises(ActionView::Template::Error) do + @template = new_template("# encoding: UTF-8\nhello \xFCmlat") + render + end + end + + def test_encoding_can_be_specified_with_magic_comment_in_erb + @template = new_template("<%# encoding: ISO-8859-1 %>hello \xFCmlat") + result = render + assert_equal Encoding::UTF_8, render.encoding + assert_equal "hello \u{fc}mlat", render + end + + def test_error_when_template_isnt_valid_utf8 + assert_raises(ActionView::Template::Error, /\xFC/) do + @template = new_template("hello \xFCmlat") + render + end + end + end +end \ No newline at end of file -- cgit v1.2.3 From ade756fe42423033bae8e5aea8f58782f7a6c517 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sun, 16 May 2010 13:52:51 -0700 Subject: Moved encoding work in progress to a feature branch. This reverts commits af0d1a88157942c6e6398dbf73891cff1e152405 and 64d109e3539ad600f58536d3ecabd2f87b67fd1c. --- actionpack/lib/action_view.rb | 6 +- actionpack/lib/action_view/template.rb | 195 +++------------------ actionpack/lib/action_view/template/error.rb | 18 -- .../lib/action_view/template/handlers/erb.rb | 137 +++++---------- actionpack/lib/action_view/template/resolver.rb | 5 +- actionpack/test/abstract_unit.rb | 4 - actionpack/test/controller/assert_select_test.rb | 4 +- actionpack/test/controller/capture_test.rb | 2 +- actionpack/test/controller/render_test.rb | 4 +- actionpack/test/fixtures/test/content_for.erb | 3 +- .../fixtures/test/content_for_concatenated.erb | 2 +- .../fixtures/test/content_for_with_parameter.erb | 2 +- .../test/non_erb_block_content_for.builder | 2 +- actionpack/test/template/render_test.rb | 10 +- actionpack/test/template/template_test.rb | 128 -------------- 15 files changed, 87 insertions(+), 435 deletions(-) delete mode 100644 actionpack/test/template/template_test.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 9f56cca869..5555217ee2 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -51,17 +51,13 @@ module ActionView autoload :MissingTemplate, 'action_view/template/error' autoload :ActionViewError, 'action_view/template/error' - autoload :EncodingError, 'action_view/template/error' - autoload :TemplateError, 'action_view/template/error' - autoload :WrongEncodingError, 'action_view/template/error' + autoload :TemplateError, 'action_view/template/error' autoload :TemplateHandler, 'action_view/template' autoload :TemplateHandlers, 'action_view/template' end autoload :TestCase, 'action_view/test_case' - - ENCODING_FLAG = '#.*coding[:=]\s*(\S+)[ \t]*' end require 'active_support/i18n' diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 5d8ac6b115..a1a970e2d2 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -1,89 +1,12 @@ +# encoding: utf-8 +# This is so that templates compiled in this file are UTF-8 require 'active_support/core_ext/array/wrap' require 'active_support/core_ext/object/blank' -require 'active_support/core_ext/kernel/singleton_class' module ActionView class Template extend ActiveSupport::Autoload - # === Encodings in ActionView::Template - # - # ActionView::Template is one of a few sources of potential - # encoding issues in Rails. This is because the source for - # templates are usually read from disk, and Ruby (like most - # encoding-aware programming languages) assumes that the - # String retrieved through File IO is encoded in the - # default_external encoding. In Rails, the default - # default_external encoding is UTF-8. - # - # As a result, if a user saves their template as ISO-8859-1 - # (for instance, using a non-Unicode-aware text editor), - # and uses characters outside of the ASCII range, their - # users will see diamonds with question marks in them in - # the browser. - # - # To mitigate this problem, we use a few strategies: - # 1. If the source is not valid UTF-8, we raise an exception - # when the template is compiled to alert the user - # to the problem. - # 2. The user can specify the encoding using Ruby-style - # encoding comments in any template engine. If such - # a comment is supplied, Rails will apply that encoding - # to the resulting compiled source returned by the - # template handler. - # 3. In all cases, we transcode the resulting String to - # the default_internal encoding (which defaults - # to UTF-8). - # - # This means that other parts of Rails can always assume - # that templates are encoded in UTF-8, even if the original - # source of the template was not UTF-8. - # - # From a user's perspective, the easiest thing to do is - # to save your templates as UTF-8. If you do this, you - # do not need to do anything else for things to "just work". - # - # === Instructions for template handlers - # - # The easiest thing for you to do is to simply ignore - # encodings. Rails will hand you the template source - # as the default_internal (generally UTF-8), raising - # an exception for the user before sending the template - # to you if it could not determine the original encoding. - # - # For the greatest simplicity, you can support only - # UTF-8 as the default_internal. This means - # that from the perspective of your handler, the - # entire pipeline is just UTF-8. - # - # === Advanced: Handlers with alternate metadata sources - # - # If you want to provide an alternate mechanism for - # specifying encodings (like ERB does via <%# encoding: ... %>), - # you may indicate that you are willing to accept - # BINARY data by implementing self.accepts_binary? - # on your handler. - # - # If you do, Rails will not raise an exception if - # the template's encoding could not be determined, - # assuming that you have another mechanism for - # making the determination. - # - # In this case, make sure you return a String from - # your handler encoded in the default_internal. Since - # you are handling out-of-band metadata, you are - # also responsible for alerting the user to any - # problems with converting the user's data to - # the default_internal. - # - # To do so, simply raise the raise WrongEncodingError - # as follows: - # - # raise WrongEncodingError.new( - # problematic_string, - # expected_encoding - # ) - eager_autoload do autoload :Error autoload :Handler @@ -93,22 +16,20 @@ module ActionView extend Template::Handlers - attr_reader :source, :identifier, :handler, :virtual_path, :formats, - :original_encoding + attr_reader :source, :identifier, :handler, :virtual_path, :formats - Finalizer = proc do |method_name, mod| + Finalizer = proc do |method_name| proc do - mod.module_eval do + ActionView::CompiledTemplates.module_eval do remove_possible_method method_name end end end def initialize(source, identifier, handler, details) - @source = source - @identifier = identifier - @handler = handler - @original_encoding = nil + @source = source + @identifier = identifier + @handler = handler @virtual_path = details[:virtual_path] @method_names = {} @@ -121,13 +42,7 @@ module ActionView # Notice that we use a bang in this instrumentation because you don't want to # consume this in production. This is only slow if it's being listened to. ActiveSupport::Notifications.instrument("!render_template.action_view", :virtual_path => @virtual_path) do - if view.is_a?(ActionView::CompiledTemplates) - mod = ActionView::CompiledTemplates - else - mod = view.singleton_class - end - - method_name = compile(locals, view, mod) + method_name = compile(locals, view) view.send(method_name, locals, &block) end rescue Exception => e @@ -135,7 +50,7 @@ module ActionView e.sub_template_of(self) raise e else - raise Template::Error.new(self, view.respond_to?(:assigns) ? view.assigns : {}, e) + raise Template::Error.new(self, view.assigns, e) end end @@ -160,97 +75,37 @@ module ActionView end private - # Among other things, this method is responsible for properly setting - # the encoding of the source. Until this point, we assume that the - # source is BINARY data. If no additional information is supplied, - # we assume the encoding is the same as Encoding.default_external. - # - # The user can also specify the encoding via a comment on the first - # line of the template (# encoding: NAME-OF-ENCODING). This will work - # with any template engine, as we process out the encoding comment - # before passing the source on to the template engine, leaving a - # blank line in its stead. - # - # Note that after we figure out the correct encoding, we then - # encode the source into Encoding.default_internal. In general, - # this means that templates will be UTF-8 inside of Rails, - # regardless of the original source encoding. - def compile(locals, view, mod) + def compile(locals, view) method_name = build_method_name(locals) return method_name if view.respond_to?(method_name) locals_code = locals.keys.map! { |key| "#{key} = local_assigns[:#{key}];" }.join - if source.encoding_aware? - if source.sub!(/\A#{ENCODING_FLAG}/, '') - encoding = $1 - else - encoding = Encoding.default_external - end - - # Tag the source with the default external encoding - # or the encoding specified in the file - source.force_encoding(encoding) - - # If the original encoding is BINARY, the actual - # encoding is either stored out-of-band (such as - # in ERB <%# %> style magic comments) or missing. - # This is also true if the original encoding is - # something other than BINARY, but it's invalid. - if source.encoding != Encoding::BINARY && source.valid_encoding? - source.encode! - # If the assumed encoding is incorrect, check to - # see whether the handler accepts BINARY. If it - # does, it has another mechanism for determining - # the true encoding of the String. - elsif @handler.respond_to?(:accepts_binary?) && @handler.accepts_binary? - source.force_encoding(Encoding::BINARY) - # If the handler does not accept BINARY, the - # assumed encoding (either the default_external, - # or the explicit encoding specified by the user) - # is incorrect. We raise an exception here. - else - raise WrongEncodingError.new(source, encoding) - end - - # Don't validate the encoding yet -- the handler - # may treat the String as raw bytes and extract - # the encoding some other way - end - code = @handler.call(self) + if code.sub!(/\A(#.*coding.*)\n/, '') + encoding_comment = $1 + elsif defined?(Encoding) && Encoding.respond_to?(:default_external) + encoding_comment = "#coding:#{Encoding.default_external}" + end source = <<-end_src def #{method_name}(local_assigns) - _old_virtual_path, @_virtual_path = @_virtual_path, #{@virtual_path.inspect};_old_output_buffer = @output_buffer;#{locals_code};#{code} + _old_virtual_path, @_virtual_path = @_virtual_path, #{@virtual_path.inspect};_old_output_buffer = output_buffer;#{locals_code};#{code} ensure - @_virtual_path, @output_buffer = _old_virtual_path, _old_output_buffer + @_virtual_path, self.output_buffer = _old_virtual_path, _old_output_buffer end end_src - if source.encoding_aware? - # Handlers should return their source Strings in either the - # default_internal or BINARY. If the handler returns a BINARY - # String, we assume its encoding is the one we determined - # earlier, and encode the resulting source in the default_internal. - if source.encoding == Encoding::BINARY - source.force_encoding(Encoding.default_internal) - end - - # In case we get back a String from a handler that is not in - # BINARY or the default_internal, encode it to the default_internal - source.encode! - - # Now, validate that the source we got back from the template - # handler is valid in the default_internal - unless source.valid_encoding? - raise WrongEncodingError.new(@source, Encoding.default_internal) - end + if encoding_comment + source = "#{encoding_comment}\n#{source}" + line = -1 + else + line = 0 end begin - mod.module_eval(source, identifier, 0) - ObjectSpace.define_finalizer(self, Finalizer[method_name, mod]) + ActionView::CompiledTemplates.module_eval(source, identifier, line) + ObjectSpace.define_finalizer(self, Finalizer[method_name]) method_name rescue Exception => e # errors from template code diff --git a/actionpack/lib/action_view/template/error.rb b/actionpack/lib/action_view/template/error.rb index d3a53d2147..6866eabf77 100644 --- a/actionpack/lib/action_view/template/error.rb +++ b/actionpack/lib/action_view/template/error.rb @@ -4,24 +4,6 @@ module ActionView class ActionViewError < StandardError #:nodoc: end - class EncodingError < StandardError #:nodoc: - end - - class WrongEncodingError < EncodingError #:nodoc: - def initialize(string, encoding) - @string, @encoding = string, encoding - end - - def message - "Your template was not saved as valid #{@encoding}. Please " \ - "either specify #{@encoding} as the encoding for your template " \ - "in your text editor, or mark the template with its " \ - "encoding by inserting the following as the first line " \ - "of the template:\n\n# encoding: .\n\n" \ - "The source of your template was:\n\n#{@string}" - end - end - class MissingTemplate < ActionViewError #:nodoc: attr_reader :path diff --git a/actionpack/lib/action_view/template/handlers/erb.rb b/actionpack/lib/action_view/template/handlers/erb.rb index bbf012ab15..237746437a 100644 --- a/actionpack/lib/action_view/template/handlers/erb.rb +++ b/actionpack/lib/action_view/template/handlers/erb.rb @@ -1,15 +1,9 @@ require 'active_support/core_ext/class/attribute_accessors' require 'active_support/core_ext/string/output_safety' -require "action_view/template" require 'erubis' module ActionView class OutputBuffer < ActiveSupport::SafeBuffer - def initialize(*) - super - encode! - end - def <<(value) super(value.to_s) end @@ -23,106 +17,65 @@ module ActionView end end - class Template - module Handlers - class Erubis < ::Erubis::Eruby - def add_preamble(src) - src << "@output_buffer = ActionView::OutputBuffer.new;" - end - - def add_text(src, text) - return if text.empty? - src << "@output_buffer.safe_concat('" << escape_text(text) << "');" - end - - BLOCK_EXPR = /\s+(do|\{)(\s*\|[^|]*\|)?\s*\Z/ + module Template::Handlers + class Erubis < ::Erubis::Eruby + def add_preamble(src) + src << "@output_buffer = ActionView::OutputBuffer.new;" + end - def add_expr_literal(src, code) - if code =~ BLOCK_EXPR - src << '@output_buffer.append= ' << code - else - src << '@output_buffer.append= (' << code << ');' - end - end + def add_text(src, text) + return if text.empty? + src << "@output_buffer.safe_concat('" << escape_text(text) << "');" + end - def add_stmt(src, code) - if code =~ BLOCK_EXPR - src << '@output_buffer.append_if_string= ' << code - else - super - end - end + BLOCK_EXPR = /\s+(do|\{)(\s*\|[^|]*\|)?\s*\Z/ - def add_expr_escaped(src, code) - src << '@output_buffer.append= ' << escaped_expr(code) << ';' + def add_expr_literal(src, code) + if code =~ BLOCK_EXPR + src << '@output_buffer.append= ' << code + else + src << '@output_buffer.append= (' << code << ');' end + end - def add_postamble(src) - src << '@output_buffer.to_s' + def add_stmt(src, code) + if code =~ BLOCK_EXPR + src << '@output_buffer.append_if_string= ' << code + else + super end end - class ERB < Handler - include Compilable - - ## - # :singleton-method: - # Specify trim mode for the ERB compiler. Defaults to '-'. - # See ERb documentation for suitable values. - cattr_accessor :erb_trim_mode - self.erb_trim_mode = '-' - - self.default_format = Mime::HTML - - cattr_accessor :erb_implementation - self.erb_implementation = Erubis - - ENCODING_TAG = Regexp.new("\\A(<%#{ENCODING_FLAG}-?%>)[ \\t]*") + def add_expr_escaped(src, code) + src << '@output_buffer.append= ' << escaped_expr(code) << ';' + end - def self.accepts_binary? - true - end + def add_postamble(src) + src << '@output_buffer.to_s' + end + end - def compile(template) - if template.source.encoding_aware? - # Even though Rails has given us a String tagged with the - # default_internal encoding (likely UTF-8), it is possible - # that the String is actually encoded using a different - # encoding, specified via an ERB magic comment. If the - # String is not actually UTF-8, the regular expression - # engine will (correctly) raise an exception. For now, - # we'll reset the String to BINARY so we can run regular - # expressions against it - template_source = template.source.dup.force_encoding("BINARY") + class ERB < Template::Handler + include Compilable - # Erubis does not have direct support for encodings. - # As a result, we will extract the ERB-style magic - # comment, give the String to Erubis as BINARY data, - # and then tag the resulting String with the extracted - # encoding later - erb = template_source.gsub(ENCODING_TAG, '') - encoding = $2 + ## + # :singleton-method: + # Specify trim mode for the ERB compiler. Defaults to '-'. + # See ERb documentation for suitable values. + cattr_accessor :erb_trim_mode + self.erb_trim_mode = '-' - if !encoding && (template.source.encoding == Encoding::BINARY) - raise WrongEncodingError.new(template_source, Encoding.default_external) - end - end + self.default_format = Mime::HTML - result = self.class.erb_implementation.new( - erb, - :trim => (self.class.erb_trim_mode == "-") - ).src + cattr_accessor :erb_implementation + self.erb_implementation = Erubis - # If an encoding tag was found, tag the String - # we're returning with that encoding. Otherwise, - # return a BINARY String, which is what ERB - # returns. Note that if a magic comment was - # not specified, we will return the data to - # Rails as BINARY, which will then use its - # own encoding logic to create a UTF-8 String. - result = "\n#{result}".force_encoding(encoding).encode if encoding - result - end + def compile(template) + source = template.source.gsub(/\A(<%(#.*coding[:=]\s*(\S+)\s*)-?%>)\s*\n?/, '') + erb = "<% __in_erb_template=true %>#{source}" + result = self.class.erb_implementation.new(erb, :trim=>(self.class.erb_trim_mode == "-")).src + result = "#{$2}\n#{result}" if $2 + result end end end diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index ef44925951..a223b3a55f 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -70,10 +70,7 @@ module ActionView Dir[query].reject { |p| File.directory?(p) }.map do |p| handler, format = extract_handler_and_format(p, formats) - - contents = File.open(p, "rb") {|io| io.read } - - Template.new(contents, File.expand_path(p), handler, + Template.new(File.read(p), File.expand_path(p), handler, :virtual_path => path, :format => format) end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 479e62b23d..89ba0990f1 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -12,10 +12,6 @@ $:.unshift(File.dirname(__FILE__) + '/fixtures/alternate_helpers') ENV['TMPDIR'] = File.join(File.dirname(__FILE__), 'tmp') -if defined?(Encoding.default_internal) - Encoding.default_internal = "UTF-8" -end - require 'test/unit' require 'abstract_controller' require 'action_controller' diff --git a/actionpack/test/controller/assert_select_test.rb b/actionpack/test/controller/assert_select_test.rb index 4f8ad23174..7012c4c9b0 100644 --- a/actionpack/test/controller/assert_select_test.rb +++ b/actionpack/test/controller/assert_select_test.rb @@ -212,12 +212,12 @@ class AssertSelectTest < ActionController::TestCase assert_nothing_raised { assert_select "div", "bar" } assert_nothing_raised { assert_select "div", /\w*/ } assert_nothing_raised { assert_select "div", :text => /\w*/, :count=>2 } - assert_raise(Assertion) { assert_select "div", :text=>"foo", :count=>2 } + assert_raise(Assertion) { assert_select "div", :text=>"foo", :count=>2 } assert_nothing_raised { assert_select "div", :html=>"bar" } assert_nothing_raised { assert_select "div", :html=>"bar" } assert_nothing_raised { assert_select "div", :html=>/\w*/ } assert_nothing_raised { assert_select "div", :html=>/\w*/, :count=>2 } - assert_raise(Assertion) { assert_select "div", :html=>"foo", :count=>2 } + assert_raise(Assertion) { assert_select "div", :html=>"foo", :count=>2 } end end diff --git a/actionpack/test/controller/capture_test.rb b/actionpack/test/controller/capture_test.rb index 47253f22b8..d1dbd535c4 100644 --- a/actionpack/test/controller/capture_test.rb +++ b/actionpack/test/controller/capture_test.rb @@ -68,6 +68,6 @@ class CaptureTest < ActionController::TestCase private def expected_content_for_output - "Putting stuff in the title!\nGreat stuff!" + "Putting stuff in the title!\n\nGreat stuff!" end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 2b1f2a27df..52049f2a8a 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -1079,7 +1079,7 @@ class RenderTest < ActionController::TestCase def test_action_talk_to_layout get :action_talk_to_layout - assert_equal "Talking to the layout\nAction was here!", @response.body + assert_equal "Talking to the layout\n\nAction was here!", @response.body end # :addressed: @@ -1096,7 +1096,7 @@ class RenderTest < ActionController::TestCase def test_yield_content_for assert_not_deprecated { get :yield_content_for } - assert_equal "Putting stuff in the title!\nGreat stuff!\n", @response.body + assert_equal "Putting stuff in the title!\n\nGreat stuff!\n", @response.body end def test_overwritting_rendering_relative_file_with_extension diff --git a/actionpack/test/fixtures/test/content_for.erb b/actionpack/test/fixtures/test/content_for.erb index 1fb829f54c..0e47ca8c3d 100644 --- a/actionpack/test/fixtures/test/content_for.erb +++ b/actionpack/test/fixtures/test/content_for.erb @@ -1 +1,2 @@ -<% content_for :title do -%>Putting stuff in the title!<% end -%>Great stuff! \ No newline at end of file +<% content_for :title do %>Putting stuff in the title!<% end %> +Great stuff! \ No newline at end of file diff --git a/actionpack/test/fixtures/test/content_for_concatenated.erb b/actionpack/test/fixtures/test/content_for_concatenated.erb index e65f629574..fb6b4b05d7 100644 --- a/actionpack/test/fixtures/test/content_for_concatenated.erb +++ b/actionpack/test/fixtures/test/content_for_concatenated.erb @@ -1,3 +1,3 @@ <% content_for :title, "Putting stuff " - content_for :title, "in the title!" -%> + content_for :title, "in the title!" %> Great stuff! \ No newline at end of file diff --git a/actionpack/test/fixtures/test/content_for_with_parameter.erb b/actionpack/test/fixtures/test/content_for_with_parameter.erb index aeb6f73ce0..57aecbac05 100644 --- a/actionpack/test/fixtures/test/content_for_with_parameter.erb +++ b/actionpack/test/fixtures/test/content_for_with_parameter.erb @@ -1,2 +1,2 @@ -<% content_for :title, "Putting stuff in the title!" -%> +<% content_for :title, "Putting stuff in the title!" %> Great stuff! \ No newline at end of file diff --git a/actionpack/test/fixtures/test/non_erb_block_content_for.builder b/actionpack/test/fixtures/test/non_erb_block_content_for.builder index d539a425a4..a94643561c 100644 --- a/actionpack/test/fixtures/test/non_erb_block_content_for.builder +++ b/actionpack/test/fixtures/test/non_erb_block_content_for.builder @@ -1,4 +1,4 @@ content_for :title do 'Putting stuff in the title!' end -xml << "Great stuff!" +xml << "\nGreat stuff!" diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index aca96e0a24..d0212024ae 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -232,13 +232,13 @@ module RenderTestCases # TODO: Move to deprecated_tests.rb def test_render_with_nested_layout_deprecated assert_deprecated do - assert_equal %(title\n\n
column
\n
content
\n), + assert_equal %(title\n\n\n
column
\n
content
\n), @view.render(:file => "test/deprecated_nested_layout.erb", :layout => "layouts/yield") end end def test_render_with_nested_layout - assert_equal %(title\n\n
column
\n
content
\n), + assert_equal %(title\n\n\n
column
\n
content
\n), @view.render(:file => "test/nested_layout.erb", :layout => "layouts/yield") end @@ -284,7 +284,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase with_external_encoding Encoding::ASCII_8BIT do result = @view.render(:file => "test/utf8_magic.html.erb", :layouts => "layouts/yield") assert_equal Encoding::UTF_8, result.encoding - assert_equal "\nРусский \nтекст\n\nUTF-8\nUTF-8\nUTF-8\n", result + assert_equal "Русский текст\n\nUTF-8\nUTF-8\nUTF-8\n", result end end @@ -302,7 +302,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase result = @view.render(:file => "test/utf8.html.erb", :layouts => "layouts/yield") flunk 'Should have raised incompatible encoding error' rescue ActionView::Template::Error => error - assert_match 'Your template was not saved as valid Shift_JIS', error.original_exception.message + assert_match 'invalid byte sequence in Shift_JIS', error.original_exception.message end end end @@ -313,7 +313,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase result = @view.render(:file => "test/utf8_magic_with_bare_partial.html.erb", :layouts => "layouts/yield") flunk 'Should have raised incompatible encoding error' rescue ActionView::Template::Error => error - assert_match 'Your template was not saved as valid Shift_JIS', error.original_exception.message + assert_match 'invalid byte sequence in Shift_JIS', error.original_exception.message end end end diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb deleted file mode 100644 index c4a65d84fc..0000000000 --- a/actionpack/test/template/template_test.rb +++ /dev/null @@ -1,128 +0,0 @@ -require "abstract_unit" - -# These are the normal settings that will be set up by Railties -# TODO: Have these tests support other combinations of these values -Encoding.default_internal = "UTF-8" -Encoding.default_external = "UTF-8" - -class TestERBTemplate < ActiveSupport::TestCase - ERBHandler = ActionView::Template::Handlers::ERB - - class Context - def initialize - @output_buffer = "original" - end - - def hello - "Hello" - end - - def partial - ActionView::Template.new( - "<%= @_virtual_path %>", - "partial", - ERBHandler, - :virtual_path => "partial" - ) - end - - def logger - require "logger" - Logger.new(STDERR) - end - - def my_buffer - @output_buffer - end - end - - def new_template(body = "<%= hello %>", handler = ERBHandler, details = {}) - ActionView::Template.new(body, "hello template", ERBHandler, {:virtual_path => "hello"}) - end - - def render(locals = {}) - @template.render(@obj, locals) - end - - def setup - @obj = Context.new - end - - def test_basic_template - @template = new_template - assert_equal "Hello", render - end - - def test_locals - @template = new_template("<%= my_local %>") - assert_equal "I'm a local", render(:my_local => "I'm a local") - end - - def test_restores_buffer - @template = new_template - assert_equal "Hello", render - assert_equal "original", @obj.my_buffer - end - - def test_virtual_path - @template = new_template("<%= @_virtual_path %>" \ - "<%= partial.render(self, {}) %>" \ - "<%= @_virtual_path %>") - assert_equal "hellopartialhello", render - end - - if "ruby".encoding_aware? - def test_resulting_string_is_utf8 - @template = new_template - assert_equal Encoding::UTF_8, render.encoding - end - - def test_no_magic_comment_word_with_utf_8 - @template = new_template("hello \u{fc}mlat") - assert_equal Encoding::UTF_8, render.encoding - assert_equal "hello \u{fc}mlat", render - end - - # This test ensures that if the default_external - # is set to something other than UTF-8, we don't - # get any errors and get back a UTF-8 String. - def test_default_external_works - Encoding.default_external = "ISO-8859-1" - @template = new_template("hello \xFCmlat") - assert_equal Encoding::UTF_8, render.encoding - assert_equal "hello \u{fc}mlat", render - ensure - Encoding.default_external = "UTF-8" - end - - def test_encoding_can_be_specified_with_magic_comment - @template = new_template("# encoding: ISO-8859-1\nhello \xFCmlat") - assert_equal Encoding::UTF_8, render.encoding - assert_equal "\nhello \u{fc}mlat", render - end - - # TODO: This is currently handled inside ERB. The case of explicitly - # lying about encodings via the normal Rails API should be handled - # inside Rails. - def test_lying_with_magic_comment - assert_raises(ActionView::Template::Error) do - @template = new_template("# encoding: UTF-8\nhello \xFCmlat") - render - end - end - - def test_encoding_can_be_specified_with_magic_comment_in_erb - @template = new_template("<%# encoding: ISO-8859-1 %>hello \xFCmlat") - result = render - assert_equal Encoding::UTF_8, render.encoding - assert_equal "hello \u{fc}mlat", render - end - - def test_error_when_template_isnt_valid_utf8 - assert_raises(ActionView::Template::Error, /\xFC/) do - @template = new_template("hello \xFCmlat") - render - end - end - end -end \ No newline at end of file -- cgit v1.2.3