Posted by marcoka on August 2, 2011 at 8:23pm
9 followers
| Project: | Drupal core |
| Version: | 6.x-dev |
| Component: | menu.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
| Issue tags: | needs backport to D6 |
Issue Summary
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.dewhich is 231 letters long, saved the menu and edited again and copied out the link:
http://www.dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddthat is 128 length, so the string is trimmed and the field is only 128 in length.
Comments
#1
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.
#2
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
#3
sorry wrong patch format
#4
#5
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.
#6
The last submitted patch, 1237290-menu-link_path-maxsize.patch, failed testing.
#7
+++ 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.
#8
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.
#9
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.
#10
#11
The last submitted patch, 1237290-test-only.patch, failed testing.
#12
Fails as expected. Marking RTBC for #8.
#13
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.
#14
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.
#15
Needs backport to D6 as per http://drupal.org/node/829190 and #288946: Increasing path length to 255 chars.
#16
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.