Introduced with #2670120: Patterns with node:menu-link tokens don't work on first node save, if a menu link has been added as a link to a node and subsequently disabled through the menu UI it will be enabled again when the node is saved again through the UI.

The offending code is in token_node_menu_link_submit() line 731, hard sets the enabled flag to 1.

This is because the function checks:

$values = $form_state->getValue('menu');
if (!empty($values['enabled']) && trim($values['title']))

To determine that the menu link should be worked on. Technically $values['enabled'] should denote whether the menu link is disabled or not but it is actually used to control the display of the menu link editing interface on the node form. I.e it is controlled by the checkbox "Provide a menu link".

A node can provide a menu link but it can be disabled.

The menu_ui gets around overwriting the enabled flag like this because it only sets the enabled flag for new menu links (see _menu_ui_node_save())

CommentFileSizeAuthor
#8 menu_link_enabled-2780423-8.patch1.97 KBBambell
#4 menu_link_enabled-2780423-4.patch1.8 KBBambell
#2 2780423-fix-disabled-menu-overwrite.patch537 bytesacbramley
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

acbramley created an issue. See original summary.

acbramley’s picture

acbramley CreditAttribution: acbramley at Department of Justice & Community Safety, Victoria commented
Status: Active » Needs review
FileSize
2780423-fix-disabled-menu-overwrite.patch537 bytes

Quickfix for now.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Issue tags: +Needs tests

enabled actually has a default value of TRUE, so we should be able to just remove this?

Bambell’s picture

Bambell CreditAttribution: Bambell at MD Systems GmbH commented
FileSize
menu_link_enabled-2780423-4.patch1.8 KB

Failing test to expose the bug. Incoming patch...

Status: Needs review » Needs work

The last submitted patch, 4: menu_link_enabled-2780423-4.patch, failed testing.

The last submitted patch, 4: menu_link_enabled-2780423-4.patch, failed testing.

Bambell’s picture

Bambell CreditAttribution: Bambell at MD Systems GmbH commented
Status: Needs work » Needs review

Actually, I think that should be done in core as well. Here's how enabled's default value is set :

'#default_value' => (int) (bool) $defaults['id']

So checked if a menu link exists. It seems to me like we can easily know if it's actually enabled or not.

enabled actually has a default value of TRUE, so we should be able to just remove this?

If we remove it completely, then the menu link is disabled each time the node is edited, if the user doesn't explicitly enable it?

Bambell’s picture

Bambell CreditAttribution: Bambell at MD Systems GmbH commented
FileSize
menu_link_enabled-2780423-8.patch1.97 KB

Here we go.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Thanks, looks good, will commit this soon. Giving @acbramley a chance to verify that this fixes the problem for him as well.

acbramley’s picture

acbramley CreditAttribution: acbramley at Department of Justice & Community Safety, Victoria commented

Confirming it works for me, thanks guys

  • Berdir committed 1c68e82 on 8.x-1.x authored by Bambell 
    Issue #2780423 by Bambell, acbramley: Menu link enabled status...
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Reviewed & tested by the community » Fixed

Thanks for confirming, committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.