First i discovered http://drupal.org/node/288946, and there is beeing said that that patch is in d7, i mean that the menu path could be 255 in length.

so i tested that and input:

http://www.ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd.de

which is 231 letters long, saved the menu and edited again and copied out the link:

http://www.ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd

that is 128 length, so the string is trimmed and the field is only 128 in length.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Status: Active » Closed (cannot reproduce)

I tested this and it works for me. I copied your original string of 'd's and pasted it into the URL path settings form. It is saved to the database—which does say that the value type is VARCHAR(255)—and the node is also is accessible from the path.

Please re-open if this continues to be an issue.

marcoka’s picture

FileSize
642 bytes

i asked another person to test this in d 7.7
as a matter of fact the maxlength of the path field in the menu is 128, so the field itself will not accept more than 128 chars

Screenshot: http://screensnapr.com/v/PGGRUm.png

So i checked the menu.admin.inc form generation:

menu_edit_item($form, &$form_state, $type, $item, $menu)

There is no maxlength defined, so somehow the std. of 128 is used. So if you add maxlength 255 it works

After applying patch: http://screensnapr.com/v/TGREeO.png

marcoka’s picture

sorry wrong patch format

marcoka’s picture

Status: Closed (cannot reproduce) » Needs review
dawehner’s picture

Version: 7.7 » 8.x-dev
Issue tags: +Needs backport to D7
FileSize
1.86 KB

This patch looks perfect!

Let it first be done in d8 and then backported.

@e-anima: Does this needs backport to d6 as well?

Here is a new patch which has a test for this, didn't runned the test yet, let's see.

Status: Needs review » Needs work

The last submitted patch, 1237290-menu-link_path-maxsize.patch, failed testing.

xjm’s picture

Status: Needs review » Needs work
+++ b/modules/menu/menu.testundefined
@@ -329,6 +330,20 @@ class MenuTestCase extends DrupalWebTestCase {
+   * Attemt to add a menu path with more then 255(the maximum length).

Should be: "Attempts to add a menu path with more than 255 characters (the maximum length)."

+++ b/modules/menu/menu.testundefined
@@ -329,6 +330,20 @@ class MenuTestCase extends DrupalWebTestCase {
+    $this->assertRaw(t('!name cannot be longer than %max characters but is currently %length characters long.', array('!name' => t('Path'), '%max' => 255, '%lenght' => 300)), 'Take sure that you cannot input too long menu link paths.');

Typo in '%lenght' so I think the test will fail for the wrong reason. Also, I'd maybe change the assertion text to "A menu path over 255 characters is rejected." for clarity.

dawehner’s picture

Status: Needs work » Needs review
FileSize
1.86 KB
Should be: "Attempts to add a menu path with more than 255 characters (the maximum length."

Next time i will c&p again :)

A menu path over 255 characters is rejected.

Oh this so much easier, simpler and better to understand. thanks!

Let's see what the testbot says.

xjm’s picture

Status: Needs work » Needs review
FileSize
1.15 KB

Let's try dereine's test by itself to demonstrate it fails without the fix. (Fails correctly when I test locally.) If testbot comes back with a fail here then I think this is RTBC.

xjm’s picture

Component: path.module » menu.module

Status: Needs review » Needs work

The last submitted patch, 1237290-test-only.patch, failed testing.

xjm’s picture

Status: Needs work » Reviewed & tested by the community

Fails as expected. Marking RTBC for #8.

Dries’s picture

I don't think we need a test for this in the menu system. We're just leveraging the form API so this should be covered by form API tests.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

So I committed the patch in #8 to 7.x and 8.x. I committed a modified patch without the test per my own suggestion in #13.

I'm going to mark this 'fixed' but if you think we actually need the test, I'd be happy to discuss that more. Just re-open the issue. I'm just trying to keep unnecessary tests out of core.

Thanks for the patches.

colan’s picture

Title: Menu path length » Menu path length is limited by the UI to 128 characters
Version: 8.x-dev » 6.x-dev
Status: Fixed » Active
Issue tags: -Needs backport to D7 +Needs backport to D6
j0rd’s picture

Just ran into this problem on a D6 site. I simply added #maxlength to link_path to get me passed the UI defaulting to 128 maxlength.

I've applied patch #45 and it resolved my problem.

From a review of the code it seems like sane changes (looks like he got most of the places to change), and the database update worked as expected.

Would like to see this committed.

Status: Active » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.