[beta blocker blocker] Complex widgets are not respecting "#disabled" state

The patch attached in the issue doesn't apply cleanly, I re-rolled it, attaching again.

The patches in #4 and #5 are identical except for the fact that #5 is not properly rolled (should be done from the Drupal root directory). Setting to needs review for #4. lut4rp, you'll need to repost that patch so the testing bot knows what's up.

Again, re-rolling and posting.

That seems sane to me. I wonder if we'll need to make changes to the javascript though.

We recently changed the behavior of #disabled, see #426056: Server-side enforcement of #disabled is inconsistent just wondering if there is anything to do because of that, for example, pass the #disabled attribute through too.

subscribing for review later.

#12: 776956-managed-file-attributes-3.patch queued for re-testing.

#12: 776956-managed-file-attributes-3.patch queued for re-testing.

Re-rolled to keep up with HEAD. I changed

'#attributes' => $element['#attributes']

to

'#disabled' => (isset($element['#disabled']) && $element['#disabled']),

as it feels icky to indiscriminately copy all attributes to the child components of the file element.

+++ modules/file/file.js	4 Jun 2010 08:38:53 -0000
@@ -102,7 +106,8 @@
+      .end().filter('.disabled').removeClass('disabled');

indenting might be needed here...

Powered by Dreditor.

left as needs review because I only looked at the style real quick, nothing functional.

Make Form API inherit #disabled and slap a #disabled on the widget container in field.form.inc. This issue is not about file.

Ah so it's not a setting much rather something like the a form alter that adds a disabled. Then this little patch should suffice.

Added two tests:

- #disabled is recursively validated (values below a #disabled container cannot be altered) [this was working before]
- #disabled is recursively added to the HTML [this is what this patch fixes]

And with a non-broken inheritance logic, it should pass better :)

*blushes* OK. I am dumb. But the logic was sound :)

+++ includes/form.inc
@@ -1432,6 +1432,11 @@ function form_builder($form_id, $element, &$form_state) {
+    // Disable child elements if parent is disabled.
+    if (isset($element['#disabled'])) {
+      $element[$key]['#disabled'] = $element['#disabled'];
+    }
+

s/isset/!empty/

+++ modules/file/file.js
@@ -92,7 +92,11 @@ Drupal.file = Drupal.file || {
-    var $disabledFields = $('div.form-managed-file input.form-file').not($enabledFields);
+    var $disabledFields = $('div.form-managed-file input.form-file').not($enabledFields).each(function() {
+      if ($(this).is(':disabled')) {
+        $(this).addClass('disabled');
+      }
+    });
 
     // Disable upload fields other than the one we're currently working with.
     $disabledFields.attr('disabled', 'disabled');
@@ -102,7 +106,8 @@ Drupal.file = Drupal.file || {

@@ -102,7 +106,8 @@ Drupal.file = Drupal.file || {
     // re-enables the file fields after other processing is complete even though
     // it is only a 1 second timeout.
     setTimeout(function (){
-      $disabledFields.attr('disabled', '');
+      $disabledFields.filter(':not(.disabled)').attr('disabled', '')
+      .end().filter('.disabled').removeClass('disabled');
     }, 1000);

I had to read this 10 times before understanding what was being done. I changed it to what makes more sense to me. Please see if I captured the intent correctly.

60 critical left. Go review some!

whether !empty or isset, does not matter, who cares if we inherit #disabled false? I am fine with either, but it should check for !isset($element[$key]['#disabled']) so that we do not squash an existing one

Are you sure? Don't we want it just like #access? What's the use-case for a disabled parent and a non-disabled child?

I have no idea but if you explicitly set #dsabled to FALSE we better honor that. It's a very edge case.

Ok. In fact, it's what we were already doing for checkboxes and radios, so this cleans that up.

Patch works as advertised with complex widgets. Although I think some more involved with the issue should mark it RTBC.

+++ modules/simpletest/tests/form.test	24 Jun 2010 21:53:26 -0000
@@ -185,26 +185,43 @@ class FormsTestCase extends DrupalWebTes
+   * Assert that the values submitted to a form matches the default values of the elements.

line too long (80 char max)

45 critical left. Go review some!

From http://drupal.org/node/1354#general

Summary here; one sentence on one line (should not, but can exceed 80 chars).

Still applies and works, also the disabled fields can't be gamed from what I can tell.

#32: 776956-disabled-inherit-32.patch queued for re-testing.

According to http://drupal.org/community-initiatives/drupal-core, #690980: Disabled form elements not properly rendered is a beta blocker. This issue is a pre-req for it, making this issue a "beta blocker blocker". Reflecting that in the issue title.

#32: 776956-disabled-inherit-32.patch queued for re-testing.

Sorry; took me awhile to get brain-space to look at this. And... looks good! And comes with tests to make sure it works.

Committed to HEAD. Thanks!

Little patch to disabled password_confirm correctly in D6.

Updating status. Ignore testbot fail.