From 4cbb9db0a5ff65b0a626a5b043331abefd89e717 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Sun, 31 Jan 2010 19:17:42 -0800 Subject: For performance reasons, you can no longer call html_safe! on Strings. Instead, all Strings are always not html_safe?. Instead, you can get a SafeBuffer from a String by calling #html_safe, which will SafeBuffer.new(self). * Additionally, instead of doing concat("".html_safe), you can do safe_concat(""), which will skip both the flag set, and the flag check. * For the first pass, I converted virtually all #html_safe!s to #html_safe, and the tests pass. A further optimization would be to try to use #safe_concat as much as possible, reducing the performance impact if we know up front that a String is safe. --- actionpack/lib/action_view/helpers/active_model_helper.rb | 11 +++++------ actionpack/lib/action_view/helpers/asset_tag_helper.rb | 6 +++--- actionpack/lib/action_view/helpers/date_helper.rb | 14 +++++++------- actionpack/lib/action_view/helpers/debug_helper.rb | 4 ++-- actionpack/lib/action_view/helpers/form_helper.rb | 4 ++-- actionpack/lib/action_view/helpers/form_options_helper.rb | 2 +- actionpack/lib/action_view/helpers/form_tag_helper.rb | 6 +++--- actionpack/lib/action_view/helpers/number_helper.rb | 2 +- actionpack/lib/action_view/helpers/prototype_helper.rb | 2 +- actionpack/lib/action_view/helpers/raw_output_helper.rb | 2 +- actionpack/lib/action_view/helpers/sanitize_helper.rb | 12 ++---------- actionpack/lib/action_view/helpers/tag_helper.rb | 8 ++++---- actionpack/lib/action_view/helpers/text_helper.rb | 10 +++++----- actionpack/lib/action_view/helpers/translation_helper.rb | 2 +- actionpack/lib/action_view/helpers/url_helper.rb | 8 ++++---- 15 files changed, 42 insertions(+), 51 deletions(-) (limited to 'actionpack/lib/action_view/helpers') diff --git a/actionpack/lib/action_view/helpers/active_model_helper.rb b/actionpack/lib/action_view/helpers/active_model_helper.rb index 87b7adf6c4..e106bb0897 100644 --- a/actionpack/lib/action_view/helpers/active_model_helper.rb +++ b/actionpack/lib/action_view/helpers/active_model_helper.rb @@ -6,7 +6,7 @@ require 'active_support/core_ext/kernel/reporting' module ActionView class Base - @@field_error_proc = Proc.new{ |html_tag, instance| "
#{html_tag}
".html_safe! } + @@field_error_proc = Proc.new{ |html_tag, instance| "
#{html_tag}
".html_safe } cattr_accessor :field_error_proc end @@ -86,12 +86,11 @@ module ActionView submit_value = options[:submit_value] || options[:action].gsub(/[^\w]/, '').capitalize contents = form_tag({:action => action}, :method =>(options[:method] || 'post'), :enctype => options[:multipart] ? 'multipart/form-data': nil) - contents << hidden_field(record_name, :id) unless record.new_record? - contents << all_input_tags(record, record_name, options) + contents.safe_concat hidden_field(record_name, :id) unless record.new_record? + contents.safe_concat all_input_tags(record, record_name, options) yield contents if block_given? - contents << submit_tag(submit_value) - contents << '' - contents.html_safe! + contents.safe_concat submit_tag(submit_value) + contents.safe_concat('') end # Returns a string containing the error message attached to the +method+ on the +object+ if one exists. diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 4df99f8293..96976ce45f 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -293,7 +293,7 @@ module ActionView else sources = expand_javascript_sources(sources, recursive) ensure_javascript_sources!(sources) if cache - sources.collect { |source| javascript_src_tag(source, options) }.join("\n").html_safe! + sources.collect { |source| javascript_src_tag(source, options) }.join("\n").html_safe end end @@ -444,7 +444,7 @@ module ActionView else sources = expand_stylesheet_sources(sources, recursive) ensure_stylesheet_sources!(sources) if cache - sources.collect { |source| stylesheet_tag(source, options) }.join("\n").html_safe! + sources.collect { |source| stylesheet_tag(source, options) }.join("\n").html_safe end end @@ -588,7 +588,7 @@ module ActionView if sources.is_a?(Array) content_tag("video", options) do - sources.map { |source| tag("source", :src => source) }.join.html_safe! + sources.map { |source| tag("source", :src => source) }.join.html_safe end else options[:src] = path_to_video(sources) diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index 34f38b0a8a..8be2f76bd6 100644 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -616,7 +616,7 @@ module ActionView build_selects_from_types(order) else - "#{select_date}#{@options[:datetime_separator]}#{select_time}".html_safe! + "#{select_date}#{@options[:datetime_separator]}#{select_time}".html_safe end end @@ -835,7 +835,7 @@ module ActionView select_html << prompt_option_tag(type, @options[:prompt]) + "\n" if @options[:prompt] select_html << select_options_as_html.to_s - (content_tag(:select, select_html, select_options) + "\n").html_safe! + (content_tag(:select, select_html, select_options) + "\n").html_safe end # Builds a prompt option tag with supplied options or from default options @@ -865,7 +865,7 @@ module ActionView :id => input_id_from_type(type), :name => input_name_from_type(type), :value => value - }) + "\n").html_safe! + }) + "\n").html_safe end # Returns the name attribute for the input tag @@ -896,7 +896,7 @@ module ActionView separator = separator(type) unless type == order.first # don't add on last field select.insert(0, separator.to_s + send("select_#{type}").to_s) end - select.html_safe! + select.html_safe end # Returns the separator for a given datetime component @@ -916,15 +916,15 @@ module ActionView class InstanceTag #:nodoc: def to_date_select_tag(options = {}, html_options = {}) - datetime_selector(options, html_options).select_date.html_safe! + datetime_selector(options, html_options).select_date.html_safe end def to_time_select_tag(options = {}, html_options = {}) - datetime_selector(options, html_options).select_time.html_safe! + datetime_selector(options, html_options).select_time.html_safe end def to_datetime_select_tag(options = {}, html_options = {}) - datetime_selector(options, html_options).select_datetime.html_safe! + datetime_selector(options, html_options).select_datetime.html_safe end private diff --git a/actionpack/lib/action_view/helpers/debug_helper.rb b/actionpack/lib/action_view/helpers/debug_helper.rb index 885945fde3..e637dc1474 100644 --- a/actionpack/lib/action_view/helpers/debug_helper.rb +++ b/actionpack/lib/action_view/helpers/debug_helper.rb @@ -27,10 +27,10 @@ module ActionView def debug(object) begin Marshal::dump(object) - "
#{h(object.to_yaml).gsub("  ", "  ")}
".html_safe! + "
#{h(object.to_yaml).gsub("  ", "  ")}
".html_safe rescue Exception => e # errors from Marshal or YAML # Object couldn't be dumped, perhaps because of singleton methods -- this is the fallback - "#{h(object.inspect)}".html_safe! + "#{h(object.inspect)}".html_safe end end end diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index a72357cf3d..76998a6ea1 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -311,7 +311,7 @@ module ActionView concat(form_tag(options.delete(:url) || {}, options.delete(:html) || {})) fields_for(object_name, *(args << options), &proc) - concat(''.html_safe!) + safe_concat('') end def apply_form_for_options!(object_or_array, options) #:nodoc: @@ -879,7 +879,7 @@ module ActionView end hidden = tag("input", "name" => options["name"], "type" => "hidden", "value" => options['disabled'] && checked ? checked_value : unchecked_value) checkbox = tag("input", options) - (hidden + checkbox).html_safe! + (hidden + checkbox).html_safe end def to_boolean_select_tag(options = {}) diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index 02ad637509..0ffe770f5f 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -296,7 +296,7 @@ module ActionView options << %() end - options_for_select.join("\n").html_safe! + options_for_select.join("\n").html_safe end # Returns a string of option tags that have been compiled by iterating over the +collection+ and assigning the diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb index 597cf311cb..9b8471c6c6 100644 --- a/actionpack/lib/action_view/helpers/form_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb @@ -446,7 +446,7 @@ module ActionView concat(tag(:fieldset, options, true)) concat(content_tag(:legend, legend)) unless legend.blank? concat(content) - concat("".html_safe!) + safe_concat("") end private @@ -474,14 +474,14 @@ module ActionView def form_tag_html(html_options) extra_tags = extra_tags_for_form(html_options) - (tag(:form, html_options, true) + extra_tags).html_safe! + (tag(:form, html_options, true) + extra_tags).html_safe end def form_tag_in_block(html_options, &block) content = capture(&block) concat(form_tag_html(html_options)) concat(content) - concat("".html_safe!) + safe_concat("") end def token_tag diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb index 64b71663c3..3d3502a08b 100644 --- a/actionpack/lib/action_view/helpers/number_helper.rb +++ b/actionpack/lib/action_view/helpers/number_helper.rb @@ -92,7 +92,7 @@ module ActionView :precision => precision, :delimiter => delimiter, :separator => separator) - ).gsub(/%u/, unit).html_safe! + ).gsub(/%u/, unit).html_safe rescue number end diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb index d335d89274..7eb6bceca0 100644 --- a/actionpack/lib/action_view/helpers/prototype_helper.rb +++ b/actionpack/lib/action_view/helpers/prototype_helper.rb @@ -610,7 +610,7 @@ module ActionView # page.hide 'spinner' # end def update_page(&block) - JavaScriptGenerator.new(@template, &block).to_s.html_safe! + JavaScriptGenerator.new(@template, &block).to_s.html_safe end # Works like update_page but wraps the generated JavaScript in a