FAPI: Document that the '#disabled' property in the FAPI is not a 1-1 equivalent of the respective HTML property

[klonos]

This is related to #69, backdrop/backdrop-issues#4083 and backdrop/backdrop-issues#2615 (comment)

I've been working with the FAPI for years now, yet this has just been a revelation to me:

• Both Backdrop FAPI as well as Drupal FAPI provide the '#disabled' attribute
• Neither Backdrop FAPI nor Drupal FAPI provide a respective '#readonly' attribute - and that's by design.
• Both Backdrop FAPI and Drupal FAPI allow for ['#attributes']['disabled'] = 'disabled'.
• Both Backdrop FAPI and Drupal FAPI allow for ['#attributes']['readonly'] = 'readonly'.

Now read https://stackoverflow.com/questions/7730695/whats-the-difference-between-disabled-disabled-and-readonly-readonly-for-ht (keep in mind that this is HTML-specific - NOT Backdrop/Drupal FAPI-specific). The gist of it is this:

A readonly element is just not editable, but gets sent when the according form submits. A disabled element isn't editable and isn't sent on submit. Another difference is that readonly elements can be focused (and getting focused when "tabbing" through a form) while disabled elements can't.

The problem is that most (all?) developers working with Backdrop/Drupal expect '#disabled' to be behaving as ['#attributes']['disabled'] = 'readonly' ("lock" the form element so that its value cannot be changed, but still allow the value to be sent on form submit), when in fact it doesn't.

I think that we should document all that in https://docs.backdropcms.org/form_api#disabled

We should also document everything mentioned in: https://www.drupal.org/project/drupal/issues/500868#comment-2833796

#426056: Server-side enforcement of #disabled is inconsistent landed, making #disabled have server-side meaning: FAPI will ignore input for elements whose #disabled is TRUE (so for example, if someone uses Firebug to un-disable an element, and submit input for it, Drupal 7 will not process it). This is an important security improvement, as people writing forms that set #disabled to TRUE generally assume that input for that element won't appear in $form_state['values'] and don't necessarily write their own security checks against users un-disabling an element client-side.

I've argued against introducing a #readonly property, as we would then need to decide whether that too should have server-side enforcement, and if it should, then we'd have two properties meaning the exact same thing with respect to server-side code. Instead, we added a #allow_focus property which can be used in conjunction with #disabled to provide the UI feature of what 'readonly' means for HTML.

If you're writing forms where you don't want server-side enforcement of 'disabled' or 'readonly', but just want what these attributes mean in HTML, then instead of setting #disabled with or without #allow_focus, you can set #attributes['disabled'] and/or #attributes['readonly']. #attributes is the appropriate place for things that should purely pass through to HTML without FAPI interpretation.

[klonos]

...should we introduce a pseudo-section in the FAPI documentation, to explain that '#readonly' is not supported by design, and that people should use #attributes['readonly'] instead? ...or mention all that in the existing section for #disabled?

[klonos]

...relevant code: https://github.com/backdrop/backdrop/blob/1.x/core/includes/form.inc#L2109

  // Setting #disabled to TRUE results in user input being ignored, regardless
  // of how the element is themed or whether JavaScript is used to change the
  // control's attributes. However, it's good UI to let the user know that input
  // is not wanted for the control. HTML supports two attributes for this:
  // http://www.w3.org/TR/html401/interact/forms.html#h-17.12. If a form wants
  // to start a control off with one of these attributes for UI purposes only,
  // but still allow input to be processed if it's sumitted, it can set the
  // desired attribute in #attributes directly rather than using #disabled.
  // However, developers should think carefully about the accessibility
  // implications of doing so: if the form expects input to be enterable under
  // some condition triggered by JavaScript, how would someone who has
  // JavaScript disabled trigger that condition? Instead, developers should
  // consider whether a multi-step form would be more appropriate (#disabled can
  // be changed from step to step). If one still decides to use JavaScript to
  // affect when a control is enabled, then it is best for accessibility for the
  // control to be enabled in the HTML, and disabled by JavaScript on document
  // ready.
  if (!empty($element['#disabled'])) {
    if (!empty($element['#allow_focus'])) {
      $element['#attributes']['readonly'] = 'readonly';
    }
    else {
      $element['#attributes']['disabled'] = 'disabled';
    }
  }

[klonos]

Pinging @bugfolder re this one. Curious to see what you think.

[bugfolder]

https://docs.backdropcms.org/form_api#disabled currently says:

Description
Disables (greys out) a form input element. Setting #disabled to TRUE results in user input being ignored, regardless of how the element is themed or whether JavaScript is used to change the control's attributes.

However, a more extensive explanation, such as the comment above, would be helpful in the FAPI property description. It seems likely that more people would learn the subtleties from the FAPI docs than will go browse the form.inc code comments.

...should we introduce a pseudo-section in the FAPI documentation, to explain that #readonly is not supported by design, and that people should use #attributes['readonly'] instead? ...or mention all that in the existing section for #disabled?

If by "pseudo-section" you mean create an entry for #readonly, I don't think it would be a good practice to add entries for nonexistent properties just to explain why they're nonexistent. So yeah, #disabled would be the appropriate place to do all the explanation.

I'll do some editing/addition there, then ping here for people to review and/or suggest further edits.

[bugfolder]

FAPI already had the #allow_focus property, at https://docs.backdropcms.org/form_api#allow_focus, but it wasn't showing up in the grids because it hadn't been assigned to any form element. I've added it to checkbox just so it will show up. (It needs to be added to other input form elements, which I will work on.)

In terms of adding further explanation, though, I ran into some puzzling behavior. I was going to focus the discussion on "what properties do you set to get desired outcomes", rather than a detailed discussion of underlying theory and standards (like the code comments). So I set up some test checkboxes with all possible combinations of the properties #disabled and #allow_focus, and then, just for good measure, setting the attributes disabled and readonly directly to see what current behavior is. Here's how the form rendered:

And here's what the submitted values were:

So, there's a bunch of puzzling issues with the current Backdrop core behavior, and before I start trying to describe this in the docs, I will ask the question if all this is really what's intended?

• If the element is #disabled, then the submitted value is a Boolean; otherwise it's an integer. Why not a consistent type?
• If the element is #allow_focus only, that property has no effect (which seems appropriate, since according to the code comments, #allow_focus only matters in conjuction with #disabled). BUT:
• If the element is both #disabled and #allow_focus, then it is no longer disabled. This seems wrong.
• If we disable the element using disabled attribute, then the element is disabled, but it still submits a value, and it submits the wrong value (0, even though the element is checked).
• If we use attribute readonly, the checkbox is still editable. This seems wrong.
• If we use both attributes disabled and readonly, then the element is disabled, but it submits the wrong value (0, even though the element is checked).

Here, for reference, is the test form code:

function backdrop_disabled_test_form($form, &$form_state) {
  $form['enabled'] = array(
    '#type' => 'checkbox',
    '#title' => t('Enabled'),
    '#default_value' => TRUE,
  );
  $form['property_disabled_only'] = array(
    '#type' => 'checkbox',
    '#title' => t('#disabled only'),
    '#default_value' => TRUE,
    '#disabled' => TRUE,
  );
  $form['property_allow_focus_only'] = array(
    '#type' => 'checkbox',
    '#title' => t('#allow_focus only'),
    '#default_value' => TRUE,
    '#allow_focus' => TRUE,
  );
  $form['property_both_disabled_and_allow_focus'] = array(
    '#type' => 'checkbox',
    '#title' => t('Both #disabled and #allow_focus'),
    '#default_value' => TRUE,
    '#disabled' => TRUE,
    '#allow_focus' => TRUE,
  );
  $form['attribute_disabled'] = array(
    '#type' => 'checkbox',
    '#title' => t("Use attribute 'disabled'"),
    '#default_value' => TRUE,
    '#attributes' => array(
      'disabled' => 'disabled',
    ),
  );
  $form['attribute_readonly'] = array(
    '#type' => 'checkbox',
    '#title' => t("Use attribute 'readonly'"),
    '#default_value' => TRUE,
    '#attributes' => array(
      'readonly' => 'readonly',
    ),
  );
  $form['attributes_disabled_and_readonly'] = array(
    '#type' => 'checkbox',
    '#title' => t("Use attributes 'disabled' and 'readonly'"),
    '#default_value' => TRUE,
    '#attributes' => array(
      'disabled' => 'disabled',
      'readonly' => 'readonly',
    ),
  );
   $form['submit'] = array(
    '#type' => 'submit',
    '#value' => t('Submit'),
    '#suffix' => l(t('Cancel'), 'disabled-test'),
  );
  return $form;
}

[bugfolder]

I put together a table summarizing the outcome of the form elements in the previous comment:

The key points that should presumably be documented in the #disabled property (assuming that the current behavior is, in fact, what is intended):

• $form_state['values'] will have a value for the element whether or not it has #disabled or allow_focus properties, however, the value may be meaningless in some cases as outlined below.
• The type of $form_state['values'] can vary (as well as its value), depending on whether or not it has #disabled property.
• If an element is #disabled but not #allow_focus,
  • it will not be editable;
  • it will be visually disabled;
  • its $form_state['values'] entry will exist and will have the value of the element.
• If an element is #allow_focus but not #disabled, the #allow_focus is ignored and it behaves like a normally enabled element.
• If an element is #disabled and #allow_focus,
  • it will be editable;
  • it will not be visually disabled;
  • its $form_state['values'] entry will exist and will have the value of the element.
• If instead of setting the property #disabled, you set the attribute disabled="disabled", the appearance and behavior of the element will be the same but in the latter case $form_state['values'] will have a meaningless value.
• Setting both properties #disabled and allow_focus creates an editable element, while setting both attributes disabled and readonly creates a non-editable element whose $form_state['values'] value is meaningless.

I think that's a accurate description of the current behavior. Is it what the current behavior actually ought to be? In particular, I would have thought that based on the statement above,

FAPI will ignore input for elements whose #disabled is TRUE (so for example, if someone uses Firebug to un-disable an element, and submit input for it, Drupal 7 will not process it). This is an important security improvement, as people writing forms that set #disabled to TRUE generally assume that input for that element won't appear in $form_state['values'] and don't necessarily write their own security checks against users un-disabling an element client-side.

that any element that is #disabled (or, more broadly, any element that doesn't submit a value in $_POST) would not create an entry in $form_state['values'], but it appears that everything does, whether the value has any relation to the value of the input element.