Invalid XHTML form generation on nested forms #398

Closed
aminm opened this Issue Oct 16, 2010 · 18 comments

Projects

None yet

3 participants

aminm commented Oct 16, 2010

We followed instructions and code sample at:

ASCIIcasts, Episode 196 and 197: Nested Model Form:

http://asciicasts.com/episodes/196-nested-model-form-part-1
http://asciicasts.com/episodes/197-nested-model-form-part-2

We want to use nested forms to implement user profiles with multiple phone numbers but the markup generated by Rails/Formtastic is invalid due to their placing a hidden input element outside an

  • tag and directly under
      in edit mode after we have previoulsy entered a phone (3 is database row id):

      See below for details. We are on Rails 3.0. Is this a formtatic or Rails bug?

      The files are as follows:


      _form.html.haml
      ....
      = form.inputs do
      = form.semantic_fields_for :profile_phones do |phone|
      = render 'profile_phone_fields', :form => phone
      = link_to_add_fields "Add Phone", form, :profile_phones, "addPhone"
      ...


      _profile_phone_fields.haml

      = form.input :number, :label => false
      = form.input :_destroy, :label => "Remove Phone", :as => :boolean


      user_profile_helper.rb

      module UserProfileHelper

      def link_to_add_fields(name, f, association)
      new_object = f.object.class.reflect_on_association(association).klass.new
      fields = f.fields_for(association, new_object, :child_index => "new_#{association}") do |builder|
      render(association.to_s.singularize + "_fields", :form => builder)
      end
      link_to_function("add_fields(this.id, "#{association}", "#{escape_javascript(fields)}")")
      end
      end


      The html output is as follows:

      1. Remove Phone
      2. . .

      Add Phone

  • Owner

    Can you please make gists or pasties of the relevant chunks of code?

    jtosey commented Oct 16, 2010

    Formtastic is given this input and generates the associated output (sorry, I'm having a bit of difficulty figuring out how to embed a gist):

    http://gist.github.com/629434

    Owner

    Can I see the whole

    ...
    output please?

    jtosey commented Oct 16, 2010

    See updated gist.

    jtosey commented Oct 16, 2010

    I've updated the example with just enough additional context that you can copy it directly into here:

    http://validator.w3.org/check

    To see the problem.

    Owner

    Thanks. No dounb it's invalid, just a question how we got here. Need t figure out if this is Rails "helping" again (in which case we need to route around the problem) or if we have a genuine bug in our own code.

    Looking into it!

    Owner

    Can you please update the gist to include the profile_phone_fields partial?

    jtosey commented Oct 17, 2010

    Okay, I've added it. I should have included it up front.

    Owner

    Ok, Rails concats the hidden fields containing the object ids automatically, so this is somewhat outside of our control, but it seems as though there's some hooks and workarounds in place, so I'll keep playing, but having trouble repeating the bug in specs, which is slowing me down.

    Owner

    Ok, can repeat something similar in the specs... Which version of Rails are you using?

    Owner

    I have a work-around for now:

    Rails renders the offending hidden id fields IF a hidden id field for that object hasn't already been rendered. The work around is simple: render our own hidden id field (in which we can control the mark-up) inside the block before Rails has to.

    I forked your gist and added a hidden input to the profile_phone_fields partial. I've dpne some quick testing in Rails 3 only, and it seems to work. Let me know how you go.

    The next challenge is to try to render this hidden id field without you having to include it in your partial. I have no idea how possible that is, so stick to the work-around for now.

    Owner

    Specs for the work-around pushed up in 03ef4a1.

    jtosey commented Oct 17, 2010

    That was fantastic, Justin. I now get "This document was successfully checked as XHTML 1.0 Strict!" from the W3C Markup Validation Service. Thanks for figuring this out so quickly!

    Owner

    @jtosey Ok, I need some more help! There's a patch in the GH-398 branch which might mean you don't have to render the hidden input in the block yourself. Please bundle that branch and test out in your app ASAP!

    Anyone else who'd care to review, please do!

    jtosey commented Oct 18, 2010

    Sorry, I get validation errors. Here's the output, even when I paired back my form to the attached.

    http://gist.github.com/633270

    Owner

    What are the errors? That HTML seems to be unrelated to the previous stuff.

    Owner

    jtosey, can I get an update please?

    Owner

    I've done my own testing and validations. The original bug is fixed and creates a valid fragment of XHTML. The problem is most likely that semantic_fields_for creates LI elements, which you're probably rendering without a containing OL. Insert your own around the semantic_fields_for block and it will all work. Closing.

    This issue was closed.
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment