Follow-up to #2034879: [Meta] Fix documentation that refers to enabling/disabling of modules

66 instances of the word "enabl". Some may not need to be changed.
40 instances of the word "disabl". Some may not beed to be changed.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

melbs’s picture

Title: [Meta] Fix documentation that refers to enabling/disabling of modules: Block Module » Block Module: Fix documentation that refers to enabling/disabling of modules
Assigned: melbs » Unassigned
Issue summary: View changes
melbs’s picture

Title: Block Module: Fix documentation that refers to enabling/disabling of modules » [Meta] Block Module: Fix documentation that refers to enabling/disabling of modules
xjm’s picture

Priority: Critical » Normal
Devin Carlson’s picture

Status: Active » Needs review
FileSize
2.89 KB

Attaching the patch from #2318813-5: Comment module: Fix documentation that refers to enabling/disabling of modules which was for block.module but was posted in the issue for comment.module.

Credit goes to @Lowell.

Status: Needs review » Needs work

The last submitted patch, 4: fix_docs_2318755-4.patch, failed testing.

effulgentsia’s picture

Status: Needs work » Needs review
FileSize
2.91 KB

Just a reroll. I think the patch is still incomplete though. For example, there are tests that say "Modules to enable." that need to be fixed.

Devin Carlson’s picture

FileSize
11.09 KB

Added the missing fixes to #6.

LinL’s picture

Status: Needs review » Reviewed & tested by the community

Looks good. I searched core/modules/block for enabl and disabl and there are none that relate to modules. RTBC.

+++ b/core/modules/block/block.module

@@ -34,7 +34,7 @@ function block_help($route_name, RouteMatchInterface $route_match) {
...
-      $output .= '<dd>' . t('You can add custom blocks, if the the <em>Custom Block</em> module is enabled on the <a href="!extend">Extend page</a>. For more information, see the <a href="!blockcontent-help">Custom Block help page</a>.', array('!extend' => \Drupal::url('system.modules_list'), '!blockcontent-help' => \Drupal::url('help.page', array('name' => 'block_content')))) . '</dd>';
+      $output .= '<dd>' . t('You can add custom blocks, if the the <em>Custom Block</em> module is installed on the <a href="!extend">Extend page</a>. For more information, see the <a href="!blockcontent-help">Custom Block help page</a>.', array('!extend' => \Drupal::url('system.modules_list'), '!blockcontent-help' => \Drupal::url('help.page', array('name' => 'block_content')))) . '</dd>';

Not related to this issue, but as we are changing the line anyway, I wonder if we can change "the the" to "the" here? If not, I'll file a separate issue.

LinL’s picture

Title: [Meta] Block Module: Fix documentation that refers to enabling/disabling of modules » Block Module: Fix documentation that refers to enabling/disabling of modules
catch’s picture

Status: Reviewed & tested by the community » Needs work

Great to see these sub-issues.

Couple of comments:

  1. +++ b/core/modules/block/block.module
    @@ -108,8 +108,9 @@ function _block_rehash($theme = NULL) {
         }
    

    I don't think this can be right. If a module is uninstalled the block will disappear altogether. However isn't this talking about block being disabled anyway?

  2. +++ b/core/modules/block/src/BlockPluginCollection.php
    @@ -64,8 +64,9 @@ protected function initializePlugin($instance_id) {
    +      // Ignore blocks belonging to uninstalled modules, but re-throw valid
    

    We should open a follow-up to review this. It shouldn't be possible to get into this state by uninstalling a module - all the blocks should be removed then.

jhodgdon’s picture

We already have an issue open for updating hook_help for the Block module but yes let's remove "the the" here. I mentioned it on #2349907: Review and fix block hook_help text in case it doesn't get fixed here.

I also agree with the block rehash assessment -- the code comment is really talking about disabled blocks, not disabled modules.

LinL’s picture

Status: Needs work » Needs review
FileSize
11.01 KB
2.62 KB

Rerolled. I changed the block rehash comment and also deleted the extra "the".

catch’s picture

Status: Needs review » Reviewed & tested by the community

Opened the follow-up for that exception #2385517: Block initialization eats exceptions if a module is missing.

Changes look good so moving back to RTBC.

  • jhodgdon committed 7c3071b on 8.0.x
    Issue #2318755 by LinL, Devin Carlson, effulgentsia, catch: Block Module...
jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

This has been properly beta-evaluated, and is purely docs fixes. Looks good to me too. Committed to 8.0.x.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.