Problem/Motivation

Help module's help_help() incorrectly contains a mention of "context-sensitive help" which actually originates from the system module. Additionally, system module's hook_help() fails to mention this fact.

Proposed resolution

Patch in #3 adequately solves the original scope of this issue, however system module's help information needs to reflect its additional functionality. Additional patch needed. Working on this now.

Remaining tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Remove mentions of "context-sensitive" from help module Novice Update documention. Patch in #3 by @er.pushpinderrana
Reroll patch in #3 to include system module edits Novice Add help text describing system module's System Help block. [DONE]
Manual test Novice Make sure both Help and System module help visible from admin/help is still formatted OK. Also, in the new section in System help (the section on displaying administrative help), make sure the links work, and that UI text matches the actual Drupal UI

User interface changes

API changes

Original report by @jhodgdon

While reviewing the patch for another issue, I learned that the functionality of "providing context-sensitive help on individual pages of your site" is actually a function of the System module, not the Help module. So this text in the help module's function help_help():

      $output .= '<dt>' . t('Providing context-sensitive help') . '</dt>';
      $output .= '<dd>' . t('The Help module displays context-sensitive advice and explanations on various pages.') . '</dd>';
      $output .= '</dl>';

and also the mention above in About, are wrong. This is not something the Help module does. It is part of the System module, and it also is only displayed if you have the System Help block shown.

So, we should remove mentions of context-sensitive help from this help text. Should be a fairly easy novice patch... it is function help_help() in core/modules/help/help.module, and inside case 'help.page.help'.

Comments

er.pushpinderrana’s picture

Status:Active» Needs review
StatusFileSize
new810 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,604 pass(es).
[ View ]

Attached patch would remove this context-sensitive help from this help text.

jhodgdon’s picture

Status:Needs review» Needs work

Thanks, what you did looks good! But there is also a mention of the context-sensitive help up a few lines in the "About" section. Can you fix that too please?

er.pushpinderrana’s picture

Status:Needs work» Needs review
StatusFileSize
new2.31 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,546 pass(es).
[ View ]

Thankyou Jennifer for correcting me.

In last patch missed it but this time deleted from About section too.

jhodgdon’s picture

Status:Needs review» Reviewed & tested by the community

Looks good, thanks!

webchick’s picture

Status:Reviewed & tested by the community» Needs work

I'm struggling with this a bit because we're not moving the help text to system module (for good reason), we're simply removing it. That means we also remove any ability for users to learn about this feature. OTOH, this contextual help is *not* in fact a user-facing feature; it's something for module developers to then simply be displayed to end-users.

I think what we should do then is move a variant of this documentation to the doxygen of hook_help() (which is sort of arguing against myself in the other issue) so that module developers realize hook_help() has this capability. Ideally this would be coupled with an example of providing dynamic help as well.

webchick’s picture

hook_help(), btw, is documented in help.api.php, which brings it back to Help module's domain. D'oh. :)

jhodgdon’s picture

I agree that the hook_help docs should be moved into a system.module *.api.php file. I also agree it should mention how the help is displayed, which it currently doesn't do. Can we do that on a separate issue please though? I filed:
#2263047: hook_help should be moved and should explain how help is displayed
for that piece.

We have a separate issue open to update the system module hook_help(). It is really missing a LOT, not just this piece, and I had already added a comment there that it needs this piece. I would rather not do this as part of this issue here, but we could... that other issue is postponed. See #2091363-12: Update hook_help for System module

jhodgdon’s picture

Issue tags:+Needs backport to Drupal 7

Discussed this with webchick in IRC... We should also (for now) update the System module help (add a "Uses" item) to talk about context-sensitive help being displayed in the System Help block. And we should do it in this patch. And then when we've got this into 8, backport to Drupal 7, because we checked and the system behaves the same way there.

aschmoe’s picture

Issue summary:View changes
Issue tags:-
aschmoe’s picture

Issue summary:View changes
aschmoe’s picture

Issue summary:View changes
aschmoe’s picture

Issue summary:View changes
jhodgdon’s picture

Issue summary:View changes

I just edited the issue summary to remove a task. We are not wanting to wait for the other issue to be resolved. For now we need to document what the modules do now, and I think we're going to continue to have top-of-page help anyway.

jhodgdon’s picture

Issue summary:View changes

Also... If someone is actually working on this, please assign the issue to yourself... for the moment have taken out the "working on this now" from the issue summary, since I do not think anyone is working on it.

er.pushpinderrana’s picture

Assigned:Unassigned» er.pushpinderrana

Thank you Jennifer for your additional information.
I am working on this now.

er.pushpinderrana’s picture

Status:Needs work» Needs review
StatusFileSize
new4.18 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,943 pass(es).
[ View ]

Added context-sensitive advice information to system module in this patch.

jhodgdon’s picture

Status:Needs review» Needs work

Looks good... but

+      $output .= '<dt>' . t('Providing context-sensitive help') . '</dt>';
+      $output .= '<dd>' . t('The System module displays context-sensitive advice and explanations on various pages.') . '</dd>';

This isn't very great documentation, because it doesn't mention that this help is displayed in a block. If someone doesn't have this block placed in a region in their admin theme, they won't get the hep displayed. So... We have hook_help pages for various modules that refer to blocks; we should do something similar here to other help that mentions blocks.

Also maybe we should say that this help is provided by other modules, and is generally for administration pages?

er.pushpinderrana’s picture

Jennifer, Thanks a lot for your valuable feedback. Please review following documentation and correct me if require some more changes in this. Once you will confirm the final documentation, I would add the same in final patch.

+      $output .= '<dt>' . t('Managing Context') . '</dt>';
+      $output .= '<dd>' . t('The System module displays context-sensitive advice and explanations on administration pages, also provided by other modules that helps logged in users to quickly configure blocks and regions contents without navigating to the admin Dashboard. Administrator can enable and disable contextual links and can specify the user roles that have permission to view and use contextual links. For more information, see the online handbook entry for <a href="http://drupal.org/documentation/modules/contextual">Contextual Links module</a>.') . '</dd>';
jhodgdon’s picture

No... We are not talking here about contextual links -- that is something different, and is provided by the Contextual Links module. We are talking here about the System Help block.

er.pushpinderrana’s picture

Sorry I get confused in between contextual links and context-sensitive help. Now I did research on this and found the difference. I hope this time I am going in right direction, please evaluate following documentation.

+      $output .= '<dt>' . t('Providing context-sensitive help') . '</dt>';
+      $output .= '<dd>' . t('The System module displays context-sensitive advice and explanations on administration pages that displays a brief explanation of various settings, uses, and configuration options to help guide administrators and users. Administrators cannot alter this help text, as it is built into each module code. Most of modules provides this help for example see the <a href="@modules">module pages</a>.  You can also access the help pages via any of the "Help" links on the Module administration page (Manage > Extend or http://example.com/admin/modules).', array('@modules' => url('admin/modules'))) . '</dd>';

Thank you for your support and guidance.

aschmoe’s picture

The last part worth mentioning is the System Help block is defined by the system module

t('The System Help block is defined by the System module, and displays context-specific (path based) help text provided by modules in a <a href="@hook-url">hook_help()</a> call.  This block can be placed, edited or removed on the <a href="@block-page">Block layout page.', array('@hook-url' => 'https://api.drupal.org/api/drupal/core!modules!help!help.api.php/function/hook_help/8', '@block-page' => url('admin/structure/block')))

jhodgdon’s picture

Ummm.... So: Generally, I like the text in #22 a lot better than the text in #21, and I believe it is more accurate as far as what the System module and other modules are doing... so let's use that as a starting point. ... how about this -- I don't think we need to say any more than this:

---
Providing administrative help

Page-specific administrative help text that is provided by the System module and other modules is displayed in the System help block. This block can be placed and configured on the Block layout page.
---

As a note: When you link to the Block layout page, make sure to check whether the Block module is enabled first. There is an example of how to do this in the Menu module hook_help().

er.pushpinderrana’s picture

Status:Needs work» Needs review
StatusFileSize
new4.37 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,056 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Thank you Jennifer and Alex.

Please review this patch, added required documentation suggested by you.

Status:Needs review» Needs work

The last submitted patch, 24: drupal8-help-mentions-context-sensitive-2261083-24.patch, failed testing.

er.pushpinderrana’s picture

Status:Needs work» Needs review
StatusFileSize
new4.42 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,061 pass(es).
[ View ]

Ahh.... I missed the else condition.

jhodgdon’s picture

Issue summary:View changes

Thanks, that looks good! Can someone please give this a quick manual test (the Help and System module help from admin/help) to make sure both are still formatted OK, and also in the new section in System help, make sure the links work, and that UI text matches the actual Drupal UI?

er.pushpinderrana’s picture

StatusFileSize
new16.73 KB
new40.46 KB

Thanks, Attached screenshots would help you to quick review of this patch.

System Help PageHelp Module Help Page

jhodgdon’s picture

Issue tags:+Needs manual testing

Thanks! The formatting looks good. So we still need a manual test on the new section in System module to verify the links work, and that the UI text matches what you see in the UI (especially the block name).

joshi.rohit100’s picture

StatusFileSize
new5.82 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,523 pass(es).
[ View ]
new3 KB

updated patch as '.' is missing in last in Performing system maintenance para.

please review now.

jhodgdon’s picture

Good catch! Patch still needs manual testing as described in #29.

amitgoyal’s picture

StatusFileSize
new15.51 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,531 pass(es).
[ View ]
new13.32 KB

Please review updated patch with following fixes in system.module,

  • <a href="@link"> converted to <a href="!link">
  • http to https for drupal.org urls
  • replacement of url() with \Drupal::url()
  • 'System help block' to 'System Help block' as the actual block name is System Help
jhodgdon’s picture

We have a separate issue to update the System help as a whole:
#2091363: Update hook_help for System module

So the patch in #32 is out of scope for this issue. Let's please keep this issue specifically only about moving this one piece of help from the Help module to System module. Thanks!

Please disregard the patch in #32 (you can put it on the other issue), and go back to the patch in #30.

amitgoyal’s picture

Sure @jhodgdon. Thanks for the update!

jhodgdon’s picture

Status:Needs review» Needs work

I went back and tested the patch in #30.

It is 99% good! Two things to fix:

a) I do not know what the change is in system.module in the patch above the new section, but can we take that out? This patch should only be adding a new section to that help function. I mean this change should be removed from the patch:

-      $output .= '<dd>' . t('In order for the site and its modules to continue to operate well, a set of routine administrative operations must run on a regular basis. The System module manages this task by making use of a system cron job. You can verify the status of cron tasks by visiting the <a href="@status">Status report page</a>. For more information, see the online handbook entry for <a href="@handbook">configuring cron jobs</a>. You can set up cron job by visiting <a href="@cron">Cron configuration</a> page', array('@status' => url('admin/reports/status'), '@handbook' => 'http://drupal.org/cron', '@cron' => url('admin/config/system/cron'))) . '</dd>';
+      $output .= '<dd>' . t('In order for the site and its modules to continue to operate well, a set of routine administrative operations must run on a regular basis. The System module manages this task by making use of a system cron job. You can verify the status of cron tasks by visiting the <a href="@status">Status report page</a>. For more information, see the online handbook entry for <a href="@handbook">configuring cron jobs</a>. You can set up cron job by visiting <a href="@cron">Cron configuration</a> page.', array('@status' => url('admin/reports/status'), '@handbook' => 'http://drupal.org/cron', '@cron' => url('admin/config/system/cron'))) . '</dd>';

b) As noted in #32, the name of the help block is "System Help", and the capitalization is incorrect in the patch.

amitgoyal’s picture

Status:Needs work» Needs review
StatusFileSize
new4.42 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,629 pass(es).
[ View ]
new3.8 KB

Please review updated patch with fixes in #35.

a) The change has been reverted.
b) The name of the help block is now "System Help".

er.pushpinderrana’s picture

Thanks @amit for quick fixing.

This patch looks same as #17 patch except "System help" changed to "System Help".

@jhodgdon, please review and let us know if still any change required.

jhodgdon’s picture

Status:Needs review» Reviewed & tested by the community

Thanks, the latest patch looks good to me too!

jhodgdon’s picture

Status:Reviewed & tested by the community» Fixed
Issue tags:-Needs manual testing

Thanks again everyone! Committed to 8.x.

  • Commit 2c77fc2 on 8.x by jhodgdon:
    Issue #2261083 by amitgoyal, joshi.rohit100, er.pushpinderrana, aschmoe...

Status:Fixed» Closed (fixed)

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