Menus are not rendered when machine name starting with hyphen

I can see that this problem is happening on Drupal 9.5.x, updating the IS with the steps to reproduce.

First try at this issue following the Proposed resolution.
Please review.

CS fixes

Cspell fix

Applied patch #11 successfully. Now, will get error when menu starts with special characters

Adding screenshots

Thanks @ranjith_kumar_k_u for fixing the problems :D

@Asha Nair, did you tested it and did a code review?
Can you change this to RTBC if it is all right?

Hi, after applying @ranjith_kumar_k_u #11 patch, I did the code review of the issue and manually tested the following charachters with the beginning of the title:

~ ,
` ,
! ,
@ ,
# ,
$ ,
% ,
^ ,
& ,
* ,
( ,
) ,
_ ,
- ,
+ ,
= ,
{ ,
[ ,
} ,
] ,
| ,
\ ,
: ,
; ,
" ,
' ,
< ,
, ,
> ,
. ,
? ,
/ .

All of them led to the Error message: The title can not begin with special characters.

For this reason, I'm changing the status of the issue to RTBC.

Menus are entities, so I think this should be handled via an entity validation constraint instead of in the form.

Hi!

I will try to work on that.
thanks!

Hello @catch, Menu is a config entity as we can see in Drupal\system\Entity\Menu.
Should we really use entity validation constraint for it? If yes, do you have any example how to implement it for config entities? All examples I see in core are used for content entities only.
I have also noticed that other config entities use the validateForm() to check the field values like in Drupal\node\NodeTypeForm

  /**
   * {@inheritdoc}
   */
  public function validateForm(array &$form, FormStateInterface $form_state) {
    parent::validateForm($form, $form_state);

    $id = trim($form_state->getValue('type'));
    // '0' is invalid, since elsewhere we check it using empty().
    if ($id == '0') {
      $form_state->setErrorByName('type', $this->t("Invalid machine-readable name. Enter a name other than %invalid.", ['%invalid' => $id]));
    }
  }

Hi!
Sorry, I really try to work on this validation, but I couldn't make it run. So I'm unassigned here.

I have updated the patch based on the suggestion from @catch in #16.

I applied #20 patch against 9.5.x-dev. Patch applied cleanly but I'm not able add menu link with special character after applying patch.
Attaching screenshots for reference.
Thanks

Edit: Apologies for not posting my review clearly. I meant that the patch I tested preventing the menus which are having special characters. So patch working as expected.

Thanks for the patch @paulocs

But that's the idea of the issue, isn't it?

The patch applied successfully and works as required.
Moving this ticket to RTBC.
Thanks!

Almost there, a few more things to do.

I've only skimmed the comments but I see that the code has not been reviewed. Setting to NW.

Since this is changing validation of user input there should be before and after screenshots in the issue summary.

Uploading screenshots and updating IS.

Don't see a tests-only patch if we can get that please.

Code looks good!

For ../core/modules/menu_ui/tests/src/Functional/MenuUiTest::testMenuName, maybe we don't need to assign $page in this line?
$page = $this->getSession()->getPage();

This is not used afterwards.

Adding patch #29 and interdiff for the consideration above in case it makes sense.

Patch #29 Tested and Applied successfully on Drupal 9.5.x.

Before Patch:

After Patch:

#29 Patch is working fine and shared the screenshot for the reference.

#29 looks like working and green.
need to reviewed by community.

I think it was a random fail of ckeditor5.

Thanks for the reviews @Manibharathi E R, @Charchil Khandelwal and @heni_deepak.

Patch works, but we still need a core review as @quietone said in #24:

I've only skimmed the comments but I see that the code has not been reviewed. Setting to NW.

Yes, @lucassc the error coming form based class trait TestFileCreationTrait { }
and mean code is $this->getTestFiles('image')[0]->uri

that function need to update as per parent function.

I found this #3231694: TestFileCreationTrait::getTestFiles() has incorrect @return type

Looks like #3231694: TestFileCreationTrait::getTestFiles() has incorrect @return type fixed and #29 patch also works.

This is related to #2920678: Add config validation for the allowed characters of machine names and #2869792: [meta] Add constraints to all config entity types

I feel that we should be able to call validate on config entities much the same way we can on content entities - that way we wouldn't need to have so much in the MenuForm class.

+++ b/core/modules/menu_ui/src/MenuForm.php
@@ -73,12 +74,15 @@ class MenuForm extends EntityForm {
-  public function __construct(MenuLinkManagerInterface $menu_link_manager, MenuLinkTreeInterface $menu_tree, LinkGeneratorInterface $link_generator, MenuLinkContentStorageInterface $menu_link_content_storage) {
+  public function __construct(MenuLinkManagerInterface $menu_link_manager, MenuLinkTreeInterface $menu_tree, LinkGeneratorInterface $link_generator, MenuLinkContentStorageInterface $menu_link_content_storage, TypedConfigManagerInterface $typed_config_manager) {
     $this->menuLinkManager = $menu_link_manager;
     $this->menuTree = $menu_tree;
     $this->linkGenerator = $link_generator;
     $this->menuLinkContentStorage = $menu_link_content_storage;
+    $this->typedConfigManager = $typed_config_manager;
   }

If we do do this we'll need to make the argument optional and issue a deprecation.

I am not sure about this. I would guess it is possible with current code to fix the suggestions in an appropriate alter hook and so maybe people can have menus which will break on import after this? Is there a reason beyond the theme suggestion to disallow them? If not, then maybe fix that code?

I tested this on 11.x, fresh standard install, today and was not able to reproduce the problem using the steps in the Issue Summary. Therefor closing as outdated.

If you are experiencing this problem on a supported version of Drupal reopen the issue, by setting the status to 'Active', and provide complete steps to reproduce the issue (starting from "Install Drupal core").

Thanks!