If you create a node and check the "Provide a menu link" checkbox but leave the menu link title field blank, then when you save the node no menu link is created - and no error message is printed either.
Although this field is usually auto-filled via JavaScript based on the node title, there are situations where that won't happen (for example, if the node title field is being hidden on the form and generated automatically).
Two possible ways to fix this:
- Make the menu link title field conditionally required whenever the "Provide a menu link" checkbox is checked.
- If an empty menu link title field is submitted, autogenerate it based on the node title.
The first fix seems like it wouldn't be backportable. So, I wrote patches for the second fix instead.
Comment | File | Size | Author |
---|---|---|---|
#45 | 1763002-after-patch.mov | 8.83 MB | nayana_mvr |
#45 | 1763002-before-patch.mov | 7.94 MB | nayana_mvr |
#44 | 1763002-44.patch | 1.59 KB | Lendude |
#44 | interdiff-1763002-37-44.txt | 669 bytes | Lendude |
#39 | After Patch 1763002.png | 341.43 KB | chetanbharambe |
Comments
Comment #1
David_Rothstein CreditAttribution: David_Rothstein commentedHere are patches for Drupal 7 and 8.
Comment #3
vaibhavjainAn updated patch for Drupal 8
Comment #5
marthinal CreditAttribution: marthinal commentedHere another option.We are inside a form alter so we could add a little validation.I think is better than write something (title) automatically by default because maybe the user in this case has no reference whats going on exactly.
Comment #7
marthinal CreditAttribution: marthinal commentedAt localhost it works so I try again.
Comment #8
dcam CreditAttribution: dcam commentedThis needed a test. There was already a test case for a blank menu link title, so I added a couple of asserts to it. They check that we're redirected to the node edit page and the validation error message appears.
This was my first attempt at adding to a test, so constructive criticism is welcome.
Comment #9
dcam CreditAttribution: dcam commented#8: empty-menu-link-title-1763002-8.patch queued for re-testing.
Comment #11
dcam CreditAttribution: dcam commented#8: empty-menu-link-title-1763002-8-tests-only.patch queued for re-testing.
Comment #12
dcam CreditAttribution: dcam commentedRerolled #8.
Comment #13
andymartha CreditAttribution: andymartha commentedOn a fresh installation of Drupal 8.x-dev on March 8, 2013, I can confirm that the problem of no menu link is created - and no error message is printed either after following the steps in summary. See screenshot.
After applying the patch empty-menu-link-title-1763002-12.patch in #12 by dcam, I can confirm that the patch worked by the form not submitting but showing the proposed error messages nicely. See screenshot.
Comment #14
David_Rothstein CreditAttribution: David_Rothstein commentedI think if we're going with option #1 from the issue summary it should be conditionally required in the standard way (i.e., it should get the red asterisk which all other required fields get, and the error message should be worded the same way too)?
I still prefer option #2 personally... This only happens under rare circumstances, and while I guess it is a bit of magic, the user already clicked on a checkbox saying they wanted a menu link so we can just give them a sensible one based on the form they submitted. If they really don't like the title, they can go back and edit it.
Comment #17
jhedstromComment #18
idebr CreditAttribution: idebr commentedI agree with David_Rothstein falling back to the node title makes more sense than conditionally validating the menu title for the same reason the menu link is prefilled with the node title.
If a menu link is a required field, it should have the #required attribute. The problem with this approach is that the States API can not yet conditionally validate required fields.
I created a new patch based on the #2 proposed resolution.
Comment #19
jhedstromSeems like adding a quick test or assertion to
MenuNodeTest
here would be worthwhile.Comment #20
idebr CreditAttribution: idebr commentedThere's a first, fixed the assertion by removing lines :)
Comment #26
aerozeppelin CreditAttribution: aerozeppelin at California State University San Bernardino commentedRerolling patch #20.
Comment #27
aerozeppelin CreditAttribution: aerozeppelin at California State University San Bernardino commentedComment #29
wturrell CreditAttribution: wturrell as a volunteer commentedPartial review (needs someone else to RTBC)
- Reproduced original problem (8.2.x)
- Patch fixes it
- No UI changes
- coding style fine
- All seems to be in scope
- Tests passed and test modified for functional change
The comment above the altered test was out of date, I've updated it.
Comment #36
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedPatch #29 can't be applied.Needs reroll.
Comment #37
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedRerolled patch #29 .Please check
Comment #38
Lan CreditAttribution: Lan commented@Abhijith S the patch seems not working for D9
Comment #39
chetanbharambe CreditAttribution: chetanbharambe at QED42 for Drupal India Association commentedHi @Abhijith S,
Verified and tested patch #37.
Patch applied successfully and not working as expected
Testing Steps:
# Goto: Appearance -> Apply Seven theme
# Create any basic page
# Click on Menu setting and check on Provide a menu link
# Save the node
# Check the result
Expected Results:
# Check the Two possible ways to fix this which are mentioned in the issue summary and it should be working as per requirement.
Actual Results:
# Currently, No message is appearing before and after applying the patch
Please refer attached screenshots for the same.
Not working as expected.
Can be a move to Needs Work.
Comment #44
LendudeThis came up as a bug smash triage target.
Rerolled.
Fixed the if/else to actually do what it is supposed to do, if the trimmed version of the provided title is empty -> set the title to the node title, in previous versions the trim did nothing.
We don't need to do the actual trimming since that is already handled by
_menu_ui_node_save
, so we only need to trim to check we actually end up with a valid title in_menu_ui_node_save
.Comment #45
nayana_mvr CreditAttribution: nayana_mvr at Srijan | A Material+ Company commentedVerified the patch #44 and tested it on Drupal version 9.5.x. The patch works fine and I have added the before and after screen recordings for reference.
Steps followed:-
After applying the patch, a menu link corresponding to the new node is autogenerated in the main navigation menu based on the node title even though the menu link title field was empty while creating the node.
Comment #46
smustgrave CreditAttribution: smustgrave at Mobomo commentedConfirmed the issue on D10.1 by manually deleting the title field for the menu link.
Applied the patch and did the same steps and the menu link was created.
Comment #48
LendudeUnrelated fail
Comment #50
catchOption #2 seems good. Double checked the test coverage to make sure we actually have test coverage for removing a link, and that's fine.
I doubt there are lots of people using the bug to remove menu links as opposed to unchecking the checkbox, but going to just commit this to 10.1.x in case.
Committed 586900e and pushed to 10.1.x. Thanks!
Comment #52
stefan.kornThis has been a bug so long, it has gotten a feature ... ;-)
In a project we rely on the old behaviour to realize a special workflow regarding creation of menu links while node editing. Now with the new behavior we have a bc break going from D9 to D10. Yes, I know this can happen going from D9 to D10.
I am also wondering if there might not be people that just clear the title to remove a menu link and then wonder why they get the menu link back even with different text if they changed the menu title before. And additionaly if you for example edit nodes from the content overview you will not get directed to the node view and might even not realize that this has happened.
I am not clearly seeing the bug in the old behavior and it was there for a long time ... But surely I have been 10 years to late for lamenting on this, but I really thought this behavior was intended the way it was.
Comment #53
stefan.kornI mean this is not straight forward:
You save a menu link with different title than the node title.
You come back later and remove the menu title and save the node.
You still have the menu link, but now with the node title again.