Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I just tried to save a new menu link using an alias. I got the following message:
The menu system stores system paths only, but will use the URL alias for display. foooo has been stored as node/1
The menu link has been saved.
Unfortunately, contrary to the displayed message, the menu link was not saved.
Comment | File | Size | Author |
---|---|---|---|
#16 | 2063191-before.png | 34 KB | manumilou |
#16 | 2063191-after.png | 32.97 KB | manumilou |
#14 | menu-path-2063191.14.interdiff.txt | 539 bytes | larowlan |
#14 | menu-path-2063191.14.patch | 4.62 KB | larowlan |
#13 | menu-path-2063191.13.interdiff.txt | 2.42 KB | larowlan |
Comments
Comment #1
larowlanAdding tag
Comment #2
larowlanDefinitely no test coverage
Comment #3
marthinal CreditAttribution: marthinal commentedHi guys,
The problem is when submitting the form, we generate an entity using the form values. Maybe we should alter the form_state value?
Cheers.
Comment #4
marthinal CreditAttribution: marthinal commentedIf we have query parameters the patch doesn't work because we are adding values to the $menu_link object into the validation.
Comment #5
larowlanWe still need tests here
Comment #6
marthinal CreditAttribution: marthinal commentedI think the place to detect this is from MenuTest.php where we add/remove menu links. I was trying today on a break, but I need more time.
When adding the new path with a query, the path is added complete to the "link_path" and to reproduce the same behavior as D7 does, the query should be added to "options" as I can verify.
So I change the status to needs work.
Comment #7
andymartha CreditAttribution: andymartha commentedAfter applying the patch 2063191-menulinks.patch by marthinal in #3 to a Drupal 8 fresh install 8/17, adding a menu link from an alias, which was previously not saving, is now saving to the menu.
Comment #8
larowlanadding tests
Comment #9
larowlanAdds tests.
Should be red/green
Comment #10
marthinal CreditAttribution: marthinal commented@larowlan Seems to work as expected with a query using this patch.
Thanks for the test. I was starting the test when I saw your test :)
Comment #12
larowlanWorking on passing patch
Comment #13
larowlanFixes test fail at #9
Interdiff is against 9.
Comment #14
larowlanLess fragile test, doesn't rely on node/5
Comment #15
larowlanSo @marthinal are you saying we need to handle links to aliases with parameters as well? eg foobar?bar=baz which is an alias to node/1?bar=baz ?
Comment #16
manumilou CreditAttribution: manumilou commentedI was able to reproduce the bug. Patch #14 fixed it. See screenshots attached.
Comment #17
SheilaSmith CreditAttribution: SheilaSmith commentedgot the same issue,I just tried and it helped. thanks :)
Comment #18
thedavidmeister CreditAttribution: thedavidmeister commentedwe have 5 reports that this works with screenshots for 2, test coverage, I don't see any coding standards issues, this looks RTBC then.
Comment #19
catchCommitted/pushed to 8.x, thanks!
Comment #21
SweetTomato CreditAttribution: SweetTomato commentedI'm getting this in Drupal 7. Will there be a patch for 7?
Comment #22
matt.rad CreditAttribution: matt.rad commentedAlso having this problem in D7. I would appreciate a patch too.
Comment #23
cilefen CreditAttribution: cilefen commented@matt.rad: See https://www.drupal.org/node/2715157
Comment #24
harshadananjaya CreditAttribution: harshadananjaya commentedPlease check your linked content page is published or not?