Update hook_help texts that link to modules that can be uninstalled

Great!

Maybe we need to define a test for the Help module that turns on each individual module and verifies that the Help page for that module exists and can be reached with a 200 response code, and maybe that it contains the name of that module? That would future-proof this. The problem is I think the Comment module used to depend on Node (at least in 7 it definitely did), and now it doesn't, and no one updated the hook_help. We may have other cases in Core and it would be easier to find them in an automated way than to test them all manually over and over.

I'm going to write this test now.

OK, here's a patch that tests all the help pages for non-hidden modules.

I'm getting tons of "route not found" exceptions when I run this locally, on routes that should not have problems. So, checking to see if the test bot agrees... not sure what is going on. Either the exceptions are reporting the wrong problems (bad), or the routing is not working (bad), or I've done something wrong in how I'm installing modules in the test (maybe someone can tell me what that is). ???

Here's a new test patch. I'm not sure whether we should include this test in Core or not, but it will at least help us identify the problems.

Since this test might take a while to run, it might be good to combine it with the ModuleInstallUninstall test in system/Tests/Module. To do this, we might need to just run that test with Help module installed for most of that test, and then add a few lines to test the help.

And the install/uninstall of Help module could be tested separately from that main test.

Anyway... for now this test will fail and we cannot install it at least until we fix the actual problems in help pages that this test is finding.

Those two fails are from the lines in the tests that verify that the name of the module that is in the info.yml file appears in the help with the word "module" after it.

So for instance: actions.info.yml apparently says the name of the module is "Actions", so it tests to see if the help text says "Actions module" in it, and apparently it doesn't. Same for tracker.info.yml, which has a module name of "Activity Tracker" so it looks to see if the help says "Activity Tracker module" in it, and apparently it doesn't.

Sorry for the review delay... I've been on vacation.

I don't see why we need a module exists('block') check in block_help() -- it's its own module??!?

+++ b/core/modules/block/block.module
@@ -30,7 +30,7 @@ function block_help($route_name, RouteMatchInterface $route_match) {
       $output .= '<dt>' . t('Toggling between different themes') . '</dt>';
       $output .= '<dd>' . t('Blocks are placed and configured specifically for each theme. The Block layout page opens with the default theme, but you can toggle to other installed themes.') . '</dd>';
       $output .= '<dt>' . t('Demonstrating block regions for a theme') . '</dt>';
-      $output .= '<dd>' . t('You can see where the regions are for the current theme by clicking the <em>Demonstrate block regions</em> link on the Block layout page. Regions are specific to each theme.', array('!blocks' => \Drupal::url('block.admin_display'))) . '</dd>';
+      $output .= '<dd>' . t('You can see where the regions are for the current theme by clicking the <em>Demonstrate block regions</em> link on the Block layout page. Regions are specific to each theme.', array('!blocks' => (\Drupal::moduleHandler()->moduleExists('block')) ? \Drupal::url('block.admin_display') : '#')) . '</dd>';

There are some other I think unnecessary tests in there:

ckeditor depends on editor, and editor depends on filter, so we should not need to check the existence of the filter module in ckeditor_help() or editor_help().

entity_reference, link, options, text, and file depend on field, so we should not need those tests either (we do need the tests for field_ui though in those help pages).

Node depends on text and entity_reference, both of which depend on the field module, so we should not need that test either.

Responsive image depends on image, which depends on file, which depends on field, so we should not need a test for field in that one.

The rest looks good though!

Better!

I think these tests are still not necessary:
block_content: !field-help' => (\Drupal::moduleHandler()->moduleExists('field'))
[block_content depends on text which depends on field]
text: '!formats' => (\Drupal::moduleHandler()->moduleExists('filter'))
[text depends on filter]

The rest looks right to me. Thanks!

That reminds me, I need to rework that test so that it is integrated with the existing install/uninstall test, so that we can actually add it to Core. Maybe that should be a separate issue though.

Great, let's get this in!

Note to committer: DO NOT COMMIT THE "with test" patch. The test is very time consuming. It was used to diagnose the problem here, but what we want to do is integrate it with the module install/uninstall test so that we can keep Core help clean while not slowing down the test runs. I've filed #2488032: Integrate help test into module uninstall test for that task.

So the suggestion here is to commit the other patch. I'm hiding the "with tests" patch just to avoid the possibility of committing the time-consuming test by mistake.

Also updating issue summary.

So, to test this out I would have to do the following:

1. Install Drupal Minimal
2. Install help module
3. Unistall node module

And my website should crash ? By installing this patch it should not crash the site ?
I try to unistall the node module via the Drupal UI and did not let me. In addition, I try using

drush pm-unistall node

and did not work.

What I'm missing to test this patch ?

Reroll in #36 is verified -- I agree that another issue fixed the Views module and that is the only change removed in #36 as compared to the RTBC patch in #31. So, back to RTBC.

I guess we didn't have a full beta eval in the issue summary, so adding one.

Apparently needs another reroll...

New patch checked over, agreed only change in config module, RTBC again.

I got permission from alexpott to commit this patch, so we can stop rerolling it. ;)

Committed to 8.0.x. Also credited contributors to #2468395: Core modules reference block module without first ensuring existance which is mostly a duplicate.

It seems crazy fragile that a bad link like this can cause the whole webpage to break - I created #2498675: Linking to a non-existent route should not throw an exception as a followup for that underlying issue.