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.

select tags do not check the :selected option properly

#5056

Hi,

When using the helpers which generate select tags and their options, I've run into a relatively universal problem.

<%= f.select :plan_id, ... %>
# and
<%= f.select :plan_id, ..., :selected => params[:subscription].try(:[], :plan_id) %>

Will both generate a nice big list of plans, but wont select one after being submitted (if errors appear or the form generates a preview first).

The simple fix is the following:

<%= f.select :plan_id, ..., :selected => params[:subscription].try(:[], :plan_id).to_i %>

Yup, that simple. The type that gets read from the record is Fixnum while the type read from the post is String, and of course no string equals a fixnum (by default). A little type coercion could go a ways to fixing this.

Reported by rk · July 6th, 2010 @ 11:53 PM

State: resolved
Milestone: 3.x
Assigned to: José Valim José Valim
Importance: Low

Activity

  1. Subba
    Subba
    • Assigned user set to Neeraj Singh

    July 16th, 2010 @ 09:15 PM

  2. Subba
    Subba

    adding patch to fix above problem this will coerce the selected , options values to string before comparison.
    above problem also happens if the selected is float

    July 20th, 2010 @ 04:31 AM

  3. Neeraj Singh
    Neeraj Singh
    • Milestone set to 3.x
    • State changed from new to open
    • Tag changed from formbuilder, forms, form_tag_helper to rails 3, formbuilder, forms, form_tag_helper
    • Importance changed from to Low

    Subba, patch looks good.

    Can you write test such that there is no need to escape double quotes. It puts too much of strain. Replace

    "<option selected=\"selected\" value=\"1\">first</option>\n<option value=\"2\">second</option>"

    with

    %(<option selected="selected" value="1">first</option>\n<option value="2">second</option>)

    July 20th, 2010 @ 06:01 PM

  4. Subba
    Subba

    i am updating tests with above suggestion

    July 21st, 2010 @ 03:47 AM

  5. Neeraj Singh
    Neeraj Singh
    • Assigned user changed from Neeraj Singh to José Valim

    +1 looks good to me

    July 22nd, 2010 @ 07:25 PM

  6. José Valim
    José Valim

    Yo, thanks for the patch! I don't think there is a need to use try. This is in fact causing a bug today, imagine we have [nil] in selected, it will convert nil to string in the selected array, but not the value, because of try.

    Also, I think we can convert these values to_s somewhere earlier since they will be converted anyway when we create (so we don't convert them twice). But this is rather secondary.

    July 26th, 2010 @ 04:13 PM

  7. José Valim
    José Valim

    Hey guys, any news?

    August 2nd, 2010 @ 04:05 PM

  8. Subba
    Subba

    i am updating the patch with jose suggestions.

    August 13th, 2010 @ 09:35 PM

  9. Repository
    Repository
    • State changed from open to resolved

    August 14th, 2010 @ 12:23 PM

  10. Subba
    Subba
    • Assigned user changed from José Valim to Santiago Pastorino

    can you reopen the ticket. commit mention is #5317 missing model require statement in json serialization tests ticket. i used wrong ticket number to create patch. this ticket still in review by jose.

    August 14th, 2010 @ 02:10 PM

  11. Neeraj Singh
    Neeraj Singh
    • State changed from resolved to open

    August 14th, 2010 @ 02:33 PM

  12. Subba
    Subba
    • Assigned user changed from Santiago Pastorino to José Valim

    above patch will no longer with recent changes. i am creating new patch this has less invasive changes than the one above.

    August 14th, 2010 @ 03:59 PM

  13. Repository
    Repository
    • State changed from open to resolved

    (from [54477c98482f37d547fa158155ae1624b1f7f7ac]) select tags coerce the :selected option, options to strings before comparison [#5056 select tags do not check the :selected option properly state:resolved]

    Signed-off-by: José Valim jose.valim@gmail.com
    http://github.com/rails/rails/commit/54477c98482f37d547fa158155ae16...

    August 15th, 2010 @ 08:50 PM

  14. Repository
    Repository

    (from [dbe5ae488eebee660cd97e59a2e15c4752fbe15b]) select tags coerce the :selected option, options to strings before comparison [#5056 select tags do not check the :selected option properly state:resolved]

    Signed-off-by: José Valim jose.valim@gmail.com
    http://github.com/rails/rails/commit/dbe5ae488eebee660cd97e59a2e15c...

    August 15th, 2010 @ 08:55 PM