Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

What should happen in the case that I add #disabled = TRUE to my form element, but then have a JavaScript that will un-disable the field on a certain condition. How will FAPI know when it can or cannot accept the input value?

boombatower’s picture

Would seem if JavaScript is enabled then you should disable/enable the field on the client side if it is dynamic, otherwise if #disabled is set it is final.

I feel this is the same as the Form API testing for invalid select options.

David_Rothstein’s picture

Subscribing. This may end up being a blocker for #91663: Permission of text format is not checked when editing an entity and instead reset to something a user can use....

I agree with @boombatower that JavaScript isn't a problem. If JavaScript wants to "pseudo-disable" a field that isn't actually disabled, it can do so on $(document).ready() and then enable/disable it at will from that point forward. But if a field is defined with #disabled in Form API, then it seems like the server should enforce that that's actually the case.

sun’s picture

Title: Form API: Ignore field value for #disabled fields » No form validation error for manipulated values of #disabled fields
Category: feature » bug
Status: Active » Needs review
FileSize
4.84 KB

Correct, and this is a bug.

Attached patch implements a test that proves this. On that note, #required, but #disabled textareas seem to be handled properly already (i.e. the last 2 assertions pass).

Now it's up to FAPI masters to make those tests pass. :)

Status: Needs review » Needs work

The last submitted patch failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
5.47 KB

Hm..

I'm not a FAPI master, but I want to fix that edit bug :)

I am not sure about the validation stuff, do we really want to display a form error if the value has been changed or is it enough if we silently enforce the #default_value?

Because doing the latter is a simple, 1-line change, adding error messages does make it quite a bit more complicated... I don't yet fully understand how that value gets set and where it does come from.

Attached patch just enforces the default value, let's see if will break something else... Atleast the form validiation assertions will still fail, obviously..

Status: Needs review » Needs work

The last submitted patch failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
5.72 KB

Ok, here is a new try. This time, with validation errors. I updated in _submitForm() to use the new form_clear_errors() function to clear the form error function cache.

All tests should now pass...

sun’s picture

+++ includes/form.inc	12 Aug 2009 13:22:00 -0000
@@ -1122,6 +1122,12 @@ function _form_builder_handle_input_elem
+      // If a #value different than #default_value has been submitted and the
+      // element is disabled, add a form error.

Let's move the primary condition to the start... :)

"Issue a validation error, when an element is disabled and its #value is different than its #default_value."

+++ includes/form.inc	12 Aug 2009 13:22:00 -0000
@@ -1122,6 +1122,12 @@ function _form_builder_handle_input_elem
+      form_set_error($element['#name'], t('Disabled elements can not be changed'));

At least there should be a trailing period (full-stop) in the error message. However, I'm also not sure whether that message is not a bit too technical...?

Beer-o-mania starts in 18 days! Don't drink and patch.

Berdir’s picture

FileSize
6.11 KB

At least there should be a trailing period (full-stop) in the error message. However, I'm also not sure whether that message is not a bit too technical...?

User Joe Common will most probably never see this message. I can see two reasons why that message would be displayed

a) Someone tries to do something "bad", like removing the required attribute with FireBug or a script and tries to send a value with it. We don't need a valiation message at all, here..

b) Someone develops a module and uses #disabled but does then add some javascript to un-disable it. Here would a technical validation error be really useful.

I also talked with chx about this and he was against a validation message (and not really for this patch at all, either ;))

(sorry for quoting without asking, you are sleeping :))

(16:40:05) chx: Berdir2: I dont think this patch makes any sense
(16:40:27) chx: Berdir2: and prolly checkboxes need to unfixed too
(16:40:40) chx: Berdir2: #disabled is not #access nor is #value - set
(16:40:57) chx: Berdir2: it's a UI element which can be removed by JS and changed and that change is valid
(16:41:23) chx: Berdir2: if anything at all, ignore the change.
(16:43:25) chx: Berdir2: but this is something we need to discuss with morepeople alas because it's an expectation issue.

We also need to update the documentation and write what #disabled means. Because http://api.drupal.org/api/file/developer/topics/forms_api_reference.html... says nothing about protection, only that the element is greyed out. However, if you just want that, you can also use $form['element']['#attributes']['disabled'].

The updated patch fixes the string/documentation issues and also removes the special handling from checkbox. That we are doing it for those imho really means that we should do it for all form elements. The only place in core where we used that "un-disabling" were the teaser fields AFAIK.

Status: Needs review » Needs work

The last submitted patch failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
8.1 KB

There were some tests that called the checkbox_value() function directly. removed them and added one more test for checkbox fields...

boombatower’s picture

@chx in #10: In the same way that you get a validation error if you tamper with a select so should you if you tamper with a disabled field. Seems simple and clean, and if a developer/user accidentally does it at least they get something instead of just wondering why the value keeps saving as blank.

boombatower’s picture

Still needs additional reviews, most of this is minor.

+++ modules/simpletest/tests/form.test	13 Aug 2009 11:53:29 -0000
@@ -72,42 +76,86 @@ class FormsTestCase extends DrupalWebTes
+    foreach ($properties as $extras) {

Would seem more standard to use $extras => $extra.

+++ modules/simpletest/tests/form.test	13 Aug 2009 11:53:29 -0000
@@ -72,42 +76,86 @@ class FormsTestCase extends DrupalWebTes
+        '#default_value' => $this->randomString(),

Why assign #default_value here if always overridden by $extras.

+++ modules/simpletest/tests/form.test	13 Aug 2009 11:53:29 -0000
@@ -72,42 +76,86 @@ class FormsTestCase extends DrupalWebTes
+      $this->assertTrue(isset($errors['textarea']), 'Form validation failed for manipulated, disabled form element.');

Pre our conversion away from t(), but I'm not apposed to leaving it off assertion messages since we have already agreed upon it.

+++ modules/simpletest/tests/form.test	13 Aug 2009 11:53:29 -0000
@@ -72,42 +76,86 @@ class FormsTestCase extends DrupalWebTes
+  private function _submitForm($form, $edit) {

Since we are using classes we don't need "_", private property is enough.

18 days to code freeze. Better review yourself.

Berdir’s picture

FileSize
8.04 KB

Re-rolled with the things mentioned in #14 fixed.

boombatower’s picture

FileSize
7.73 KB

Tested manually by editing a few forms and read through the code...looks good. Only comment I have is included below. I tweaked in patch.

+++ modules/simpletest/tests/form.test	14 Aug 2009 08:27:23 -0000
@@ -72,42 +76,85 @@ class FormsTestCase extends DrupalWebTes
+    foreach ($extras as $extras) {

Seems like bad form to have same variable name...although it appears to work, not my first guess.

I'm on crack. Are you, too?

boombatower’s picture

Status: Needs review » Reviewed & tested by the community

Meant to mark it.

Dries’s picture

So, is chx still against this or? It is not clear whether we reached consensus.

chx’s picture

Status: Reviewed & tested by the community » Needs work

I _think_ disabled is a display gimmick and we actually should remove the checkboxes special handling of it. One can set #value on an element to prevent tampering, or use #access FALSE... #disabled is neither of them. Actually, there are other ways of making a form element usually unchangeable in a browser, readonly or you can hide it with visibility: hidden or display:none. These are visibility gimmicks and we should not cover them. I think.

boombatower’s picture

After discussing it with chx in person I agree that #access and other methods make sense to use for form elements that you don't want changed...and that #disabled is purely for cosmetics.

chx’s picture

Usually I post more to clarify my position further. Not in this case. There is no clear cut "this is correct" solution in this case. The option checker throws a validation error which is cryptic and largely useless to the user however there is something at least. We can do the validation message here as proposed if people think that's better. We can throw away simply what the user entered. We can disregard #disabled. Hpwever, as Berdir points out, attributes['#disabled'] can be used for the meaningless UI role so the last option is probably not necessary. I let the community decide what do they want. I do not have strong feelings , either of the three is fine.

Berdir’s picture

Status: Needs work » Needs review
FileSize
8.65 KB

I do not care if we have a validation message or not, so I left it in.

In the patch, I changed a few things to have the #disabled handling after #process functions, so that it can be used for #91663: Permission of text format is not checked when editing an entity and instead reset to something a user can use. too. If this does not work, then this patch isn't that important anyway... ;) Not sure if the tests will pass, I have some weird issues locally.

Status: Needs review » Needs work

The last submitted patch failed testing.

effulgentsia’s picture

This issue has resurfaced due to a bug initially reported in field_ui.module: #687572: When disabled textfields are submitted, their values are forced to empty string instead of the default value. We should probably mark either this one or that one a dupe. That one has an up to date patch. Would folks like to continue the discussion there, or should I close the other one, and move relevant comments and patch to here?

effulgentsia’s picture

Status: Needs work » Needs review
FileSize
2.7 KB

Never mind about #24. I updated the other issue with a patch that's specific to that issue. Here's an updated patch for this issue (without tests and without validation message). I agree with earlier comments that #disabled should be a FAPI concept (meaning this element really is disabled and don't let client-side code get around that) and that #attributes['disabled']='disabled' should be used by developers who want the element to start off greyed out in the browser, but that shouldn't be considered disabled by FAPI.

1. Is there consensus that this is indeed a bug that needs fixing?
2. What's the consensus on whether to re-roll the patch to include a validation error message instead of simply disregarding the input?

effulgentsia’s picture

Related issue: #500868: Forms API is missing the readonly form field option. That one introduces an interesting problem: if HTML supports two different attributes that implement different UI for the same functionality desired server-side, what should we do?

effulgentsia’s picture

Here's a thought on how to support the HTML distinction of 'readonly' vs. 'disabled' while still maintaining a single property #disabled that is used during validation (so that hook_form_alter()'s have one property rather than two to control whether input is accepted).

effulgentsia’s picture

Changed proposed new property name to #allow_focus instead of #disabled_allow_focus.

Eric_A’s picture

+    // For an unchecked checkbox, we return numeric 0, so we can explicitly
+    // test for a value different than string '0'.

How about integer 0? My point being that string '0' is very numeric to is_numeric()...

EDIT: my bad, this was in the original code already...

effulgentsia’s picture

How about integer 0? My point being that string '0' is very numeric to is_numeric()... my bad, this was in the original code already

It's a good point though, so this patch fixes it.

I hope this issue gets resolved before D7 release, but if not, I just realized that it is possible to work around it by having forms set #value on elements that they don't want user input overriding, and potentially this should be the preferred way of identifying an element for which input isn't wanted, leaving #disabled as only a UI default rather than having any server-side meaning. But if that's our decision, we should document it, including updating http://api.drupal.org/api/drupal/developer--topics--forms_api_reference.... to make it clear that all elements support a #value property.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

I'm fine with either the latest patch or "#disabled as only a UI default rather than having any server-side meaning". since we have a patch for one approach, lets go with that.

effulgentsia’s picture

And just to clarify/summarize, as alluded to in #10 and #19, HEAD currently has an inconsistency in that for checkboxes and checkboxes only, #disabled means server-enforced disabled, while for all other controls, #disabled is a UI default only. #30 makes all controls treat #disabled the same way that HEAD currently does for checkboxes. Other options have been discussed in earlier comments, and in #21, chx indicated neutrality on which option is chosen as long as it is consistent. There seems to be more desire from the people who participated in this issue to make #disabled mean server-enforced disabled (making everything more like checkboxes rather than making checkboxes more like everything else). Earlier patches implemented validation errors (which would be new functionality in that HEAD does not set validation errors if input is submitted for a disabled checkbox, the input is simply silently ignored). I'm not clear whether consensus was reached on that, so #30 doesn't implement validation errors, it simply makes all controls treat #disabled the same way that checkboxes currently do. I'm certainly open to merging the code from earlier patches that implement validation errors if there's consensus that it's desired and that doing so is in-scope for D7.

webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Thanks for the great summary! I agree that a logical assumption for a developer is that a FAPI property #disabled would have some 'magic' behind it to auto-validate its value, especially since checkboxes do this.

However, as with anytime we go monkeying around in the innards of FAPI, we need to have tests for this. It looks like there were some earlier that sun posted; can we just integrate them into the new patch?

sun’s picture

Awesome job, effulgentsia!

Reading both patches/approaches, I really prefer #30. Recalling when I got bit by this issue myself, I was quite confused that #disabled doesn't really mean "no input allowed here". I expected that the form element input value would be ignored, no matter what. So effulgentsia's patch brings Form API to (at least my) expectations.

And if you want a semi-#disabled element that can still be enabled, you can use #attributes. But then again, it's also highly debatable and highly depends on the actual form/workflow, whether JavaScript shouldn't set the initial 'disabled' attribute instead. In that manner, a disabled form element is indeed pure cosmetics without any required server-side logic, so the developer has to decide on a case-by-case basis.

My original tests can be found in #4. However, I'm not sure whether they are still valid, might require some tweaking.

effulgentsia’s picture

Status: Needs work » Needs review
FileSize
11.52 KB

@sun: are you okay with this as the test instead of what you had? This test causes several failures in HEAD (as expected) and succeeds with the patch. And just for additional confirmation that test-driven development is good, it identified 2 more hunks needed in form.inc to deal with checkboxes and radios.

effulgentsia’s picture

But then again, it's also highly debatable and highly depends on the actual form/workflow, whether JavaScript shouldn't set the initial 'disabled' attribute instead.

So true, and we shouldn't be giving tacit approval for developers to write inaccessible forms, so this patch expands the comment to mention this.

Eric_A’s picture

Hmm, It looks like we encounter problematic behaviour when disabling a checkbox with an undefined #default_value? Seems the patch replaces old problematic behaviour with new problematic behaviour.

The submit handler expects integer 0 or the #return_value string, but:

Before patch:
Notice: Undefined index: #default_value in form_type_checkbox_value() ... (See #642820: PHP notice when submitting form with disabled checkbox)
($form_state child element populated with the NULL value, I guess. EDIT: nope, it's ''. First NULL is passed to form_type_checkbox_value(), then FALSE. Nothing is returned and since there is no default value we end up with ''.)

After patch:
$form_state child element populated with the empty string. Auch.

I'm not 100% sure at this moment so I'm leaving status as is for now.
Also, I have no idea if single checkboxes without #default_value exist in core. If not, this behaviour will not surface with core, but if so, then any contrib module that disables one will run into a WTF with core.

Eric_A’s picture

Auch, seems I saw things simply wrong. NULL values are set as input so we get either 0 or #return_value...

EDIT: Well, now I'm thinking I was right after all. Seems the above mentioned logic is skipped when an element is disabled and we're simply passing FALSE as input to the value callback. Then nothing is returned and $form_state is populated with either #default_value (when set) or ''.
I cannot do any testing now, just reading code...

Eric_A’s picture

Better description of #37 and #38:
This patch reveals bad behaviour of an unchecked checkbox when there is no default value and #disabled == TRUE.
(#access == FALSE probably triggers the exact same logic as #disabled == TRUE, so I'm thinking the problem exists there as well).

This patch makes visible that form_type_checkbox_value() does not return the same value for these 2 scenario's with an unchecked checkbox and no default value:
1) unchecked, enabled and accessible; $input === NULL; should return 0 and it does.
2) unchecked, disabled; $input === FALSE; should return 0 but it returns nothing; In the end #value will be set to ''. This is very problematic when you happen to use '' as your checkbox value.

effulgentsia’s picture

Status: Needs review » Postponed

Yeah, good catch, separate but related issue. Therefore, let's resolve #642820: PHP notice when submitting form with disabled checkbox first, then come back to this one.

effulgentsia’s picture

thekevinday’s picture

Okay, so here is my argument on this matter.

I believe that (for numerous reasons, such as accessibility support) the use of disabled and readonly should be treated exactly as is in accordance to the web standards.
A person should not have to expect custom or non-standard stuff to happen when they use standard DOM.
The end result of adding custom functionality to standards is proven by Internet Explorer.

That said, there is an obvious need (if not requirement) for programmatic interpretation of data.
Drupal is a CMS, obviously.

Instead of breaking web standards (per say), why not add another option that can provide the needs as defined in this issue while still supporting web standards exactly as they are?

Lets say we have a checkbox called "Ignore Field Input If.."
Which is followed by a multi-select drop-down with:
- ReadOnly
- Disabled

When the check box is selected, Disabled is selected in the list box, and the Disabled checkbox is checked, then:
- When a user saves a form, all disabled fields will not trigger any validation and the disabled data will be dropped in the event that something on the user end injects something into that field.

The same would be true for readonly.

If the user wants to have the data in the Disabled or ReadOnly fields be sent to the server, then they MUST be validated for security purposes (ie: js injection attacks).
Therefore when the "Ignore Field Input If.." check box is unselected, validation will still happen and the value of the field will be passed to the server.

The use case for having a disabled field in this way is the use of Conditional Fields.

Lets say I have a case where I have a Conditional Field that is dependent on having a Controlling Field be set to a certain value.
If that field is set to the appropriate value, then the Conditional Field can be edited by the user.
If the Controlling Field is set to something else, I might force a default and disable the users ability to change that value.
I still need to interpret the value and pass it to the server.
I may or may not want the user to be able to copy and paste data from within the disabled field if they are allowed to see that field.

This example can become even more complex if the situation becomes dependent on multiple fields and each of those multiple fields applies their own twists.

Forms is a drupal core project and not an extra module, so it must be taken into consideration that modules will override things and add extra complexity (as in the provided use case).
One must also consider that drupal is a web-based application and therefore one would expect its users to expect drupal to follow and provide something that follows web standards (without gotchas).

What I am trying to get at is:
Don't force added interpretation that is outside of a standard, but feel free to provide the end-user to select an option to do so.
Please add an extra option similar to (if not the same as) the "Ignore field.." concept I presented here instead of forcing all Disabled data to be ignored.

And just to be thorough, here is what my favorite source claims how Disabled and ReadOnly fields should function:
(http://w3schools.com/tags/att_textarea_disabled.asp) (http://w3schools.com/tags/att_textarea_readonly.asp)

disabled

The disabled attribute specifies that a text area should be disabled. A disabled text area is unusable and un-clickable.

The disabled attribute can be set to keep a user from using a text area until some other condition has been met (like selecting a checkbox, etc.). Then, a JavaScript is required to remove the disabled value, and make the text area usable.

readonly

The readonly attribute specifies that a text area should be read-only.

In a read-only text area, the content cannot be changed, but a user can tab to it, or highlight and copy the content from it.

The readonly attribute can be set to keep a user from using a text area until some other condition has been met (like selecting a checkbox, etc.). Then, a JavaScript is required to remove the readonly value, and make the text area editable.

effulgentsia’s picture

@thekevinday: sorry, I'm not quite following. Can you summarize a little more succinctly what you're requesting?

It's already the case that every form element has a #attributes, and that's where you can put stuff to affect HTML attributes. So, if you want to apply the "disabled" or "readonly" HTML attributes to an element, without that affecting server-side logic, you can do so. If the conditional field module wants to start an element off as disabled to make the UI better, but not really have it be disabled as far as server-side processing is concerned, it can do so by setting $element['#attributes']['disabled']. Though if it's interested in making the UI work well for users with JavaScript disabled, it may prefer to not do that, and instead, to use JavaScript to disable the necessary elements on document ready.

The #disabled property is not an HTML attribute, it is a Form API property. It means to disable the element as far as the Form API is concerned, which means: server-side code does not want user input for this element, but since PHP can't prevent a rogue user agent from submitting the input, the Form API needs to take steps to ensure that the rogue input doesn't get used where it shouldn't be. This isn't violating HTML conventions, it is choosing to use the word "disabled" to mean something appropriate for what the Form API is. This is similar to the #type property, which has meaning in Drupal rendering that is different from the meaning of the HTML 'type' attribute. The English language is limited, and sometimes the precise meaning of a word depends on its context and Form API is a different context than HTML.

By the way, this issue is still postponed due to #335035: drupalPost() incorrectly submits input for disabled elements being needed before the correct tests can be added to this patch.

thekevinday’s picture

Sorry, I sort of got redirected here and was thinking of this in the same context as the other post I was on. (for reference, see: http://drupal.org/node/500868)

EDIT:
After spending a few days width watchdog(), hook_form_alter(), and callback_after_build(), I managed to make since of how disabled and readonly are actually being handled.

If I edit the $form[$key][0]['value']['#attributes']['disabled'] which ultimately boils down to the form API's $form['#attributes']['disabled'], will the '#disabled' discussed in this thread be affected by the 'disabled' inside of '#attributes' (and vice-versa)?

Or that was what I was asking, so your first statement answered this.
Thanks for clearing this up!

effulgentsia’s picture

Assigned: Unassigned » effulgentsia
Status: Postponed » Needs work

@thekevinday: cool. I'm glad we're on the same page!

#335035: drupalPost() incorrectly submits input for disabled elements landed, so setting back to "needs work". I'm gonna try to roll an updated patch with proper tests soon.

effulgentsia’s picture

Assigned: effulgentsia » Unassigned
Status: Needs work » Needs review
FileSize
13.61 KB

Re-rolled and with test. Moshe or sun, please RTBC or suggest changes. Thanks!

effulgentsia’s picture

Title: No form validation error for manipulated values of #disabled fields » Server-side enforcement of #disabled is inconsistent

Better title, see #32. This patch does not consider it a validation error if input is submitted for a disabled element. It just makes FAPI not process that input into #value and $form_state['values']. This is already what HEAD does for checkboxes. This patch applies that logic for all elements.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

The inline docs here approach the level of poetry. Very nicely done. Thanks.

effulgentsia’s picture

Priority: Normal » Critical

Bugs related to inconsistencies that touch on FAPI security seems to qualify as "critical" to me.

sun’s picture

Critical bump.

tstoeckler’s picture

+++ modules/simpletest/tests/form.test	19 Mar 2010 16:22:25 -0000
@@ -156,23 +156,54 @@ class FormsTestCase extends DrupalWebTes
+  ¶

Extra whitespace.

No other changes. Leaving at RTBC.

Powered by Dreditor.

fleetcommand’s picture

(subscribe)

Eric_A’s picture

Issue tags: -Needs tests

Removed "Needs tests" tag...

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks all. :)

EugenMayer’s picture

Any reason why this is D7 only?

effulgentsia’s picture

@#55: It would be a backwards compatibility break for D6. Breaking BC of an already released major Drupal version should only be done if necessary to fix a real security hole or similarly important bug. While ensuring consistency of the server-side meaning of #disabled across element types is good for DX and makes it less likely that module authors will accidentally expose security holes in their forms, it's not clear to me that D6 functionality is actually insecure (there may be modules that are, if they're incorrectly assuming that FAPI is enforcing #disabled during input processing, but that can be fixed by the module to not assume that). If someone finds a real security hole in Drupal 6, and porting this fix to D6 is the best way to solve it, then this issue's version should be changed to "6.x-dev" and the status should be changed to "patch (to be ported)".

EugenMayer’s picture

Fine

effulgentsia’s picture

By the way, in case you didn't know, in forms where you want to control what gets into $form_state['values'] regardless of what gets submitted in $_POST, you can set #value. So anywhere in D6 where you want #disabled to have the meaning it now has in D7, you can do:

$element['#disabled'] = TRUE;
$element['#value'] = $element['#default_value'];

Not as elegant or intuitive as #disabled working as one would expect, but a way for D6 modules to ensure security where it's needed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

arpeggio’s picture

Status: Closed (fixed) » Active

Re-openning... Same issue mentioned here 803212: Protection against forgery of input selection value doesn't work with checkboxes post #12 are the steps to reproduce the issue in D7.

effulgentsia’s picture

ledut’s picture

subscribing...

Blooniverse’s picture

Status: Closed (fixed) » Needs work

Verrry sorrry for reopening! Reason: @see #1837306: Textfield input attrib #disabled doesn't work! !

Blooniverse’s picture

... at first glance, I would say there is something wrong with if (!empty($element['#allow_focus'])) { in the patch of comment#51.

Actually, this whole paragraph below looks odd to me! Why?: The two HTML element attributes are in reality not linked together (remark: this is a practical/experiential point of view; I didn't take the time to check the browser implementation/specs)! In current Drupal code (v7.17) they seem to be linked, though! Really, 'disabled' is 'disabled' & 'readonly' is 'readonly'. You might find the following summary about the meaning of both elements concisely useful: http://htmlcodetutorial.com/....

Original patch paragraph (diff merged)

if (!empty($element['#disabled'])) {
  if (!empty($element['#allow_focus'])) {
    $element['#attributes']['readonly'] = 'readonly';
  }
   else {
     $element['#attributes']['disabled'] = 'disabled';
   }
}

Well, I have to admit to not having had enough time to check the patch properly. It's just that in my imagination the code should look really simple, almost like:

New paragraph

if (!empty($element['#readonly'])) {
  $element['#attributes']['readonly'] = 'readonly';
}
if (!empty($element['#disabled'])) {
  $element['#attributes']['disabled'] = 'disabled';
}

((Btw, if I disable a textfield, it gets submitted nonetheless!))

tim.plunkett’s picture

Status: Needs work » Closed (fixed)

Please open a new issue, or continue work in the issue you linked.