Problem/Motivation

The Help page for token module (/admin/help/token) only shows a single character - '1'. This should show a token browser.

Comments

hussainweb created an issue. See original summary.

hussainweb’s picture

Status: Active » Needs review
FileSize
785 bytes

I should clarify that this might not always happen. The problem is this code:

    $current_url = Url::fromRoute('');
    if ($current_url->toString() != '/admin/help/token') {
      // Because system_modules() executes hook_help() for each module to 'test'
      // if they will return anything, but not actually display it, we want to
      // return a TRUE value if this is not actually the help page.
      return TRUE;
    }

The toString() method returns the full path including any directory prefix (which was the case in my test instance). We can use getInternalPath() instead which returns only the part of the route we care about.

However, that said, I am not sure if this part is really required anymore. This might have been required in Drupal 7 where the $path passed by the help module would be a made-up path; however, the route here is not. I tested this manually by applying breakpoints and trying various help pages. The patch removes the block altogether.

Berdir’s picture

The reason it's there is to avoid wasting a lot of time on admin/modules for building that token tree but then not actually using it, since it is not displayed.

I think that still happens, but it's possible that there is a better way to check that now, can you check what kind of routes are passed in on that page exactly? And if hook_help() is really still called there? ( it used to be to decide if the help link for a module should be shown or not)

hussainweb’s picture

@Berdir: Not as per my tests. The hook is still called but the route_name is different because of which it does not even enter the first if check.

I repeated the tests to be sure, though. I cleared the cache before loading the help pages.

  • For admin/help, $route_name is 'help.main'. It never enters the if block.
  • For admin/help/field, $route_name is 'help.page'. Never enters the if block.
  • For admin/help/rdf, $route_name is 'help.page'. Never enters the if block.
  • For admin/help/token, this hook is fired twice. First with $route_name = 'help.page.token' which enters the first if check but returns without rendering the token browser (which is the bug). Second invocation of hook has $route_name = 'help.page', and never enters the if block.

This means that the hook is invoked for all hook pages but the route name we are checking is only set for token's own help page. This also means that we can safely remove the entire inner if block.

  • Berdir committed c0f0b3f on 8.x-1.x authored by hussainweb
    Issue #2639324 by hussainweb: Fixed help page does not display the token...
Berdir’s picture

Status: Needs review » Fixed

Yeah, other help pages are definitely fine, not worried about that. But admin/modules would be different. But that was fixed in core, the relevant part is:

// Generate link for module's help page. Assume that if a hook_help()
    // implementation exists then the module provides an overview page, rather
    // than checking to see if the page exists, which is costly.
    if ($this->moduleHandler->moduleExists('help') && $module->status && in_array($module->getName(), $this->moduleHandler->getImplementations('help'))) {
      $row['links']['help'] = array(
        '#type' => 'link',
        '#title' => $this->t('Help'),
        '#url' => Url::fromRoute('help.page', ['name' => $module->getName()]),
        '#options' => array('attributes' => array('class' =>  array('module-link', 'module-link-help'), 'title' => $this->t('Help'))),
      );
    }

This does solve the exact problem that we have. Yay D8 :) So yes, removing it s fine. Committed.

Status: Fixed » Closed (fixed)

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