Refactor menu callbacks and eliminate redundant access checks

The attached patch demonstrates the refactoring approach on statistics.module.
- The _admin() hook is removed.
- Usages of

to print titles have been replaced by proper drupal_set_title() calls.
- Many arg() usages dropped in favor of meaningful parameters.
- Doxygen comments standardized and expanded.
- Some grammatical corrections to help text.
- Broken /statistics page linked from page navigation restored.
- Fixed small bug in menu.inc pertaining to menu callbacks without arguments.

Please comment on and/or commit the patch before I work on more modules. If/when the patch is committed, reset status to "active" to await patches on more modules.

- a comment like 'implementation of the hook_help()' is not useful IMO.
This comment is extremely useful for developers who are not intimately aware of what hooks exist. In addition, it adds a link in Doxygen to the specification of hook_help(), so this saves commenting what the heck the $section parameter does here; instead, you can just follow the link and read up on the hook usage.

- instead of:

case 'admin/logs/access':
case 'admin/logs/access/node':
case 'admin/logs/access/user':
case 'admin/logs/access/host':

use the following:

case strstr($section, 'admin/logs/access'):

Good point. Not my code, though. :-) Actually, now that I look at it, the last three cases will never be true, since the help system is activated only on exact path matches. Should this be extended to search for a registered menu item, as is done for page callbacks?

Similar improvements for story and blog modules.

Also fixes a bug in menu.inc's handling of MENU_FALLTHROUGH for denied-access parent items, made apparent by yesterday's statistics patch.

The #1 patch in this thread introduced the problem with statistics.module; the "comment #5" patch (not yet applied to HEAD) fixes the underlying error in menu.inc. Are you seeing this bad behavior after applying the #5 patch yourself? I can not reproduce it here.

The "create forum topics" issue is that if a user has the "create forum topics" permission, you should also grant them the "create content" permission. My story and blog patches (also part of #5) change the "create" links to use MENU_FALLTHROUGH, which obviates the problem; a future patch will do the same for forum.module. Again, this relies on the menu.inc changes in the #5 patch.

Sorry for thinking you were on crack.

There is an off-by-one error that caused the "create content" menu item not to show up if the user could only create one kind of content. This is not a regression, but a previously-uncaught bug. The attached patch fixes this.

A typo in my bug fix for MENU_FALLTHROUGH caused the bad statistics.module behavior only when certain permissions were set, which is why I was unable to reproduce it.

The attached patch supersedes #5, which has not yet been applied.

Patch for book, forum, page, and poll modules attached.

It is possible to not use fallthroughs, but note that there is a semantic difference between this and using the same handler multiple times.

For an example, take a look at the links made to statistics_top_refer(). In order to pass "internal" as a parameter to the function when accessing admin/logs/referrer/internal, this path is registered as a fallthrough. Without this approach, we either need separate functions for the internal and external logs, or the function needs to use arg() to inspect the path directly. The fallthrough allows us to encapsulate the menu structure entirely inside the _link() hooks, and remove knowledge of the paths from the callbacks.

Another approach would be to use a variation of http://drupal.org/node/view/5219 to pass an argument in explicitly.

I'm quite willing to investigate other approaches; there are only a few instances of MENU_FALLTHROUGH being used, and they are obviously easy to find and change later if desired. I still feel at this point that *not* re-registering the callback is most transparent; these links are really "shortcuts" to an already-defined page, not new pages themselves.

Patch for admin, aggregator, archive, and block modules.

Patch for blogapi and comment modules. The comment module will be done in multiple phases; it is a large and complex beast, and I want to be very careful (same for node.module when I get to that).

Yes, it does... the $nid parameter is passed in from the URL as part of the menu refactoring instead of using arg(). Is this causing an error somewhere? The next patch (after the blogapi one goes in) will add a default value of 0 for this parameter which should make it safe for backwards compatibility.

Patch for drupal, filter, and help modules.

This patch also changes some punctuation from comments in earlier patches to better accommodate Doxygen's indexing, and adds a default to book_admin() as mentioned above.

Patch for menu, path, and ping modules.

More of the same.

Three more. (This patch is independent of #25; they can be applied in either order.)

User and watchdog modules.

Regression fix due to interaction between admin.module and watchdog.module. Reported by Morbus.

Unapplied chunks of the patch, cleaned up and synced to current HEAD. Should be able to commit it now that anonymous comment support has landed.

Work on this is completed for now (at least until other menu work is concluded). Further work to be done in new issues.