When disabled textfields are submitted, their values are forced to empty string instead of the default value

Indeed. After a quick look, seems to be some FAPI-related mess up.

Er, critical.

That affects text fields.

That's because the 'Maximum length' input at the bottom of the form is '#disabled' (which, BTW, Seven doesn't really hint : #690980: Disabled form elements not properly rendered) - see text_field_settings_form().

When the form is submitted, $form_state['values'] contains an empty string for the element, instead of the '#default_value'.
That empty string is taken as the new setting for the 'Maximum length', and since you're not supposed to be able to change this at this point (there's already data in the field, Field API doesn't support changing the db schema), this raises an exception.

I don't know when this broke. Definitely worked at some point earlier.
This shows we lack tests on Field UI, but I'm a bit surprised that a #disabled element is handed back by FAPI as having actually changed ;-).

Need feedback from FAPI gurus here.

When a textfield element is #disabled, _form_builder_handle_input_element() does:

$input_exists = FALSE;
$input = NULL;

      if (!$input_exists && !$form_state['rebuild'] && !$form_state['programmed']) {
        // We leverage the internal logic of form_set_value() to change the
        // input values by passing $form_state['input'] instead of the usual
        // $form_state['values']. In effect, this adds the necessary parent keys
        // to $form_state['input'] and sets the element's input value to NULL.
        _form_set_value($form_state['input'], $element, $element['#parents'], NULL);
        $input_exists = TRUE;
      }

comes back with NULL in $form_state['values'], then

        if (function_exists($value_callback)) {
          $element['#value'] = $value_callback($element, $input, $form_state);
        }

runs form_type_textfield_value(), which puts back '' (empty string) in $element['#value'].

Last refactor in this area was #622922: User input keeping during multistep forms, crossposting over there.

@yched: can you confirm that this fixes the original issue reported? If it does, since this touches on security, a FAPI maintainer should review this prior to it being RTBC.

@effulgentsia: Yup, I confirm it fixes it.

I've also tested this and confirm that the patch on #4 solves the issue.

Now we just need chx or another FAPI maintainer to approve the change (or identify why it's completely wrong).

This one adds one more hunk for the form_type_textfield_value() that's commented with why. If FAPI maintainers disagree with the change introduced by #4, then just this patch's form_type_textfield_value() hunk alone can be committed (with a modified comment) and it will fix the specific use-case this issue was started with, but I continue to think this full patch is in order. It makes no sense to me that there is no way currently to set something as not being available to user input, but still visible. There was a recent comment posted to the D6 FAPI reference to this effect as well: http://api.drupal.org/api/drupal/developer--topics--forms_api_reference..... It makes a lot more sense to me to treat #disabled as meaning not just default the browser to having it disabled, but make the server-side code treat it as disabled too. If someone doesn't want an element disabled in this sense, they can set #attributes['disabled']='disabled' instead of setting #disabled, and this would mean: only disable it client-side, and if client-side code enables the element, then user input can be accepted by the server.

Changed so that even programmatically submitted forms can't bypass #disabled restrictions (note, this is how checkboxes work in HEAD, so this patch just applies the same rule to all elements), and removed the @todo from #9. I'm quite confident this is now correct and ready to fly, but it needs someone familiar enough with FAPI and security to RTBC.

If someone doesn't want an element disabled in this sense, they can set #attributes['disabled']='disabled' ...

would be nice to have it documented

Yep, I think the most logical place, if this patch goes in, would be in http://api.drupal.org/api/drupal/developer--topics--forms_api_reference..... For better or worse, that seems to be where Form API properties are explained. Since this is also an API change from D6 (in terms of making the #disabled property mean more than it meant in D6), this will also need to be mentioned in http://drupal.org/update/modules/6/7 (again, assuming the patch gets committed).

This seems related to (or maybe a duplicate of) #426056: Server-side enforcement of #disabled is inconsistent?

Ok, since there's the other issue, this one is just a patch to fix the original issue reported. This one's a no-brainer, but still needs someone to RTBC (though doesn't have to be a FAPI guru).

Better title given that this is a regression that occurred due to #622922: User input keeping during multistep forms, and all this issue does is fix that regression.

Is this really a regression due to #622922: User input keeping during multistep forms? Or let me put it this way: I got bitten by this in D6. In simple forms I would set the #disabled property on textfields with default values and was left with empty strings after submission. At the time I decided this was just the way FAPI worked and that I either had to set the values myself in a submit handler or set #access FALSE and display a disabled copy of the original textfield.

@Eric_A: You're correct. It's not a regression. I looked at the D6 code and at the D7 code prior to #622922: User input keeping during multistep forms, and both have the same bug. I assumed it was a regression because of comment #2, but I guess there's some other reason FieldUI worked before and now doesn't.

I still think it's a critical bug, and the current behavior makes no sense, because, for example, #type='select' and #type='textarea' do not have this bug. It should at least be fixed for D7, and #14 can be easily rolled for D6 too if that's desired.

The best way to hack around this limitation of textfields until this issue is fixed is to set #value on the element which is the way to force a value on any element regardless of user input. In fact, until #426056: Server-side enforcement of #disabled is inconsistent is fixed, I highly recommend doing this on all #disabled elements that the form doesn't want input for, since currently, nothing prevents javascript from enabling a disabled element.

Looks proper to me. I've been bitten by this too.

Aw, sorry for wrongly assigning blame, effulgentsia.

Confirming that #14 fixes the bug.

Let's get some tests for this.

subscribing

I'd like #426056: Server-side enforcement of #disabled is inconsistent to go in first. There's a dependency between that issue and proper testing of this one. If that issue weren't on the table, this one could proceed, but since that one has reached consensus, and is just awaiting bot to return and someone to RTBC it, it makes sense to get that one in first.

@Shai: once that issue goes in, I'll write the test for this one. It'll be pretty simple: it can leverage existing FAPI tests.

.

This got fixed as part of #335035: drupalPost() incorrectly submits input for disabled elements. The test for this already existed as part of FormsTestCase::testDisabledElements(), but the test wasn't working correctly due to that bug.

subscribing