Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Utf8 enforcer param customization #549

Merged
merged 4 commits into from

5 participants

@dlee

This allows you to change the parameter name of the UTF8 enforcer tag in Rails generated forms.

@josevalim
Owner

I am -1 for the customization option. Can't it simply be a method that you would override in ApplicationHelper? :D

@dlee

This would allow users to set the option and know that it'll work instead of having to monkey patch not knowing when the internals of ApplicationHelper would change. A big part of Rails3 was to reduce the number of monkey patching by providing supported customizable options, and I think this change goes with that spirit.

@josevalim
Owner

@dlee I completely disagree with the argument about Rails 3. The number of configuration options hardly changed at all in Rails 3. Instead, we are using well defined methods and APIs, so you can include modules and customize Rails behavior by redefining methods with the appropriate behavior. Inheritance != monkey patching.

@dlee

Ah, I misunderstood "ApplicationHelper" as "ActionView::Helpers", my bad.

I agree that this can be also be solved with well defined overridable methods. I have two arguments for going with the config option:

  • There is already a precedent set with request_forgery_protection_token.
  • Setting a config option is the natural way to customize a value of some option. Method override is the natural way to customize the logic.

I'm not too opposed to the overrideable method idea, but I think the config option is a bit cleaner, consistent with precedence, and feels more natural than overriding a method.

@lapluviosilla

Furthermore, who would ever not want a snowman on their page? Seriously?

@dlee

@lapluviosilla Sadly, the snowman has melted away in favor of a checkmark some months ago. :'(

But this patch is not talking about about the utf8 value but the name of the hidden field input.

@lapluviosilla

Damn, I did not catch that.

+1 for renaming to a more appropriate name now that the snowman is not used.
-1 for adding a config customization option. Overriding in ApplicationHelper is fine.

@dlee

What are the reasons you don't want the customization option? I've given two reasons in my second comment that I think should be compelling, and reasons against having the option should outweigh those reasons.

@josevalim
Owner

Reasons for a method: flexibility. What if you want to customize the name just for some specific controllers or under special circumstances?

"Setting a config option is the natural way to customize a value of some option. Method override is the natural way to customize the logic."

The rails version might have no logic, but that does not mean we should block applications from building a logic on top of that. If I could, I would make many of the customization options methods. Next week on Railsconf I will talk exactly about the issue with configuration options as it is in Rails today.

@dlee

@josevalim I pushed a commit that does what you want for additional flexibility.

@NZKoz
Owner

@dlee José was asking for a method for utf8_enforcer_param instead of the config option, not what you've done here.

@dlee

@NZKoz I think what I've done here satisfies both our requirements: one for simplicity/consistency and the other for flexibility.

@josevalim
Owner

The whole point is to remove the need for a configuration option. Again, we should be removing configuration options instead of adding them.

@dlee

@josevalim alright, I guess you won't pull until I remove the configuration option. I'll try getting something in soon.

Also, can you provide a link to your Railsconf talk? I'm curious what the reasons are for moving away from configuration options. I saw the presentation notes PDF, but it didn't mention anything about configuration options.

@dlee

@josevalim: Just a reminder so that this doesn't fall through the cracks and gets harder to merge.

@josevalim
Owner

Thanks for reminding me, could you please rebase? I cannot merge it already. :(

@dlee

@josevalim I just rebased... hopefully it merges cleanly.

@josevalim josevalim merged commit 28f2b98 into rails:master
@djwonk

I was looking for an example of how to take advantage of the utf8_enforcer_tag method in a custom form builder (corrected in a recent edit). Any recommendations? http://stackoverflow.com/questions/16011285/how-to-override-utf8-enforcer-tag-in-rails

@djwonk

As I understand it, the only option is to override utf8_enforcer_tag globally. Right?

Even so, I could test controller_name and action_name. (Not bad, but not quite what I want.)

I'd like to be able to check the context of what form I'm inside as well. In particular, I'd like to check if a custom form builder is being used. How could I do this?

After looking at self inside utf8_enforcer_tag, I don't see a way to do it. If a new design is considered, I would suggest the following: since utf8_enforcer_tag is applied at the form level, it makes sense to allow it to be overridden in a way that is form-specific.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
2  actionpack/lib/action_view/helpers/controller_helper.rb
@@ -20,4 +20,4 @@ def assign_controller(controller)
end
end
end
-end
+end
View
19 actionpack/lib/action_view/helpers/form_tag_helper.rb
@@ -17,6 +17,13 @@ module FormTagHelper
include UrlHelper
include TextHelper
+ # You can change what the name will be for the hidden tag that forces utf8
+ # encoding for forms generated with Rails form helpers.
+ #
+ # ActionView::Helpers::FormTagHelper.utf8_enforcer_param = "_unicode"
+ mattr_accessor :utf8_enforcer_param
+ @@utf8_enforcer_param = "utf8"
+
# Starts a form tag that points the action to an url configured with <tt>url_for_options</tt> just like
# ActionController::Base#url_for. The method for the form defaults to POST.
#
@@ -597,6 +604,13 @@ def range_field_tag(name, value = nil, options = {})
number_field_tag(name, value, options.stringify_keys.update("type" => "range"))
end
+ # Creates the hidden UTF8 enforcer tag. Override this method in a helper
+ # to customize the tag. If you just need to change the field name, set the
+ # +config.action_view.utf8_enforcer_param+ configuration option instead.
+ def utf8_enforcer_tag
+ tag(:input, :type => "hidden", :name => utf8_enforcer_param, :value => "&#x2713;".html_safe)
+ end
+
private
def html_options_for_form(url_for_options, options, *parameters_for_url)
options.stringify_keys.tap do |html_options|
@@ -611,9 +625,6 @@ def html_options_for_form(url_for_options, options, *parameters_for_url)
end
def extra_tags_for_form(html_options)
- snowman_tag = tag(:input, :type => "hidden",
- :name => "utf8", :value => "&#x2713;".html_safe)
-
authenticity_token = html_options.delete("authenticity_token")
method = html_options.delete("method").to_s
@@ -629,7 +640,7 @@ def extra_tags_for_form(html_options)
tag(:input, :type => "hidden", :name => "_method", :value => method) + token_tag(authenticity_token)
end
- tags = snowman_tag << method_tag
+ tags = utf8_enforcer_tag << method_tag
content_tag(:div, tags, :style => 'margin:0;padding:0;display:inline')
end
View
4 actionpack/test/template/form_helper_test.rb
@@ -1890,7 +1890,7 @@ def test_form_for_with_labelled_builder
assert_dom_equal expected, output_buffer
end
- def snowman(method = nil)
+ def hidden_fields(method = nil)
txt = %{<div style="margin:0;padding:0;display:inline">}
txt << %{<input name="utf8" type="hidden" value="&#x2713;" />}
if method && !method.to_s.in?(['get', 'post'])
@@ -1918,7 +1918,7 @@ def whole_form(action = "/", id = nil, html_class = nil, options = nil)
method = options
end
- form_text(action, id, html_class, remote, multipart, method) + snowman(method) + contents + "</form>"
+ form_text(action, id, html_class, remote, multipart, method) + hidden_fields(method) + contents + "</form>"
end
def test_default_form_builder
View
4 actionpack/test/template/form_tag_helper_test.rb
@@ -9,7 +9,7 @@ def setup
@controller = BasicController.new
end
- def snowman(options = {})
+ def hidden_fields(options = {})
method = options[:method]
txt = %{<div style="margin:0;padding:0;display:inline">}
@@ -34,7 +34,7 @@ def form_text(action = "http://www.example.com", options = {})
end
def whole_form(action = "http://www.example.com", options = {})
- out = form_text(action, options) + snowman(options)
+ out = form_text(action, options) + hidden_fields(options)
if block_given?
out << yield << "</form>"
View
2  railties/guides/source/configuring.textile
@@ -330,7 +330,7 @@ And can reference in the view with the following code:
<%= stylesheet_link_tag :special %>
</ruby>
-* +ActionView::Helpers::AssetTagHelper::AssetPaths.cache_asset_ids+ With the cache enabled, the asset tag helper methods will make fewer expensive file system calls (the default implementation checks the file system timestamp). However this prevents you from modifying any asset files while the server is running.
+* +config.action_view.cache_asset_ids+ With the cache enabled, the asset tag helper methods will make fewer expensive file system calls (the default implementation checks the file system timestamp). However this prevents you from modifying any asset files while the server is running.
h4. Configuring Action Mailer
Something went wrong with that request. Please try again.