Skip to content

Commit

Permalink
Fix how file_ and password_field_tag edit options
Browse files Browse the repository at this point in the history
This commit fixes the behavior of `file_field_tag` and `password_field_tag`
when invoked with a hash of options.

These two helpers are different from all the other ones in that they modify the
options hash passed as a parameter, whereas all the other helpers duplicate it
before updating it.

The result is that *bad things* can happen if the user re-uses the same hash.
For instance, users who write the following code to display a file field
followed by a text field (both with the same class):

```rhtml
<% options = {class: 'important'} %>
<%= file_field_tag 'Upload', options %>
<%= text_field_tag 'Name', options %>
```

would instead see **two file fields!**

```html
<input class="important" id="Upload" name="Upload" type="file">
<input class="important" id="Name" name="Name" type="file" value="value">
```

This PR replaces `update` with `merge` in the code of the two helpers,
fixing the issue above.

The included test verifies the change, since it passes after this PR, but
fails before with the following error:

```
Expected: <input type="text" name="title" id="title" value="Hello!" class="important" />
Actual: <input type="password" name="title" id="title" value="Hello!" class="important" />
```
  • Loading branch information
claudiob committed Oct 15, 2014
1 parent c86957b commit dee00df
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 2 deletions.
4 changes: 2 additions & 2 deletions actionview/lib/action_view/helpers/form_tag_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ def hidden_field_tag(name, value = nil, options = {})
# file_field_tag 'file', accept: 'text/html', class: 'upload', value: 'index.html'
# # => <input accept="text/html" class="upload" id="file" name="file" type="file" value="index.html" />
def file_field_tag(name, options = {})
text_field_tag(name, nil, options.update("type" => "file"))
text_field_tag(name, nil, options.merge(type: :file))
end

# Creates a password field, a masked text field that will hide the users input behind a mask character.
Expand Down Expand Up @@ -296,7 +296,7 @@ def file_field_tag(name, options = {})
# password_field_tag 'pin', '1234', maxlength: 4, size: 6, class: "pin_input"
# # => <input class="pin_input" id="pin" maxlength="4" name="pin" size="6" type="password" value="1234" />
def password_field_tag(name = "password", value = nil, options = {})
text_field_tag(name, value, options.update("type" => "password"))
text_field_tag(name, value, options.merge(type: :password))
end

# Creates a text input area; use a textarea for longer text inputs such as blog posts or descriptions.
Expand Down
7 changes: 7 additions & 0 deletions actionview/test/template/form_tag_helper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,13 @@ def test_password_field_tag
assert_dom_equal expected, actual
end

def test_multiple_field_tags_with_same_options
options = {class: 'important'}
assert_dom_equal %(<input name="title" type="file" id="title" class="important"/>), file_field_tag("title", options)
assert_dom_equal %(<input type="password" name="title" id="title" value="Hello!" class="important" />), password_field_tag("title", "Hello!", options)
assert_dom_equal %(<input type="text" name="title" id="title" value="Hello!" class="important" />), text_field_tag("title", "Hello!", options)
end

def test_radio_button_tag
actual = radio_button_tag "people", "david"
expected = %(<input id="people_david" name="people" type="radio" value="david" />)
Expand Down

0 comments on commit dee00df

Please sign in to comment.