Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check_box :disabled => true doesn't work as expected in Rails 3.0.7 or 3.0.9 #1953

Closed
HHRy opened this issue Jul 4, 2011 · 30 comments · Fixed by #4337
Closed

check_box :disabled => true doesn't work as expected in Rails 3.0.7 or 3.0.9 #1953

HHRy opened this issue Jul 4, 2011 · 30 comments · Fixed by #4337
Assignees

Comments

@HHRy
Copy link

HHRy commented Jul 4, 2011

In the case of:

    <%=  fields_for some_model[some_new_model], foo do |form| %>
      <%= form.check_box :bar, :disabled => true %>
    <% end %>

Correctly generates a disabled checkbox for this field; but the automatically generated hidden form field is not disabled as one would expect.

@dmathieu
Copy link
Contributor

dmathieu commented Jul 4, 2011

Have you tried if it's not already fixed in 3.0.9 ?

@HHRy
Copy link
Author

HHRy commented Jul 4, 2011

I've just verified that it's present in 3.0.9 as well.

@pixeltrix
Copy link
Contributor

The hidden field value is flipped depending on whether the checkbox is disabled or not. This is the intended behaviour as there's a test for it.

@jaybee
Copy link

jaybee commented Jul 4, 2011

Hi there. I don't think the problem is being understood here. When a field is marked as :disabled, the value for that field should not be submitted to the server when the form is submitted. By leaving the hidden field in a non-disabled state, the value for that field is submitted, which would seem to break this suggestion:

http://www.w3.org/TR/html401/interact/forms.html#h-17.12.1

Could you clarify why this is intended?

@HHRy
Copy link
Author

HHRy commented Jul 4, 2011

+1 on the above, this breaks the expectation that disabling a field won't submit any value for it. Disabled elements should never be successful. If this behaviour is required, perhaps readonly is the correct attribute to use.

@pixeltrix
Copy link
Contributor

@jaybee I understand the problem - the test name explicitly makes it clear. The original commit refers to an original Rails Trac ticket that I can't access so I can't tell exactly what the rationale is, but it's obvious it's intent is to submit the current value when the checkbox is disabled. Until some comes up an explanation for the original commit and why that explanation doesn't apply any longer then nothing should be changed. Even then it shouldn't be changed for 3.0.x or 3.1.x as it's breaking backwards compatibility.

@pixeltrix
Copy link
Contributor

@HHRy the check_box form helper already breaks expectations somewhat by creating a hidden field. You can use the readonly attribute to prevent changes but that doesn't change the appearance of the form element - it may be you want to disable the element so that it can't be changed but still want the current value submitted back.

@jaybee
Copy link

jaybee commented Jul 4, 2011

Thanks for responding. I understand the motivation behind the hidden field, and agree with it. It's an important use case where a checkbox is cleared, and you want the "flipped" value to be submitted, not "no value".

What seems to be lost is an understanding of the difference between readonly and disabled when applied to a field. readonly is a presentation-level attribute, where a field is disabled from user input, but its value is submitted to the server. That seems to be what the original commit really means. disabled absolutely, positively should never be submitted to the server. The original commit comment:

Disabled checkboxes don't submit a form value

is fixing a bug that isn't a bug. Disabled checkboxes should not submit a form value, though readonly ones should. The current behaviour intentionally makes it impossible to mark a checkbox field as truly disabled, and it's impossible to override it using an option.

@pixeltrix
Copy link
Contributor

Okay got the original ticket from archive.org. Looking at the description its intent is to fix the hidden field overriding a disabled checked check box which could've been fixed by applying :disabled to the hidden field so it'll be fine to change on master but I'm against about changing on 3-0-stable and 3-1-stable just in case someone is relying on this behaviour.

@jaybee
Copy link

jaybee commented Jul 4, 2011

Phew, thanks for that :)

Should this issue be re-opened, or do you want me to file a separate, more explicit issue?

@pixeltrix
Copy link
Contributor

I'd like a pull request with tests and a changelog entry. :-)

If you don't feel up to it I'll reopen this ticket.

@jaybee
Copy link

jaybee commented Jul 4, 2011

Hehe: I'm up to it, just not sure when I'll have the time soon ;-)

I'll see how I get on and revisit if it's taking me too long to get to.

@ghost ghost assigned pixeltrix Jul 4, 2011
@pixeltrix pixeltrix reopened this Jul 4, 2011
@pixeltrix
Copy link
Contributor

There's no rush - we still haven't released 3.1, so 3.2 will be a while yet. I've reopened the ticket and assigned it to me so it doesn't get lost in the noise.

@jaybee
Copy link

jaybee commented Jul 4, 2011

All good. I've got a note to pick away at it. Should be an easy enough fix.

@dmathieu
Copy link
Contributor

dmathieu commented Jul 4, 2011

I think this should come with a change in jquery-ujs and prototype-ujs.
When a checkbox is disabled after loading the page, the hidden field should disappear.
Same thing when the checkbox is reenabled.

@jaybee
Copy link

jaybee commented Jul 4, 2011

Not sure if it should "disappear": simply disabling it is good enough (and "correct", IMHO). If we remove it, we then have to put it back, and I don't think JS should be modifying the DOM like that for something that should still work without JS.

@pixeltrix
Copy link
Contributor

@dmathieu are you thinking that if it's re-enabled by javascript and the checkbox is left disabled then unchecking the checkbox won't work.

@dmathieu
Copy link
Contributor

dmathieu commented Jul 4, 2011

I mean that if javascript enables a checkbox which was loaded by rails as disabled (so with a disabled hidden field), the hidden field should be enabled too.
Otherwise, you end up with different behaviors when javascript enables/disables the checkbox.

@jaybee
Copy link

jaybee commented Jul 4, 2011

Oh, yes, definitely this. In the broad strokes, whatever "happens" to
the checkbox should be mirrored on the hidden field.

@JangoSteve
Copy link
Member

If this ends up being something that needs to be addressed in jquery-ujs, please open a ticket (or submit a pull request) over there when the time comes and I can take a look.

@joeellis
Copy link

Was running into this same issue, just wondering if it'll make it in the new 3.2 release

@JangoSteve
Copy link
Member

I'm curious as well. If so, I'll need to update jquery-ujs to remove the disabled property of the hidden attribute when the form is submitted for check boxes that are enabled.

@tadast
Copy link
Contributor

tadast commented Jan 2, 2012

Let me know if there are any outstanding issues with the pull request, I'd be happy to change it if there is a need. @pixeltrix ?

carlosantoniodasilva pushed a commit to carlosantoniodasilva/rails that referenced this issue Jan 27, 2012
@jacob-carlborg
Copy link

I just run into an issue caused by the fix for this issue. It's very annoying that the disabled checkbox isn't sent from the form. Yes, I know the the most correct behavior is to not send disabled controls but in this case it's very inconvenient. Some people here suggested to use readonly, but, surprise surprise, readonly on on a checkbox doesn't work.

@pixeltrix
Copy link
Contributor

@jacob-carlborg care to describe what the issue is?

@jacob-carlborg
Copy link

The value of the checkbox is not sent to Rails if it's disabled. I think it's inconvenient.

@pixeltrix
Copy link
Contributor

@jacob-carlborg that's the point of disabled - I mean how did it affect your application. For example was a column left with a NULL value?

@jacob-carlborg
Copy link

No, we had a check to see if the checkbox was enabled. If it was, we associated another model with the model being updated. It failed to run some code that it should run.

You can give the same argument for checkboxes that are not disabled but unchecked.

"When a form is submitted, only "on" checkbox controls can become successful."

http://www.w3.org/TR/html4/interact/forms.html#checkbox

But we have hidden fields as a workaround for that, because it's more convenient.

@pixeltrix
Copy link
Contributor

I'm unable to see how this change will have affected your application. The hidden field is only present when using form_for and check_box - the check_box_tag helper doesn't add it. If the input isn't submitted then it will retain the existing value surely? Unless you're checking the attributes hash directly rather than the model.

Either way, the new behaviour is correct in my opinion and I don't see any way of easing the pain.

@jacob-carlborg
Copy link

Yes, I did check the attributes hash directly.

al2o3cr pushed a commit to al2o3cr/phoenix_html that referenced this issue Feb 11, 2020
When a checkbox is disabled, it doesn't send its value when the form is
submitted - but the hidden input DOES. The result is that a disabled
checkbox will always submit as the unchecked value (`"false"` by
default).

See also rails/rails#1953 where a similar
helper function had the same bug with the same fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants