search

backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

[DX] Remove usages of deprecated `$form_state['clicked_button']` from core (so that contrib doesn't use it as example) #6637

Open klonos opened 3 months ago

klonos commented 3 months ago

$form_state['clicked_button'] was removed in favor of $form_state['triggering_element'] even in D7. See https://git.drupalcode.org/project/drupal/-/blob/7.x/includes/form.inc?ref_type=heads#L257

...
 *   - triggering_element: (read-only) The form element that triggered
 *     submission. This is the same as the deprecated
 *     $form_state['clicked_button']. It is the element that caused submission,
 *     which may or may not be a button (in the case of Ajax forms). This key is
 *     often used to distinguish between various buttons in a submit handler,
 *     and is also used in Ajax handlers.
 *   - clicked_button: Deprecated. Use triggering_element instead.
...

We also have a @todo note in form_builder() to remove the backwards compatibility workaround in Backdrop 2.0. See https://github.com/backdrop/backdrop/blob/1.x/core/includes/form.inc#L2056:

      // @todo Legacy support. Remove in Backdrop 2.x.
      $form_state['clicked_button'] = $form_state['triggering_element'];

This task here is NOT to remove the backwards-compatible workaround itself (which would be a 2.x tasks), rather than to make sure that we are not using it in core. A quick grep shows 40+ instances of that currently.

It would be a nice bonus if there was a way to log a warning, similar to watchdog_deprecated_function(), so that contrib and custom code that is using this can be notified to switch to triggering_element instead.

klonos commented 3 months ago

PR up for review/testing: https://github.com/backdrop/backdrop/pull/4821

It would be a nice bonus if there was a way to log a warning, similar to watchdog_deprecated_function(), so that contrib and custom code that is using this can be notified to switch to triggering_element instead.

I could not figure out any way to do that. If anyone has any ideas, please let me know.

Related to the above, I have tried adding clicked_button as a "forbidden word" in our CSpell configuration, with the suggestion of triggering_element, which should catch any accidental additions to the core codebase in the future.

PS: There is a general problem with CSpell flagged words and suggestions not working as expected for us at the moment, which could either be a CSpell bug, or us not having configured CSpell correctly. Please follow https://github.com/streetsidesoftware/cspell/issues/5835 for details on that. In any case, the configuration will be there for when things start working with CSpell.