Posted by melbs on

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.

Files: 
CommentFileSizeAuthor
#12 interdiff.txt2.62 KBLinL
#12 fix_docs_2318755-12.patch11.01 KBLinL
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,825 pass(es).
[ View ]
#7 fix_docs_2318755-7.patch11.09 KBDevin Carlson
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,757 pass(es).
[ View ]
#6 fix_docs_2318755-4.patch2.91 KBeffulgentsia
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,159 pass(es).
[ View ]
#4 fix_docs_2318755-4.patch2.89 KBDevin Carlson
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch fix_docs_2318755-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

melbs’s picture

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

melbs commented
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

xjm commented
Priority:Critical» Normal
Devin Carlson’s picture

Devin Carlson commented
Status:Active» Needs review
StatusFileSize
new fix_docs_2318755-4.patch2.89 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch fix_docs_2318755-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

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

effulgentsia commented
Status:Needs work» Needs review
StatusFileSize
new fix_docs_2318755-4.patch2.91 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,159 pass(es).
[ View ]

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

Devin Carlson commented
StatusFileSize
new fix_docs_2318755-7.patch11.09 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,757 pass(es).
[ View ]

Added the missing fixes to #6.

LinL’s picture

LinL commented
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

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

catch commented
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

jhodgdon commented

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

LinL commented
Status:Needs work» Needs review
StatusFileSize
new fix_docs_2318755-12.patch11.01 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,825 pass(es).
[ View ]
new interdiff.txt2.62 KB

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

catch’s picture

catch commented
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

jhodgdon commented
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.