The function for the "more help" link assumes the path will always have two arguments... which is just not true for D7. As I write this, matt2000 and Bdragon are working on it at #drupal-contribute. I'm going to point them to this issue and I hope they will at least annotate their thinking.

The following code starts at line 1497 in includes/menu.inc:

    // Add "more help" link on admin pages if the module provides a
    // standalone help page.
    if ($arg[0] == "admin" && user_access('access administration pages') && module_exists('help') && $function('admin/help#' . $arg[2], $empty_arg) && $help) {
      $output .= theme("more_help_link", array('url' => url('admin/help/' . $arg[2])));
    }
  }
  return $output;
}
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Shai’s picture

Matt2000 and Bdragon are working on a fix at: http://drupalbin.com/13025

matt2000’s picture

FileSize
992 bytes

Here's the simplest solution.

It could theoretically fail if a a module tried to implement more than one hook_help('admin/help#something') or if it did not follow the convention of using modulename as something. But in practice, this code covers core's actual usage, AFAIK.

bdragon’s picture

Status: Active » Needs review
FileSize
1.19 KB

And here's my version.

Move the stuff up inside the first if(), it's much easier to grok that way.

Shai’s picture

I applied the patch in #3 to a fresh HEAD install. It applied cleanly and fixed the problem.

bdragon’s picture

Let's add a test for good measure.

bdragon’s picture

Title: function menu_get_active_help() is broken (in part) » Many "More help" links missing from interface.

Changing title to describe the actual problem better.

matt2000’s picture

Priority: Normal » Critical
Status: Needs review » Reviewed & tested by the community

bdragon's re-organization of the function does indeed make it more readable, and the test should help avoid similar bugs.

Marking critical, because we definitely don't want the alpha to ship with missing help links.

webchick’s picture

Priority: Critical » Normal
Status: Reviewed & tested by the community » Fixed

Awesome! I'm glad we won't break this yet again the next time we get crazy with paths. :)

Committed to HEAD.

matt2000’s picture

Component: menu system » documentation
Assigned: Unassigned » matt2000
Status: Fixed » Needs review
FileSize
751 bytes

A quick addition to the api documentation to make explicit what the code does.

jhodgdon’s picture

Status: Needs review » Needs work

Your help doc patch has a spelling error "pathes" instead of "paths". Also, we don't normally put a line break in the middle of a paragraph -- lines in help files should wrap at nearly 80 characters. So I would start the new sentence on the same line as the end of the previous sentence.

I also don't think the sentence you added is very clear... maybe it can be clarified somewhat?

webchick’s picture

Issue tags: +Needs tests

Additionally, this appears to have introduced a regression. If you go to a page like admin/content, now it's showing "More help" even when there isn't help to begin with.

Let's fix this bug, as well as get some tests to document how this ought to be working.

jhodgdon’s picture

arianek suggested to me by email this week that the words "More help" should be changed anyway, since it's misleading. She hasn't apparently filed an issue...

The link really points to the main module help. Her suggestion was to make it say something like "Help for the xyz module", instead of "more help", because it doesn't really provide more specifics for the page you're looking at.

So if you change the name of the link in this manner, it will make more sense *and* point every page to the help screen for the module in question.

webchick’s picture

Title: Many "More help" links missing from interface. » Fix "More Help"
Priority: Normal » Critical
Issue tags: +Usability, +webchick's D7 alpha hit list

That makes sense, but this still looks crappy:

Ick

I'm also not sure how jazzed I am about exposing the concept of modules to every user with access to something in the administration panel (there is no separate "access module help" permission at the moment; it's based on administration page access).

Since there are a lot of inter-tangled issues here, let's re-purpose this one since we have all interested parties' attention.

Proposal:

- Remove this "More help" functionality altogether, since as Ariane points out, it is not in fact "More help" at all. In D7, we now link to these pages right from the modules page, so I think that's as contextual as we need for the purposes of these pages.

- Add an explicit permission around access to these pages so it's not tied to "access administration pages" (this will require an upgrade path). User 1 would still see these pages, as well as anyone in the admin role. Disadvantage is that people like content editors would no longer see a "Help" link in the toolbar. Advantage is they'd have no idea what to do with anything in there anyway. :P

Sound good?

Since this is a fairly major functionality change, I would like to get this done by Friday.

Shai’s picture

Re: "more helps" actually isn't "more help." I wrote almost the exact same e-mail to Jennifer this week that "More help" isn't really more help (maybe she got me and arianek confused?). But in any case, there seems to be agreement that the "More info" link on the config pages is a problem. My first thought was also to get rid of the link altogether. But I thought that would be a harder change to push through than changing the link text to, "More information about the $module module." But now that @webchick is proposing exactly that... I'm for it (removing the "more info" links altogether).

+1 for removing the "More info" links from the config screens.

-1 on the proposed permissions changes. I think it is unnecessary. Let's create fine grain when there is demand for it and real utility. Adding it here just adds permissions bloat, imho.

[Slightly OT: During the sprint we had the other night, I was looking at a bunch of the config screens and I found the per-config-setting help-texts to be either missing or confusing in many cases. I'll see if I can post issues on the ones that I think are in the most need of better help-text in the next couple of days.]

Shai

webchick’s picture

Bear in mind that when communication happens via e-mail, IRC discussions, etc. the rest of the community is left out of it. Please have discussions like this in the issue queue in the future.

I'm ambivalent about the permission changes. IMO, the stuff under "Help" makes no sense to anyone who isn't a site builder, and right now it's accessible to everyone with access to any page in the admin panel, even if it's only the comment approval page, for example. But this is also the case in D6 and below, so it's not critical to do anything about this here.

Bojhan’s picture

There are locations where the More help has certain value, I agree it doesn't at most places make sense. But it does in some, could we special case those? And only have them show where it makes sense? To me that sounds more like something "designed" :)

Shai’s picture

@Bojhan,

Can you give an example of a special case?

Every time we special-case something we make the code less flexible, harder to maintain.

Also, from a user experience perspective it's bad when links show up on certain pages of like type but not on others. It can seem like a bug. And anyhow, maybe the special cases you are talking about are cases in which per-config-setting help-text would better solve the problem.

Shai

Bojhan’s picture

There is absolutely no reason why it would be bad for usability, to show up in some places but not in others. This is totally dependant on the context, I would not see why people would be confused by the fact that, there are only a few places that have a More help link - only if they would start to see it as a pattern it will be confusing - so places where I imagine are :

Actions
Taxonomy
Text formats

Again what we should be aware of, that contrib might want to expose these links. Special casing in this case if implemented well should not be any trouble for contrib.

arianek’s picture

Hey - yeah, that totally wasn't me (must have been Shai!), this is the first I've heard about this.

That said, I am basically a +1 on what @webchick is saying in comment #13. The help content only makes sense at this point for site admins. So whatever contextual help links there may or may not end up being, should really be restricted from *all* users being able to see it.

(To me this brings up the issue of supporting end-users more with some parallel help content for them, but that is a whole 'nother can of worms, perhaps for D8!)

webchick’s picture

K, Bojhan has a good point. There are indeed places where it is definitely prudent to have the "More help" link to show up.

How about this? If the current path matches the path specified by the module as its "configure" path (e.g. admin/structure/block for Block module), then output the More help link.

Also, given that this actually /does/ have a point, I think we can scrap the permission change from the proposal.

Shai’s picture

@arianek,

webchick isn't talking about all users having access to help pages. She's talking about separating

  • Use the administration pages and help

into

  • Use the administration pages
  • Use the help system

I think there may be a problem with that permission in general in that it is a kind of default setting. Site admins might get confused about what it actually does. Many modules define specific admin functions which trump "use administration pages." It's not a setting like Views' "access all views" permission which gives permission to all views no matter what role based controls are set. This permission here, rather, is a default one when more specific permissions have not been set.

I'm -1 on the permissions changes for the following reasons:

  1. we are in the last push to get D7 alpha out, lots to be done, and the suggested permissions change is less important than other work that needs to be done, imho
  2. the fact that down the pike there likely could be a thoughtful overhaul of access to admin and help page permissions makes a quickly done change now less interesting
  3. the fact that this concern hasn't generated an issue in the queue means that if it is a "problem" it isn't a big one.
  4. it adds to the permissions page overwhelm by adding another permission

As I said previously, I'm +1 to removing the "More help" links.

Shai

Shai’s picture

If we go with what @webchick is proposing in #20, then "More help" should be changed to, "More information about $module."

"More" is wrong because there may not be help provided on the config page and "Help" is wrong because what they get to is general information, not anything that is specific enough to be considered "help", especially when one is drilling down and expecting more specific info and not general info.

Shai

webchick’s picture

No, I do not want to expose the concept of modules to content editors directly. I also really don't want to lead this issue off track nit-picking the label, because it's alpha critical and we need to get it fixed within the next day or so. So let's leave the label as "More help", because that's what it is, as long as the logic is changed accordingly as outlined in #20.

Shai’s picture

@webchick

Getting back to your #11, the "regression" problem, I'm assuming is the part that is alpha critical... I'm not sure I understand the bug completely.

Does the following count as bug and is it part of the same regression problem: admin/config/people/ip-blocking

On that page the "more help" link takes you to very general information about the system module. It's only reference to "IP blocking" is a link back to the page you just came from. Would your proposed solution remove the "more help" link in this case?

Shai

webchick’s picture

Correct. It'd remove "More help" from any page that is not designated as "the" primary configuration page for the module. (See the "Configuration" links on admin/modules)

Shai’s picture

@webchick, I'm now in synch with your proposals for changing the logic of when the "more help" will show as outlined in #11 and #20.

"So let's leave the label as "More help", because that's what it is, as long as the logic is changed accordingly as outlined in #20.

It's not help. It's general information. The UI suggests drilling down in which the user will expect more detailed information, when in fact, the user is zooming out to a more general view.

New string proposal: Replace "More help" with "More information".

Shai

Bojhan’s picture

@Shai Could you provide a patch that does what we are talking about? I am not sure how to do this, and we could probably leave renaming the label to another issue - to avoid killing to many kittens in this issue.

webchick’s picture

Yes, we are not debating the string change here. Follow-up for that. Let's please just fix the bug.

Shai’s picture

Re: string change... great, I'll file an issue after the bug is fixed.

Re: coding this fix. I'm sorry, but I can't offer to do it now since I'm not familiar with the relevant code and don't have time to get up to speed on it. The time I have this week to help modestly with the push for D7 Alphia will be better spent doing other things.

Since @matt2000 and @bdragon familiarized themselves with the code and the issue when they came up with the initial fix, maybe they would be in a position to do it?

Shai

matt2000’s picture

I'm working on it right now...

matt2000’s picture

Component: documentation » help.module
Status: Needs work » Needs review
FileSize
4.3 KB

Re #13, the link on 'admin/content' is not a regression, it's a documented 'feature' that's been there all along.

See node.module node_help() :

    case 'admin/content':
      // Return a non-null value so that the 'more help' link is shown.
      return ' ';

*cough* *hack* *cough*

As for the ugliness in the screenshot, that's a bug in contextual links, AFAICT. (It's still an issue under the new paradigm on admin/config/people, for example.) I could have hacked the CSS to shift the help link, but didn't feel comfortable with that for this patch. Let's figure that out in a follow up, because it's a more general problem.

For the record, I'm in favor of renaming to 'more information,' but willing to concede that to a separate non-critical issue, so I left it alone.

The attached patch implements the current proposal. Since this makes the node_help hack ineffective anyway, I removed it.

arianek’s picture

ack sorry, just ignore my comment, i didn't understand what was going on. i'm just gonna stay out of this one, i don't have time to get caught up enough to give proper input for another week!

Shai’s picture

I applied matt2000's patch in #31 to a clean install of D7 HEAD with all modules turned on. It applied cleanly and is working the way it is supposed to.

Only slightly OT: So far I've found two main config pages that have "More help" links on which there is no introductory explanation:

  1. Aggregator: admin/config/services/aggregator/settings
  2. Shortcuts: admin/config/system/shortcut

For the "More" to be meaningful, a sentence describing the purpose of the config screen should be added like it is on all the other pages that have a "More help" link.

[Totally OT note to self: Aggregator is referred to as "Feed Aggregator" in some places and "Aggregator" in others. Seems like that should be standardized... I'm sure there is some history on that one :)]

Shai

jhodgdon’s picture

RE: Shai vs. arianek: Yes, it was shai, and sorry about the confusion. Anyway, everyone is here on the discussion now.

My thought: Completely remove the More Help link as an automatic link.

If a particular screen needs a link to the module's help page, put the link in the text of the help.

webchick’s picture

Status: Needs review » Needs work

Oh, snap. +1 to jhodgdon's suggestion. It's the only sensible thing to do; based on Shai's report in #33, it seems you clear that we can't programmatically derive when the link is appropriate, and the code to do so right now is quite verbose. Maybe we can come back to auto-generation in Drupal 12 when Drupal can read and parse the help text and make its own decisions. Hmmm... ;)

Shai’s picture

I'm also +1 with Jennifer's suggestion.

Can I presume that a re-roll consists of the minus sections of matt2000's patch, but without the plus parts? If so I'd be happy to do the re-roll.

Thanks to matt2000 and Bdragon for all your help on this.

Shai

webchick’s picture

I haven't looked into it, but probably something like that. It'd be a place to start anyway. :)

matt2000’s picture

I'm going back and forth on this. My first reaction was to agree, but then I thought, do we really want to make it harder to find what information we do provide? IMO, the solution to sub-optimal help text is not to hide it, but to improve it, or at least to improve it's identification.

I'll hazard suggesting that this brings us back around to the idea of re-naming the link. Maybe 'Information for Administrators' or 'Administration Guide' ?

Here's another question which might solve our UI collision woes -- is it possible to use Contextual Links to provide the link? That gives us another layer of permissioning, as discussed earlier -- 'the 'use contextual links' permission.

Whatever we do, I think, as a general principle, removing links to documentation is bad. People can always ignore what's not useful, but not if they don't know it exists.

Shai’s picture

@matt2000,

I disagree with you about links to information always being better than no links. Even if we prevailed on the "More information" link text, I still think that most people would be disappointed after clicking on the link and will not find the help (or "information") they desire. There is indeed value in the current module help screens. But I think people will more likely get what they were looking for when they get to those pages via the main help menu. Also note that links to those pages also show up on the modules listing page. So I don't think we will be making the information harder to find, but rather we'll make the config screens cleaner while removing links that help create expectations for a more specific kind of help which you wouldn't get by clinking on one of those links.

I do think there are problems on many config screens. But I think what we need to do is provide better help-text for each setting.

Shai

jhodgdon’s picture

So have we identified a list of config screens that really could benefit from a link to their respective module help pages?

Patching the code to remove the "more help" links is relatively easy (#31 patch, sections for menu.inc, node.module, and help.text: take out the - lines and don't add the + lines. Leave out the rest of the patch).

But in my opinion, we should also patch the contextual help sections to add a link where needed (e.g. that part of node.module from the #31 patch, which added a link from admin/content to node.module help).

Do we have a list of pages where that would be helpful, besides admin/content?

Shai’s picture

@jhodgdon

So have we identified a list of config screens that really could benefit from a link to their respective module help pages?

No.

But in my opinion, we should also patch the contextual help sections to add a link where needed (e.g. that part of node.module from the #31 patch, which added a link from admin/content to node.module help).

Jennifer, I'm almost certain your example is wrong. Matt2000's patch does not add the "More help" link at admin/content. So it's harder for me to understand your point given that your example doesn't fit. I don't quite grok the contextual help suggestions, not sure if they are hard-coded or done programmatically.

I'm a big picture person. I just looked at the UI again and I'm more convinced that the module help pages make sense when clicked from the main help menu and from the module listing page, but not from the config screens.

I hope we can come to closure on this soon. I'd like to get onto some really urgent stuff, e.g. take a look at the main "shortcuts" config screen. The user will expect to be able to edit shortcuts but instead you get to a page to define shortcut "sets." There is no link nor is there a tab to the edit shortcuts screen. From my perspective, those kind of fixes are the priority.

Shai

jhodgdon’s picture

Title: Fix "More Help" » Remove "More Help"
Status: Needs work » Needs review
FileSize
2.31 KB

Sorry for the confusion.

Here's a patch that removes all the More Help links.

I guess if we decide a particular admin screen needs a link to the module help page, that is a separate issue.

Bojhan’s picture

@jhodgdon I would think so too.

Shai’s picture

I downloaded the patch @jhodgdon submitted in #42 and applied it to a just-downloaded version of D7 HEAD.

* It applied cleanly.
* Functionality behaving as proposed by @jhodgdon in #34.

Shai

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

In that case, can we mark as RTBC? Shai and I both confirmed the patch works, and I think (?) we are all agreed now that it's a good idea. Speak now...

Shai’s picture

@jhodgdon,

Thanks for the patch, +1 for RTBC.

Folks... please check out the issue/patch I just posted at regarding the big usability problem at the Shortcuts config page (admin/config/system/shortcut).

http://drupal.org/node/681814

Your review, feedback, ideas, are most appreciated.

Shai

jhodgdon’s picture

Shai: your new issue is a duplicate of #681814: Shortcuts Config Page in Desperate Need of Help-text (just marked as such). At least I think so.

yoroy’s picture

Very good. Props to jhodgdon for better design through removing things :)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

BAM. Committed. Thanks, folks!

OK, now how's about a follow-up issue where we try and quickly come to consensus about what pages deserve linking off to their own modules' help page, so we can make the necessary string adjustments before Friday?

jhodgdon’s picture

I think Shai and I kind of agree that there are very few, if any. Module help is an overview, not details that will help someone editing a page.

Status: Fixed » Closed (fixed)

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

andrew.elvis’s picture

I just posted a follow up patch to give us a "more help" link for the Forum module once we go into the configuration screen. http://drupal.org/node/728644#comment-2664678