Currently Drupal is rather schizophrenic when it comes to handling user-specified paths. We don't require the user to type in paths often, which is good, but there are a few situations where this is unavoidable. Examples are menu administration, settings for 403/404 pages, the front page selection, and block path regular expressions. In these locations, there is no clear policy for whether the user is expected to enter an aliased or real Drupal path, and we certainly don't tell the user in the help text. We should decide on a policy, document it in the code and/or user help, and enforce it in the modules.

The possible policies I can imagine are:
1) Require the user to enter a real Drupal path.
2) Require the user to enter an aliased path.
3) Allow the user to enter either, but always store a real Drupal path.
4) Allow the user to enter either, but always store an aliased path.
5) Allow the user to enter either, store what the user entered, and convert at read time.

Now 1 and 3 have the advantage that if the alias for a page changes, links to that page in the various settings pages do not break. 2 and 4, on the other hand, allow the administrator to affect what page a setting refers to by simply changing a path alias, which could be construed as good behavior. 3 and 4 have the downside of possibly presenting the admin with different information than she entered when she goes back to edit it later. 5 is the most permissive, but is the hardest policy to enforce because the value could be read in many different places in the code. Also, with 5 the behavior is like 1 and 3 if the user enters a real path or 2 and 4 if the user enters an alias. This is good because it allows the administrator to have the behavior she wants, but bad because it is inconsistent.

What policy should we go with? I'm leaning toward either 3 or 5. Once we decide, I will:
- Add documentation about the decision in the code
- Change existing usage to match the decision
- Update user documentation on the administration pages (probably only necessary with approach 1 or 2)

CommentFileSizeAuthor
#12 menu.inc_4.diff859 bytesdrumm
#3 menu_17.patch781 bytesJonBob
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JonBob’s picture

Another note: policies 3 and 4 will not work with block paths, as we cannot reliably convert the regex to the appropriate form. With these, we must perform any necessary conversion on the path that is being matched against the regex instead of on the regex itself.

JonBob’s picture

The issue Xtemplate mission does not work with an alias is really a part of this one.

JonBob’s picture

FileSize
781 bytes

Here's a patch to bring the menu system in line with #5, which seems to be the user expectation.

Any advice on where to document the appropriate behavior for modules? Perhaps the PHPdoc for drupal_get_path_alias()?

JonBob’s picture

The patch here has been applied, but I still need to know where to add documentation. Anyone? Anyone? Bueller?

wulff’s picture

The current documentation documents policy #1.

Since the patch has been applied, should this be moved to the Documentation project?

killes@www.drop.org’s picture

Project: Drupal core » Documentation
Component: base system » Developer Guide
Priority: Normal » Critical

Moving as suggested and setting to critical. The menu system isn't understood by everyone and we sure need up to date docs on it.

drumm’s picture

Project: Documentation » Drupal core
Component: Developer Guide » menu system
Assigned: JonBob » drumm
Priority: Critical » Normal
Status: Active » Needs review

I think the patch which was applied should be reverted. It is the best solution to the bug described at http://lists.drupal.org/pipermail/development/2006-January/012956.html.

Menu item paths are always the internal path since they are the actual menu item. They may later have aliases made of them, like all menu items.

Richard Archer’s picture

I can't reproduce this bug. Could you please give more explicit instructions? Sorry...

Also, do you have any contrib modules installed which could be interfering with the menu system? I'm thinking of i18n in particular... it contains a nasty, nasty hack that screws really badly with paths. There is a bug posted against i18n that discusses this issue.

drumm’s picture

I just confirmed on a fresh checkout.

Enable the path module and create an alias with an editable menu item, such as 'blog' or 'forum' (be sure the relevant module is enabled), as the alias (not the system path, you are aliasing over an existing path).

One duplicate item will be added to the menu table every time you load the menu administration page. Look at it at the DB prompt, the extra items only show up in the UI under certain conditions since they are corrupt data.

Richard Archer’s picture

How about only calling drupal_get_normal_path if the path isn't known to the menu system already? So:

      if (!isset($_menu['path index'][$item->path])) {
        $item->path = drupal_get_normal_path($item->path);
      }

On the other hand, what are the wider implications of removing the drupal_get_normal_path call completely? JonBob's original post implies that without this call the user must enter real Drupal paths into the menu system. But it seems that aliases work just fine.

Perhaps there has been some work done elsewhere in the aliases/URL code that make this call obsolete?

dreed47’s picture

I've been experiencing this problem and doing this check before calling drupal_get_normal_path seems to have fixed this for me. I don't know if there are any other side affects of this but it seems to be working fine for me.

drumm’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
859 bytes

I tested Richard's approach and it works. That was one of my initial ideas and I think it probably is a bit safer. I can't think of any side effects.

Here is a patch. Setting to ready to commit since it effectively has two reviews.

Dries’s picture

Worth documenting or not?

drumm’s picture

Sure, we can revert this back to the previous status. For menu items this approach effectively means that both aliases and system paths work; and system paths should be translated on output since I expect they are passed through l().

Dries’s picture

Does that mean there will be a new patch, or not?

Richard Archer’s picture

The patch is good, I don't intend to roll another one.

The patch is already documented in the code.

As for documentation in the handbook... I don't think documentation is required since this issue just adds functionality you would expect to be present (i.e. being able to place a drupal path or an alias in the menu).

Richard Archer’s picture

This patch also fixes a problem with the i18n module. http://drupal.org/node/40879

This issue was initially raised against the menu system but I moved it to i18n. Looks like it belongs here after all, even if I don't like what i18n does with paths :)

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)