Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

FormTagHelper generates illegal html if name contains e.g. square brackets

#1238

ActionView::Helpers::FormHelper works perfectly and uses sanitized_object_name() to create the html element id. In contrast, the ActionView::Helpers::InstanceTag methods, e.g. text_field_tag simply pass the name parameter as an id to the tag method, creating invalid html for a valid name parameter:

tag :input, { "type" => "text", "name" => name, "id" => name, ...

According to the documentation of ActionView::Helpers::FormTagHelper#text_field_tag 'name' is expected as parameter. If I provide a legal name, e.g. 'widget[existing_archive_attributes][-1][filename]' an illegal html is created. The id attribute with square brackets is not allowed.

To fix the problem following methods in the FormTagHelper should be adjusted: select_tag, text_field_tag, label_tag, check_box_tag. For example, as follows:

  def select_tag(name, option_tags = nil, options = {})
    content_tag :select, option_tags, { "name" => name, "id" => sanitize_to_id(name) }.update(options.stringify_keys)
  end

  def text_field_tag(name, value = nil, options = {})
    tag :input, { "type" => "text", "name" => name, "id" => sanitize_to_id(name), "value" => value }.update(options.stringify_keys)
  end

  # hidden_field_tag is based on text_field_tag so no patching needed

  def label_tag(name, text = nil, options = {})
    content_tag :label, text || name.humanize, { "for" => sanitize_to_id(name) }.update(options.stringify_keys)
  end

  def check_box_tag(name, value = "1", checked = false, options = {})
    html_options = { "type" => "checkbox", "name" => name, "id" => sanitize_to_id(name), "value" => value }.update(options.stringify_keys)
    html_options["checked"] = "checked" if checked
    tag :input, html_options
  end

  private

  def sanitize_to_id(name)
    name.to_s.gsub(/[^-a-zA-Z0-9:.]/, "_").sub(/_$/, "")
  end

Reported by Vladimir Dobriakov · October 20th, 2008 @ 04:32 PM

State: committed
Milestone: 2.x
Assigned to: nobody
Importance: none

Activity

  1. DHH
    DHH

    Please turn this into a real patch with tests. Thanks!

    October 30th, 2008 @ 11:51 AM

  2. Vladimir Dobriakov
    Vladimir Dobriakov
    • Tag changed from actionview, helper to actionview, helper, patch

    Attached the patch.

    Please also see http://www.w3.org/TR/html4/types... for the background.

    Thanks!

    November 4th, 2008 @ 01:32 PM

  3. Repository
    Repository
    • State changed from new to committed

    (from [5fad229e43e2b2541ed39c6ef571975176e6a8d2]) Fixed that FormTagHelper generates illegal html if name contains e.g. square brackets [#1238 FormTagHelper generates illegal html if name contains e.g. square brackets state:committed]

    Signed-off-by: David Heinemeier Hansson david@loudthinking.com http://github.com/rails/rails/co...

    November 4th, 2008 @ 05:25 PM

  4. Ken Collins
    Ken Collins

    After upgrading to rails 1.2.6 to 2.2, this broke a lot of my manual hidden form tags where I had [] in the IDs. :(

    February 12th, 2009 @ 08:35 PM

  5. Vladimir Dobriakov
    Vladimir Dobriakov

    @Ken:

    This fix only changes the way the ids are automatically calculated. Now they are created according to the HTML 4.01 specification. If you manually provide a name and an id to the hidden_field_tag, then attributes provided by you will be used.

    BTW, brackets are not allowed in the id:

    ID and NAME tokens must begin with a letter ([A-Za-z]) and may be followed by any number of letters, digits ([0-9]), hyphens ("-"), underscores ("_), colons (), and periods (".").

    Source: http://www.w3.org/TR/html4/types...

    February 16th, 2009 @ 07:53 AM

  6. Ken Collins
    Ken Collins

    Until this is fixed, it's best to pass in the :id HTML option. For instance

    
    
    select_tag 'note[to_group_id]', select_options, :id => 'user_groups_select'...
    select_tag 'parameters[foobar]', options_for_select(...), :id => 'foobar_select'...
    
    

    April 15th, 2009 @ 09:16 PM