Static menu link override saving double encodes configurations resulting configuration loss

Created on 7 January 2015, about 10 years ago

Updated 22 January 2025, 6 days ago

Problem/Motivation

In \Drupal\Core\Menu\StaticMenuLinkOverrides::saveOverride) we encode the $id but the id is also encoded in StaticMenuLinkOverrides::loadOverride() which means that the configuration is not merged as expected because loadOverride does not find an override.

Proposed resolution

Fix double encoding and sort the config before saving it

Remaining tasks

Add a test to insure the overrides remain sorted

Original report by @alexpott →

In StaticMenuLinkOverrides::save() we encode the $id but the id is also encoded in StaticMenuLinkOverrides::loadOverride() which means that the configuration is not merged as expected because loadOverride does not find an override.

      $id = static::encodeId($id);
      $all_overrides = $this->getConfig()->get('definitions');
      // Combine with any existing data.
      $all_overrides[$id] = $definition + $this->loadOverride($id);
      $this->getConfig()->set('definitions', $all_overrides)->save();

🐛 Bug report

Status

Needs work

Version

11.0 🔥

Component

menu system

Created by

🇬🇧United Kingdom alexpott 🇪🇺🌍

Live updates comments and jobs are added and updated live.

Configuration system

Needs tests
The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

DrupalCampNJ2017

Triaged core major
There is consensus among core maintainers that this is a major issue. Only core committers should add this tag.

Incomplete comments

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Comment 6 days ago →
🇦🇹Austria mvonfrie
It seems that my comment #2996410-14: Review Drupal\Core\Menu\StaticMenuLinkOverrides::saveOverrides for performance improvements → belongs more to this issue which I just found after posting there. Repeating and crosslinking the issues.

When changing an override on the test system and exporting config I observe constant changes in the order of keys per overridden menu item. Especially weight and enabled always change their position between local dev and test environment.

Could this be related to the line

$all_overrides[$id] = $definition + $this->loadOverride($id);

as well?

Furthermore, in my opinion the config should only include what actually can be overridden, aka filter $expected with Drupal\Core\Menu\MenuLinkBase::$overrideAllowed (if applicable).

contrib.social Blog FAQ Discussions

Production build 0.71.5 2024