Skip to content

Commit

Permalink
Switched to use display:none in extra_tags_for_form method.
Browse files Browse the repository at this point in the history
The use of `display:inline` with the content_tag call in the
extra_tags_for_form method potentially causes display issues with some
browsers, namely Internet Explorer. IE's behaviour of not collapsing
the line height on divs with ostensibly no content means that the
automatically added div containing the hidden authenticity_token, utf8
and _method form input tags may interfere with other visible form
elements in certain circumstances. The use of `display:none` rather
than `display:inline` fixes this problem.

Fixes #6403
  • Loading branch information
gaelian authored and pixeltrix committed Jan 5, 2014
1 parent 6b54883 commit 7a085da
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 8 deletions.
6 changes: 6 additions & 0 deletions actionview/CHANGELOG.md
@@ -1,3 +1,9 @@
* Use `display:none` instead of `display:inline` for hidden fields

Fixes #6403

*Gaelian Ditchburn*

* The `video_tag` helper accepts a number as `:size`

The `:size` option of the `video_tag` helper now can be specified
Expand Down
8 changes: 4 additions & 4 deletions actionview/lib/action_view/helpers/form_helper.rb
Expand Up @@ -51,7 +51,7 @@ module Helpers
# The HTML generated for this would be (modulus formatting):
#
# <form action="/people" class="new_person" id="new_person" method="post">
# <div style="margin:0;padding:0;display:inline">
# <div style="display:none">
# <input name="authenticity_token" type="hidden" value="NrOp5bsjoLRuK8IW5+dQEYjKGUJDe7TQoZVvq95Wteg=" />
# </div>
# <label for="person_first_name">First name</label>:
Expand Down Expand Up @@ -81,7 +81,7 @@ module Helpers
# the code above as is would yield instead:
#
# <form action="/people/256" class="edit_person" id="edit_person_256" method="post">
# <div style="margin:0;padding:0;display:inline">
# <div style="display:none">
# <input name="_method" type="hidden" value="patch" />
# <input name="authenticity_token" type="hidden" value="NrOp5bsjoLRuK8IW5+dQEYjKGUJDe7TQoZVvq95Wteg=" />
# </div>
Expand Down Expand Up @@ -315,7 +315,7 @@ module FormHelper
# The HTML generated for this would be:
#
# <form action='http://www.example.com' method='post' data-remote='true'>
# <div style='margin:0;padding:0;display:inline'>
# <div style='display:none'>
# <input name='_method' type='hidden' value='patch' />
# </div>
# ...
Expand All @@ -333,7 +333,7 @@ module FormHelper
# The HTML generated for this would be:
#
# <form action='http://www.example.com' method='post' data-behavior='autosave' name='go'>
# <div style='margin:0;padding:0;display:inline'>
# <div style='display:none'>
# <input name='_method' type='hidden' value='patch' />
# </div>
# ...
Expand Down
2 changes: 1 addition & 1 deletion actionview/lib/action_view/helpers/form_tag_helper.rb
Expand Up @@ -722,7 +722,7 @@ def extra_tags_for_form(html_options)

enforce_utf8 = html_options.delete("enforce_utf8") { true }
tags = (enforce_utf8 ? utf8_enforcer_tag : ''.html_safe) << method_tag
content_tag(:div, tags, :style => 'margin:0;padding:0;display:inline')
content_tag(:div, tags, :style => 'display:none')
end

def form_tag_html(html_options)
Expand Down
Expand Up @@ -59,7 +59,7 @@ def test_nested_fields_for_with_child_index_option_override_on_a_nested_attribut
protected

def hidden_fields(method = nil)
txt = %{<div style="margin:0;padding:0;display:inline">}
txt = %{<div style="display:none">}
txt << %{<input name="utf8" type="hidden" value="&#x2713;" />}
if method && !%w(get post).include?(method.to_s)
txt << %{<input name="_method" type="hidden" value="#{method}" />}
Expand Down
2 changes: 1 addition & 1 deletion actionview/test/template/form_helper_test.rb
Expand Up @@ -3023,7 +3023,7 @@ def test_form_for_only_instantiates_builder_once
protected

def hidden_fields(method = nil)
txt = %{<div style="margin:0;padding:0;display:inline">}
txt = %{<div style="display:none">}
txt << %{<input name="utf8" type="hidden" value="&#x2713;" />}
if method && !%w(get post).include?(method.to_s)
txt << %{<input name="_method" type="hidden" value="#{method}" />}
Expand Down
2 changes: 1 addition & 1 deletion actionview/test/template/form_tag_helper_test.rb
Expand Up @@ -14,7 +14,7 @@ def hidden_fields(options = {})
method = options[:method]
enforce_utf8 = options.fetch(:enforce_utf8, true)

txt = %{<div style="margin:0;padding:0;display:inline">}
txt = %{<div style="display:none">}
txt << %{<input name="utf8" type="hidden" value="&#x2713;" />} if enforce_utf8
if method && !%w(get post).include?(method.to_s)
txt << %{<input name="_method" type="hidden" value="#{method}" />}
Expand Down

3 comments on commit 7a085da

@jcoglan
Copy link
Contributor

@jcoglan jcoglan commented on 7a085da Sep 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change prevents Capybara's has_css matcher from finding any hidden form inputs. Why is this container div necessary when all it does is contain hidden inputs?

@jcoglan
Copy link
Contributor

@jcoglan jcoglan commented on 7a085da Sep 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, disregard my previous comment, I didn't know about has_css(selector, visible: false).

@chancancode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#6608

Protip: paste the commit sha in the search box on the top to find the associated pull request

Please sign in to comment.