Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Switched to use `display:none` in extra_tags_for_form method. #6608

Closed
wants to merge 1 commit into from
@gaelian

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.

Closes #6403.

@gaelian gaelian Switched to use `display:none` in extra_tags_for_form method.
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.

Closes #6403.
d5f6fdb
@spastorino
Owner

/cc @jeremy

@drogus
Collaborator

I like that, but we need to know which browsers (if any) stop sending those hidden fields if they're inside display: none element.

@gaelian

@drogus, this point was discussed in issue #6403 where @pixeltrix mentioned that failure to send the hidden fields due to display:none being set on the containing div was only a historical problem now. Perhaps he can shed some definite light on this matter.

@drogus
Collaborator

@gaellan I missed that, thanks for clarification. If it's the case, I'm fine with merging it.

@gaelian

@drogus cool. No worries.

@getnashty

Any word on this? I'd like the functionality

@rafaelfranca

Some browsers still doesn't send the data inside display:none elements. We need to investigate better.

@gaelian

@rafaelfranca which browsers are the ones suspected to still be causing trouble?

@rafaelfranca

Firefox 12

@gaelian

I see. And later versions of FF do not exhibit the same behaviour?

@rafaelfranca

I don't know. We need to investigate. I tried in the same version to reproduce but without success.

@pixeltrix
Owner

@rafaelfranca who reported the FF12 failure? There are some issues regarding invalid HTML5 form elements that are hidden:

https://bugzilla.mozilla.org/show_bug.cgi?id=595451
https://bugzilla.mozilla.org/show_bug.cgi?id=693025
https://bugzilla.mozilla.org/show_bug.cgi?id=724537

I wonder if this was the cause of their failure?

@rafaelfranca

One guy at the Brazilian ruby on rails mailing list (I hope the google portuguese-english translation can help). In that case he was using radio buttons.

@pixeltrix
Owner

Sounds like the category_id field is a belongs_to association and if it's set to :null => false in schema.rb Formtastic will mark it as required - so it could fall into the being the bug described above.

@rafaelfranca

Yes, make sense.

@schneems
Collaborator

Any update on this?

@mhuggins

Sorry if this is a dumb question, but I've always wondered why Rails does this:

<div style="margin:0;padding:0;display:none">
  <input name="authenticity_token" type="hidden" value="NrOp5bsjoLRuK8IW5+dQEYjKGUJDe7TQoZVvq95Wteg=" />
</div>

Instead of this:

<input name="authenticity_token" type="hidden" value="NrOp5bsjoLRuK8IW5+dQEYjKGUJDe7TQoZVvq95Wteg=" />

In other words, why even wrap hidden inputs inside a div in the first place?

@carlosantoniodasilva

@mhuggins it's "kinda similar" to what @pixeltrix explained here: #6403 (comment). For historical reasons, it's due to some browser weirdness and rendering quirks with hidden inputs.

@steveklabnik
Collaborator

So, did we ever come to any consensus, @pixeltrix @rafaelfranca ?

@eval

Triage fairy: any update on this?

@al2o3cr

@mhuggins - late to the discussion, but the biggest reason the input tags are wrapped in a block-level element is that the old HTML 4.01 Strict doctype required that wrapping - bare input tags inside a form were not valid.

@pixeltrix
Owner

Closed by 7a085da

@gaelian thanks for your contribution!

@pixeltrix pixeltrix closed this
@jumski

Guys I'm struggling how to get following form to display properly:

.row
  = form_for @form, url: admin_copies_path do |f|
    .span6
      = render 'some/partial/with/fields'
    .span6
      = render 'some/other/partial/with/fields'

I'm using bootstrap 2 and it cant handle any additional element before my span6's - what I see is columns displaying vertically instead of horizontally.

Any ideas how to elegantly move this additional fields to the end of the form?

Thanks in advance!

@pixeltrix
Owner

@jumski try putting the form around the row - is bootstrap using :first or an adjacent selector?

@jumski

@pixeltrix wow didn't expected so fast response :)

after fiddling with reopening FormTagHelper and overriding form methods I had an aha moment and tried exacly what you proposed.

Guest what - problem solved!

Thanks a lot for you support!

@chancancode chancancode referenced this pull request from a commit
@gaelian gaelian Switched to use `display:none` in extra_tags_for_form method.
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
7a085da
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 3, 2012
  1. @gaelian

    Switched to use `display:none` in extra_tags_for_form method.

    gaelian authored
    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.
    
    Closes #6403.
This page is out of date. Refresh to see the latest.
View
6 actionpack/lib/action_view/helpers/form_helper.rb
@@ -55,7 +55,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="margin:0;padding:0;display:none">
# <input name="authenticity_token" type="hidden" value="NrOp5bsjoLRuK8IW5+dQEYjKGUJDe7TQoZVvq95Wteg=" />
# </div>
# <label for="person_first_name">First name</label>:
@@ -85,7 +85,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="margin:0;padding:0;display:none">
# <input name="_method" type="hidden" value="put" />
# <input name="authenticity_token" type="hidden" value="NrOp5bsjoLRuK8IW5+dQEYjKGUJDe7TQoZVvq95Wteg=" />
# </div>
@@ -323,7 +323,7 @@ def convert_to_model(object)
# 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='margin:0;padding:0;display:none'>
# <input name='_method' type='hidden' value='put' />
# </div>
# ...
View
2  actionpack/lib/action_view/helpers/form_tag_helper.rb
@@ -705,7 +705,7 @@ def extra_tags_for_form(html_options)
end
tags = utf8_enforcer_tag << method_tag
- content_tag(:div, tags, :style => 'margin:0;padding:0;display:inline')
+ content_tag(:div, tags, :style => 'margin:0;padding:0;display:none')
end
def form_tag_html(html_options)
View
2  actionpack/test/template/form_helper_test.rb
@@ -2654,7 +2654,7 @@ def initialize(object_name, object, template, options, block)
protected
def hidden_fields(method = nil)
- txt = %{<div style="margin:0;padding:0;display:inline">}
+ txt = %{<div style="margin:0;padding:0;display:none">}
txt << %{<input name="utf8" type="hidden" value="&#x2713;" />}
if method && !method.to_s.in?(['get', 'post'])
txt << %{<input name="_method" type="hidden" value="#{method}" />}
View
2  actionpack/test/template/form_tag_helper_test.rb
@@ -14,7 +14,7 @@ def setup
def hidden_fields(options = {})
method = options[:method]
- txt = %{<div style="margin:0;padding:0;display:inline">}
+ txt = %{<div style="margin:0;padding:0;display:none">}
txt << %{<input name="utf8" type="hidden" value="&#x2713;" />}
if method && !method.to_s.in?(['get','post'])
txt << %{<input name="_method" type="hidden" value="#{method}" />}
Something went wrong with that request. Please try again.