PrepareModulesEntityUninstallForm::formTitle does not exist

I am facing the same issue, before seeing this ticket I created following ticket:

https://www.drupal.org/node/2876872

I applied the patch its working fine.

Looks good to me as well :)

+++ b/core/modules/system/system.routing.yml
@@ -427,7 +427,7 @@ system.prepare_modules_entity_uninstall:
-    _title_callback: '\Drupal\system\Form\PrepareModulesEntityUninstallForm::formTitle'
+    _title: 'Confirm content uninstall'

Feels like we're missing test coverage somewhere then. Why isn't \Drupal\system\Tests\Module\PrepareUninstallTest::testUninstall() broken? It is because of the theme we're testing in? And how do the title's appear correct?

I guess it because of

$form['#title'] = $this->getQuestion();

in \Drupal\Core\Form\ConfirmFormBase::buildForm()

I got this from breadcrumbs. Do we not print breadcrumbs on this page by default?

@heddn breadcrumbs are on in the Seven theme.

Patch #3 fixed my issue, thanks @markcarver

Patch #3 resolved my issue The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">InvalidArgumentException</em>: The controller for URI &quot;&quot; is not callable.

This error would occur when attempting to uninstall a module's entities. After applying the patch and perform a cache rebuild the error is resolved on Drupal 8.7.3 PHP 7.3.5.

+++ b/core/modules/system/system.routing.yml
@@ -427,7 +427,7 @@ system.prepare_modules_entity_uninstall:
+    _title: 'Confirm content uninstall'

I think we should make this exist so that the form title matches the actual form title. So something like:

  /**
   * Gets the form title.
   *
   * @param string $entity_type_id
   *   The entity type ID.
   *
   * @return \Drupal\Core\StringTranslation\TranslatableMarkup
   *   The form title.
   */
  public function formTitle($entity_type_id = NULL) {
    $this->entityTypeId = $entity_type_id;
    if (!$this->entityTypeManager->hasDefinition($this->entityTypeId)) {
      throw new NotFoundHttpException();
    }
    return $this->getQuestion();
  }

And we're missing test coverage :(

this not been fixed yet? I have 8.7.7 and still seeing this.

+1 for fixing this ;-)

this is 3 years old issue about restoring a form title :)

I do agree with #9 and #18 and I experienced on my own skin that confirm forms make things unpredictable, but the underlying core issue should be resolved in a followup task I believe. (Maybe there won't be a BC solution for confirm forms vs. page titles.)

 public function formTitle($entity_type_id = NULL) {
    $this->entityTypeId = $entity_type_id;
    if (!$this->entityTypeManager->hasDefinition($this->entityTypeId)) {
      throw new NotFoundHttpException();
    }

How this could practically happen if there is a proper access checking on the route?

In my case I could drag an issue with this down to Easy Breadcrumb module, see #3164758: Easy breadcrumb breaks the form for removing module content entities data before uninstallation.

But it still does not make sense to me that core defines a _title_callback that is not existing.

It seems that core is not validating (or even using) this _title_callback in its implementation of the PrepareModulesUninstallForm, therefore no error is raised. Easy Breadcrumb is using TitleResolver service and that breaks the form by raising an exception.

What would be the downside or risk of fixing this in core either by implementing the _title_callback in PrepareModulesEntityUninstallForm or using static title like proposed by @markcarver?

Thank you stefan, I also faced the same issue with the easy breadcrumn module and the patch of easy breadcrumb module worked for me. I would definitely be good to have core defines a _title callback that is not existing.

Thank you @Ankit Agrawal. My problem was related to Easy Breadcrumbs as well. Updating to the latest dev version fixed it for me.

- Added the method formTitle as mentioned in ##1
- Added test case to ensure that title exists and as per the method's output
- Test to ensure that entity deletion process works as expected.

Patch #29 works as expected on Drupal 8.9.14 so assuming it also applies cleanly to 9.0.x , 9.1.x as well as 8.9.x

Thanks so much for your work on this!

Great work everyone above!

Could we get a test-only patch to show it failing?

If I'm reading the issue correctly, when the page is actually visited, this title callback doesn't get called, because ConfirmForm takes over. Therefore the title callback is only getting called in edge cases like breadcrumbs - but if so I don't think the test coverage is actually exercising this.

Also needs an issue summary update.

+++ b/core/modules/system/src/Form/PrepareModulesEntityUninstallForm.php
@@ -90,6 +90,23 @@ public function getCancelUrl() {
+    }

Additionally, if this is really necessary, it needs a comment as to how we're running this code when the route ought to be a 404.

PR created from #29

Thanks for the heads up @joseph.olstad and @mohit_aghera for the original patch! Moving to D9 is easier when you can uninstall modules that are getting in the way of a clean upgrade.

#29 worked for me on 8.9.20 with some fuzz:

cd {my_drupal_root}
patch -p1 < 2862702-28_0.patch 
patching file core/modules/system/src/Form/PrepareModulesEntityUninstallForm.php
Hunk #1 succeeded at 89 (offset -1 lines).
patching file core/modules/system/tests/src/Functional/Module/InstallUninstallTest.php
Hunk #1 succeeded at 4 with fuzz 2 (offset 1 line).
Hunk #2 succeeded at 229 with fuzz 1 (offset 6 lines).

Attached is a rerolled version (2862702-38.patch) of #29 that applies cleanly (no fuzz) on 8.9.20 and reroll_diff: reroll_diff-29-38.txt.

NOTE: the reroll name is based on the comment where it was originally posted (#29), not the file suffix on the original patch (-28_0)

Drupal 8 is EOL-ed, if this patch needs a reroll then the reroll should be for Drupal 9.4.x otherwise it never gets fixed in core :S

Hiding D8 patches, the MR above which needs review.

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Left a few comments in the MR.
Also needs issue summary update per #33

@dscl and I worked on this at DrupalSouth
it is ready for review now

Left some comments on the MR

FWIW the test passes without the fix. That's because we're not actually calling any code results in the title callback being used.

Maybe we should go back to #3. Does this page actually need a title callback - hmmm maybe we do as I guess Easy Breadcrumb is using it.

Anyway to test this we need to use the title resolver service on this route. Visiting the page won't trigger the code because of the #title in the form.

Addressed feedback

Only moving to NW for the issue summary update

Remove the _title_callback for this route and just replace it with a static title. The confirm form overrides this value anyway.

Is that still true?

Thanks! Summary now appears to match the MR.

Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!