Issue #2707109 by alexpott, jhodgdon: Help block should not be displayed when...

Issue #2707109 by alexpott, jhodgdon: Help block should not be displayed when hook_help() implementations return an empty string

core/modules/help/src/Plugin/Block/HelpBlock.php

@@ -80,46 +80,24 @@ public static function create(ContainerInterface $container, array $configuratio
   }
 
   /**
-   * Returns the help associated with the active menu item.
-   *
-   * @param \Symfony\Component\HttpFoundation\Request $request
-   *   The current request.
-   *
-   * @return array|string
-   *   Render array or string containing the help of the matched route item.
+   * {@inheritdoc}
    */
-  protected function getActiveHelp(Request $request) {
+  public function build() {
     // Do not show on a 403 or 404 page.
-    if ($request->attributes->has('exception')) {
-      return '';
+    if ($this->request->attributes->has('exception')) {
+      return [];
     }
 
     $help = $this->moduleHandler->invokeAll('help', array($this->routeMatch->getRouteName(), $this->routeMatch));
     $build = [];
-    foreach ($help as $item) {
-      if (!is_array($item)) {
-        $item = ['#markup' => $item];
-      }
-      $build[] = $item;
-    }
 
-    return $build;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function build() {
-    $help = $this->getActiveHelp($this->request);
-    if (!$help) {
-      return [];
-    }
-    elseif (!is_array($help)) {
-      return ['#markup' => $help];
-    }
-    else {
-      return $help;
+    // Remove any empty strings from $help.
+    foreach (array_filter($help) as $item) {
+      // Convert strings to #markup render arrays so that they will XSS admin
+      // filtered.
+      $build[] = is_array($item) ? $item : ['#markup' => $item];
     }
+    return $build;
   }
 
   /**

core/modules/help/src/Tests/HelpBlockTest.php

+<?php
+
+namespace Drupal\help\Tests;
+
+use Drupal\simpletest\WebTestBase;
+
+/**
+ * Tests display of help block.
+ *
+ * @group help
+ */
+class HelpBlockTest extends WebTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['help', 'help_page_test', 'block'];
+
+  /**
+   * The help block instance.
+   *
+   * @var \Drupal\block\Entity\Block
+   */
+  protected $helpBlock;
+
+  protected function setUp() {
+    parent::setUp();
+    $this->helpBlock = $this->placeBlock('help_block');
+  }
+
+  /**
+   * Logs in users, tests help pages.
+   */
+  public function testHelp() {
+    $this->drupalGet('help_page_test/has_help');
+    $this->assertText(t('I have help!'));
+    $this->assertText($this->helpBlock->label());
+
+    $this->drupalGet('help_page_test/no_help');
+    // The help block should not appear when there is no help.
+    $this->assertNoText($this->helpBlock->label());
+  }
+
+}

core/modules/help/tests/modules/help_page_test/help_page_test.module

@@ -17,6 +17,11 @@ function help_page_test_help($route_name, RouteMatchInterface $route_match) {
       // Make the help text conform to core standards. See
       // \Drupal\system\Tests\Module\InstallUninstallTest::assertHelp().
       return t('Read the <a href=":url">online documentation for the Help Page Test module</a>.', [':url' => 'http://www.example.com']);
+    case 'help_page_test.has_help':
+      return t('I have help!');
   }
 
+  // Ensure that hook_help() can return an empty string and not cause the block
+  // to display.
+  return '';
 }

core/modules/help/tests/modules/help_page_test/help_page_test.routing.yml

+help_page_test.has_help:
+  path: '/help_page_test/has_help'
+  defaults:
+    _controller: '\Drupal\help_page_test\HelpPageTestController::hasHelp'
+  requirements:
+    _access: 'TRUE'
+
+help_page_test.no_help:
+  path: '/help_page_test/no_help'
+  defaults:
+    _controller: '\Drupal\help_page_test\HelpPageTestController::noHelp'
+  requirements:
+    _access: 'TRUE'

core/modules/help/tests/modules/help_page_test/src/HelpPageTestController.php

+<?php
+
+namespace Drupal\help_page_test;
+
+/**
+ * Provides controllers for testing the help block.
+ */
+class HelpPageTestController {
+
+  /**
+   * Provides a route with help.
+   *
+   * @return array
+   *   A render array.
+   */
+  public function hasHelp() {
+    return ['#markup' => 'A route with help.'];
+  }
+
+  /**
+   * Provides a route with no help.
+   *
+   * @return array
+   *   A render array.
+   */
+  public function noHelp() {
+    return ['#markup' => 'A route without help.'];
+  }
+
+}