Help page does not display the token browser [#2639324]

Problem/Motivation

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

Comment	File	Size	Author
#2	help_page_does_not-2639324-2.patch	785 bytes	hussainweb
#2	8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 32 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

22 December 2015 at 15:01

hussainweb created an issue. See original summary.

Comment #2

hussainweb

English

CreditAttribution: hussainweb at Axelerant commented 22 December 2015 at 15:07

Status:

Active

» Needs review

File	Size
help_page_does_not-2639324-2.patch	785 bytes
8.x-1.x: PHP 5.5 & MySQL 5.5, D8.1 32 pass

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

    $current_url = Url::fromRoute('<current>');
    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.

Comment #3

Berdir

German

Switzerland

CreditAttribution: Berdir at MD Systems GmbH commented 23 December 2015 at 07:07

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)

Comment #4

hussainweb

English

CreditAttribution: hussainweb at Axelerant commented 23 December 2015 at 07:16

@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.

Comment #5

23 December 2015 at 07:24

Berdir committed c0f0b3f on 8.x-1.x authored by hussainweb

Issue #2639324 by hussainweb: Fixed help page does not display the token...

Comment #6

Berdir

German

Switzerland

CreditAttribution: Berdir at MD Systems GmbH commented 23 December 2015 at 07:25

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.