Contrib.social

Deprecate magic cancel submit handlers in views ui

Open on Drupal.org →
Created on 16 July 2025, 11 days ago

Problem/Motivation

ViewsUI::getStandardButtons adds has some magic to find cancel submit handlers, that looks for functions with the same name as the form id, with the suffix "_cancel".

The code looks like this, and has existed since at least views 6.x-2.x in contrib.

$cancel_submit = function_exists($form_id . '_cancel') ? $form_id . '_cancel' : [$this, 'standardCancel'];

But this is a relic to a time when forms exists in procedural functions in the global namespace. Forms now belong in their own classes, with submit handlers bundled in to the class. There are no _cancel functions left in core that are associated with forms.

There is a possibility existing contrib or custom modules are relying on this behaviour, but if these functions exist in .module files they will stop working once .module files are deprecated and removed. By deprecating now these can be updated to manually specify the submit handlers in the form class.

Steps to reproduce

$ grep -r "function .*_cancel(" core
core/modules/user/user.api.php:function hook_user_cancel($edit, UserInterface $account, $method): void {
core/modules/user/user.module:function user_cancel($edit, $uid, $method): void {
core/modules/user/user.module:function _user_cancel($edit, $account, $method): void {

Proposed resolution

Refactor $cancel_submit to throw a deprecation if the function_exists call finds something.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

📌 Task
Status

Active

Version

11.0 🔥

Component

views_ui.module

Created by

🇦🇺Australia mstrelan

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mstrelan
  • Merge request !12734Deprecate → (Open) created by mstrelan
  • Comment 11 days ago →
    🇦🇺Australia mstrelan

    Did the deprecation, needs a change record. Could possibly do a deprecation test but it would involve adding a test module with a form that calls $view->getStandardButtons similar to \Drupal\views_ui\Form\Ajax\ReorderDisplays and also declare a function with the magic naming convention.

  • Pipeline finished with Failed
    11 days ago
    Total: 184s
    #548674
  • Pipeline finished with Failed
    11 days ago
    Total: 157s
    #548677
  • Comment 11 days ago →
    🇺🇦Ukraine voleger Ukraine, Rivne

    Link related issue https://www.drupal.org/project/drupal/issues/3043725 ✨ Provide a Entity Handler for user cancelation Needs work

  • Comment 11 days ago →
    🇦🇺Australia mstrelan

    I don't think that issue is related

  • Comment 10 days ago →
    🇺🇸United States nicxvan

    This needs a CR and the deprecation needs to be updated with the correct CR.

    Deprecations don't need a test, the rest of the MR does look good.

    I removed the unrelated issue.

  • Comment 10 days ago →
    🇦🇺Australia mstrelan

    Agree we don't normally need a test for a deprecation, unless it's a weird implementation. I think what I really meant that we don't actually have any evidence that the behaviour we're deprecating even works in the first place, so we could test for that as a deprecation test. But I don't know if it's worth it.

  • Comment 9 days ago →
    🇬🇧United Kingdom catch

    It feels very unlikely that anyone is relying on this, and fairly unlikely that it even works at all, so for me fine to skip deprecation test coverage here.

  • Comment 9 days ago →
    🇺🇸United States nicxvan

    Great thanks! I created a CR and added a suggestion to change the deprecation to use the CR.

    It could use a pass.

  • Comment 6 days ago →
    🇦🇺Australia mstrelan

    Applied the suggestion from the MR and updated the CR to make it specific to Views UI and with a more realistic example.

  • Pipeline finished with Failed
    6 days ago
    Total: 153s
    #552574
  • Comment 6 days ago →
    needs-review-queue-bot

    The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • Pipeline finished with Success
    6 days ago
    Total: 585s
    #552585
  • Comment 6 days ago →
    🇦🇺Australia mstrelan
  • Comment 6 days ago →
    🇺🇸United States nicxvan

    Huh it seemsmy comment was eaten but you got my suggestions anyway.

    CR is far better now thanks!

    Deprecation is correct now too.

    Good to go.

  • Comment 1 day ago →
    🇺🇸United States xjm

    Nice find. This is an excellent cleanup, but we should probably have subsystem maintainer signoff to ensure we're not leaving behind any trailing related bits etc. I'm technically only a maintainer for Views, not Views UI, so bumping back to NR. I'll also ping Lendude.

  • Comment 1 day ago →
    🇺🇸United States xjm

    We might want to target Drupal 13 for this deprecation. I tried to search contrib, but ran into problems with the fuzziness (it refusing to do an exact search for _cancel().

  • Comment about 12 hours ago →
    🇳🇱Netherlands Lendude Amsterdam

    Can't think of (or find) anything either that could possibly rely on this. So great clean up as far as I can tell.

  • Comment about 4 hours ago →
    🇦🇺Australia mstrelan

    So is this RTBC or need to be updated for D13? It seems incredibly niche.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024