Issue #2084197 by tim.plunkett: Fixed Uninstalling menu module removes all custom menus.

core/modules/block/block.api.php

@@ -48,9 +48,9 @@ function hook_block_view_alter(array &$build, \Drupal\block\BlockPluginInterface
  *
  * In this hook name, BASE_BLOCK_ID refers to the block implementation's plugin
  * id, regardless of whether the plugin supports derivatives. For example, for
- * the \Drupal\system\Plugin\block\block\SystemPoweredByBlock block, this would
- * be 'system_powered_by_block' as per that class's annotation. And for the
- * \Drupal\system\Plugin\block\block\SystemMenuBlock block, it would be
+ * the \Drupal\system\Plugin\Block\SystemPoweredByBlock block, this would be
+ * 'system_powered_by_block' as per that class's annotation. And for the
+ * \Drupal\system\Plugin\Block\SystemMenuBlock block, it would be
  * 'system_menu_block' as per that class's annotation, regardless of which menu
  * the derived block is for.
  *

core/modules/block/block.module

@@ -589,7 +589,7 @@ function block_user_role_delete($role) {
  */
 function block_menu_delete($menu) {
   foreach (entity_load_multiple('block') as $block) {
-    if ($block->get('plugin') == 'menu_menu_block:' . $menu->id()) {
+    if ($block->get('plugin') == 'system_menu_block:' . $menu->id()) {
       $block->delete();
     }
   }

core/modules/block/lib/Drupal/block/Tests/BlockHtmlIdTest.php

@@ -39,7 +39,7 @@ function setUp() {
     \Drupal::state()->set('block_test.content', $current_content);
 
     // Enable our test blocks.
-    $this->drupalPlaceBlock('system_menu_block:menu-tools');
+    $this->drupalPlaceBlock('system_menu_block:tools');
     $this->drupalPlaceBlock('test_html_id', array('machine_name' => 'test_id_block'));
   }
 

core/modules/block/lib/Drupal/block/Tests/BlockTemplateSuggestionsUnitTest.php

@@ -38,7 +38,7 @@ function testBlockThemeHookSuggestions() {
     // and generates possibilities for each level of derivative.
     // @todo Clarify this comment.
     $block = entity_create('block', array(
-      'plugin' => 'system_menu_block:menu-admin',
+      'plugin' => 'system_menu_block:admin',
       'region' => 'footer',
       'id' => \Drupal::config('system.theme')->get('default') . '.machinename',
     ));
@@ -51,7 +51,7 @@ function testBlockThemeHookSuggestions() {
     // Test adding a class to the block content.
     $variables['content_attributes']['class'][] = 'test-class';
     template_preprocess_block($variables);
-    $this->assertEqual($variables['theme_hook_suggestions'], array('block__system', 'block__system_menu_block', 'block__system_menu_block__menu_admin', 'block__machinename'));
+    $this->assertEqual($variables['theme_hook_suggestions'], array('block__system', 'block__system_menu_block', 'block__system_menu_block__admin', 'block__machinename'));
     $this->assertEqual($variables['content_attributes']['class'], array('test-class', 'content'), 'Default .content class added to block content_attributes');
   }
 

core/modules/block/lib/Drupal/block/Tests/BlockUiTest.php

@@ -50,7 +50,7 @@ function setUp() {
       array(
         'label' => 'Tools',
         'tr' => '5',
-        'plugin_id' => 'system_menu_block:menu-tools',
+        'plugin_id' => 'system_menu_block:tools',
         'settings' => array('region' => 'sidebar_second', 'machine_name' => 'tools'),
         'test_weight' => '-1',
       ),

core/modules/menu/lib/Drupal/menu/Form/MenuDeleteForm.php

@@ -94,9 +94,8 @@ public function getConfirmText() {
   public function submit(array $form, array &$form_state) {
     $form_state['redirect'] = 'admin/structure/menu';
 
-    // System-defined menus may not be deleted - only menus defined by this module.
-    $system_menus = menu_list_system_menus();
-    if (isset($system_menus[$this->entity->id()])) {
+    // Locked menus may not be deleted.
+    if ($this->entity->isLocked()) {
       return;
     }
 

core/modules/menu/lib/Drupal/menu/MenuFormController.php

@@ -73,8 +73,6 @@ public function form(array $form, array &$form_state) {
       drupal_set_title(t('Edit menu %label', array('%label' => $menu->label())), PASS_THROUGH);
     }
 
-    $system_menus = menu_list_system_menus();
-
     $form['label'] = array(
       '#type' => 'textfield',
       '#title' => t('Title'),
@@ -87,7 +85,6 @@ public function form(array $form, array &$form_state) {
       '#default_value' => $menu->id(),
       '#maxlength' => MENU_MAX_MENU_NAME_LENGTH_UI,
       '#description' => t('A unique name to construct the URL for the menu. It must only contain lowercase letters, numbers and hyphens.'),
-      '#field_prefix' => $menu->isNew() ? 'menu-' : '',
       '#machine_name' => array(
         'exists' => array($this, 'menuNameExists'),
         'source' => array('label'),
@@ -95,7 +92,7 @@ public function form(array $form, array &$form_state) {
         'replace' => '-',
       ),
       // A menu's machine name cannot be changed.
-      '#disabled' => !$menu->isNew() || isset($system_menus[$menu->id()]),
+      '#disabled' => !$menu->isNew() || $menu->isLocked(),
     );
     $form['description'] = array(
       '#type' => 'textfield',
@@ -128,7 +125,7 @@ public function form(array $form, array &$form_state) {
     }
 
     // Add menu links administration form for existing menus.
-    if (!$menu->isNew() || isset($system_menus[$menu->id()])) {
+    if (!$menu->isNew() || $menu->isLocked()) {
       // Form API supports constructing and validating self-contained sections
       // within forms, but does not allow to handle the form section's submission
       // equally separated yet. Therefore, we use a $form_state key to point to
@@ -157,9 +154,8 @@ public function menuNameExists($value) {
       return TRUE;
     }
 
-    // Check for a link assigned to this menu. 'menu-' is added to the menu name
-    // to avoid name-space conflicts.
-    return $this->entityQueryFactory->get('menu_link')->condition('menu_name', 'menu-' . $value)->range(0, 1)->count()->execute();
+    // Check for a link assigned to this menu.
+    return $this->entityQueryFactory->get('menu_link')->condition('menu_name', $value)->range(0, 1)->count()->execute();
   }
 
   /**
@@ -185,17 +181,6 @@ protected function actions(array $form, array &$form_state) {
     return $actions;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function validate(array $form, array &$form_state) {
-    if ($this->entity->isNew()) {
-      // The machine name is validated automatically, we only need to add the
-      // 'menu-' prefix here.
-      $form_state['values']['id'] = 'menu-' . $form_state['values']['id'];
-    }
-  }
-
   /**
    * Submit handler to update the bundle for the default language configuration.
    */
@@ -214,11 +199,7 @@ public function languageConfigurationSubmit(array &$form, array &$form_state) {
    */
   public function save(array $form, array &$form_state) {
     $menu = $this->entity;
-    // @todo Get rid of menu_list_system_menus() https://drupal.org/node/1882552
-    //   Supposed menu item declared by hook_menu()
-    //   Should be moved to submitOverviewForm()
-    $system_menus = menu_list_system_menus();
-    if (!$menu->isNew() || isset($system_menus[$menu->id()])) {
+    if (!$menu->isNew() || $menu->isLocked()) {
       $this->submitOverviewForm($form, $form_state);
     }
 

core/modules/menu/lib/Drupal/menu/Plugin/Block/MenuBlock.php

-<?php
-
-/**
- * @file
- * Contains \Drupal\menu\Plugin\Block\MenuBlock.
- */
-
-namespace Drupal\menu\Plugin\Block;
-
-use Drupal\system\Plugin\Block\SystemMenuBlock;
-use Drupal\block\Annotation\Block;
-use Drupal\Core\Annotation\Translation;
-
-/**
- * Provides a generic Menu block.
- *
- * @Block(
- *   id = "menu_menu_block",
- *   admin_label = @Translation("Menu"),
- *   derivative = "Drupal\menu\Plugin\Derivative\MenuBlock"
- * )
- */
-class MenuBlock extends SystemMenuBlock {
-
-  /**
-   * {@inheritdoc}
-   */
-  public function build() {
-    list($plugin, $menu) = explode(':', $this->getPluginId());
-    return menu_tree($menu);
-  }
-
-}

core/modules/menu/lib/Drupal/menu/Plugin/Derivative/MenuBlock.php

-<?php
-
-/**
- * @file
- * Contains \Drupal\menu\Plugin\Derivative\MenuBlock.
- */
-
-namespace Drupal\menu\Plugin\Derivative;
-
-use Drupal\system\Plugin\Derivative\SystemMenuBlock;
-
-/**
- * Provides block plugin definitions for custom menus.
- *
- * @see \Drupal\menu\Plugin\block\block\MenuBlock
- */
-class MenuBlock extends SystemMenuBlock {
-  /**
-   * {@inheritdoc}
-   */
-  public function getDerivativeDefinitions(array $base_plugin_definition) {
-    // Provide block plugin definitions for all user-defined (custom) menus.
-    foreach (menu_get_menus(FALSE) as $menu => $name) {
-      $this->derivatives[$menu] = $base_plugin_definition;
-      $this->derivatives[$menu]['admin_label'] = $name;
-      $this->derivatives[$menu]['cache'] = DRUPAL_NO_CACHE;
-    }
-    return $this->derivatives;
-  }
-
-}

core/modules/menu/lib/Drupal/menu/Tests/MenuLanguageTest.php

@@ -68,8 +68,6 @@ function testMenuLanguage() {
     $this->drupalPost('admin/structure/menu/add', $edit, t('Save'));
 
     // Check that the language settings were saved.
-    // The menu name should have been prefixed.
-    $menu_name = 'menu-' . $menu_name;
     $this->assertEqual(entity_load('menu', $menu_name)->langcode, $edit['langcode']);
     $language_settings = language_get_default_configuration('menu_link', $menu_name);
     $this->assertEqual($language_settings['langcode'], 'bb');

core/modules/menu/lib/Drupal/menu/Tests/MenuTest.php

@@ -158,14 +158,12 @@ function addCustomMenu() {
     $this->drupalGet('admin/structure/menu');
     $this->assertText($label, 'Menu created');
 
-    // Enable the custom menu block.
-    $menu_name = 'menu-' . $menu_name; // Drupal prepends the name with 'menu-'.
     // Confirm that the custom menu block is available.
     $this->drupalGet('admin/structure/block/list/' . \Drupal::config('system.theme')->get('default'));
     $this->assertText($label);
 
     // Enable the block.
-    $this->drupalPlaceBlock('menu_menu_block:' . $menu_name);
+    $this->drupalPlaceBlock('system_menu_block:' . $menu_name);
     return menu_load($menu_name);
   }
 
@@ -407,7 +405,7 @@ function testSystemMenuRename() {
   public function testBlockContextualLinks() {
     $this->drupalLogin($this->drupalCreateUser(array('administer menu', 'access contextual links', 'administer blocks')));
     $this->addMenuLink();
-    $block = $this->drupalPlaceBlock('system_menu_block:menu-tools', array('label' => 'Tools', 'module' => 'system'));
+    $block = $this->drupalPlaceBlock('system_menu_block:tools', array('label' => 'Tools', 'module' => 'system'));
     $this->drupalGet('test-page');
 
     $id = 'block:admin/structure/block/manage:' . $block->id() . ':|menu:admin/structure/menu/manage:tools:';

core/modules/menu/lib/Drupal/menu/Tests/MenuUninstallTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\menu\Tests\MenuUninstallTest.
+ */
+
+namespace Drupal\menu\Tests;
+
+use Drupal\simpletest\WebTestBase;
+
+/**
+ * Tests that uninstalling menu does not remove custom menus.
+ */
+class MenuUninstallTest extends WebTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('menu');
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Uninstall menu test',
+      'description' => 'Tests that uninstalling menu does not remove custom menus.',
+      'group' => 'Menu',
+    );
+  }
+
+  /**
+   * Tests Menu uninstall.
+   */
+  public function testMenuUninstall() {
+    \Drupal::moduleHandler()->disable(array('menu'));
+    \Drupal::moduleHandler()->uninstall(array('menu'));
+
+    $this->assertTrue(entity_load('menu', 'admin', TRUE), 'The \'admin\' menu still exists after uninstalling menu module.');
+  }
+
+}

core/modules/menu/menu.install

@@ -80,7 +80,7 @@ function menu_update_8004() {
   $result = db_query('SELECT * FROM {menu_custom}');
   foreach ($result as $menu) {
     // Save the config object.
-    Drupal::config('menu.menu.' . $menu->menu_name)
+    Drupal::config('system.menu.' . $menu->menu_name)
       ->set('id', $menu->menu_name)
       ->set('uuid', $uuid->generate())
       ->set('label', $menu->title)

core/modules/menu/menu.module

@@ -141,7 +141,7 @@ function menu_entity_info(&$entity_info) {
  */
 function menu_entity_bundle_info() {
   $bundles = array();
-  $config_names = config_get_storage_names_with_prefix('menu.menu.');
+  $config_names = config_get_storage_names_with_prefix('system.menu.');
   foreach ($config_names as $config_name) {
     $config = Drupal::config($config_name);
     $bundles['menu_link'][$config->get('id')] = array(
@@ -386,7 +386,10 @@ function _menu_parents_recurse($tree, $menu_name, $indent, &$options, $exclude,
  */
 function menu_block_view_system_menu_block_alter(array &$build, BlockPluginInterface $block) {
   // Add contextual links for system menu blocks.
-  if (isset($build['content'])) {
+  $menus = menu_list_system_menus();
+  // @todo Clean up when http://drupal.org/node/1874498 lands.
+  list(, $menu_name) = explode(':', $block->getPluginId());
+  if (isset($menus[$menu_name]) && isset($build['content'])) {
     foreach (element_children($build['content']) as $key) {
       $build['content']['#contextual_links']['menu'] = array('admin/structure/menu/manage', array($build['content'][$key]['#original_link']['menu_name']));
     }

core/modules/system/config/schema/system.schema.yml

@@ -308,7 +308,7 @@ system.theme:
         - type: string
           label: 'Theme'
 
-menu.menu.*:
+system.menu.*:
   type: mapping
   label: 'Menu'
   mapping:

core/modules/menu/config/menu.menu.account.yml → core/modules/system/config/system.menu.account.yml

@@ -2,3 +2,4 @@ id: account
 label: 'User account menu'
 description: 'Links related to the user account.'
 langcode: en
+locked: 1

core/modules/menu/config/menu.menu.admin.yml → core/modules/system/config/system.menu.admin.yml

@@ -2,3 +2,4 @@ id: admin
 label: Administration
 description: 'Contains links to administrative tasks.'
 langcode: en
+locked: 1

core/modules/menu/config/menu.menu.footer.yml → core/modules/system/config/system.menu.footer.yml

@@ -2,3 +2,4 @@ id: footer
 label: Footer
 description: 'Use this for linking to site information.'
 langcode: en
+locked: 1

core/modules/menu/config/menu.menu.main.yml → core/modules/system/config/system.menu.main.yml

@@ -2,3 +2,4 @@ id: main
 label: 'Main navigation'
 description: 'Use this for linking to the main site sections.'
 langcode: en
+locked: 1

core/modules/menu/config/menu.menu.tools.yml → core/modules/system/config/system.menu.tools.yml

@@ -2,3 +2,4 @@ id: tools
 label: Tools
 description: 'Contains links for site visitors. Some modules add their links here.'
 langcode: en
+locked: 1

core/modules/system/lib/Drupal/system/Entity/Menu.php

@@ -23,7 +23,7 @@
  *     "storage" = "Drupal\Core\Config\Entity\ConfigStorageController",
  *     "access" = "Drupal\system\MenuAccessController"
  *   },
- *   config_prefix = "menu.menu",
+ *   config_prefix = "system.menu",
  *   entity_keys = {
  *     "id" = "id",
  *     "label" = "label",
@@ -61,4 +61,34 @@ class Menu extends ConfigEntityBase implements MenuInterface {
    */
   public $description;
 
+  /**
+   * The locked status of this menu.
+   *
+   * @var bool
+   */
+  protected $locked = FALSE;
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getExportProperties() {
+    $properties = parent::getExportProperties();
+    // @todo Make $description protected and include it here, see
+    //   https://drupal.org/node/2030645.
+    $names = array(
+      'locked',
+    );
+    foreach ($names as $name) {
+      $properties[$name] = $this->get($name);
+    }
+    return $properties;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function isLocked() {
+    return (bool) $this->locked;
+  }
+
 }

core/modules/system/lib/Drupal/system/MenuAccessController.php

@@ -24,10 +24,8 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A
       return TRUE;
     }
     elseif ($operation == 'delete') {
-      // System menus could not be deleted.
-      // @todo Refactor in https://drupal.org/node/1882552
-      $system_menus = menu_list_system_menus();
-      if (isset($system_menus[$entity->id()])) {
+      // Locked menus could not be deleted.
+      if ($entity->isLocked()) {
         return FALSE;
       }
     }

core/modules/system/lib/Drupal/system/MenuInterface.php

@@ -14,4 +14,12 @@
  */
 interface MenuInterface extends ConfigEntityInterface {
 
+  /**
+   * Determines if this menu is locked.
+   *
+   * @return bool
+   *   TRUE if the menu is locked, FALSE otherwise.
+   */
+  public function isLocked();
+
 }

core/modules/system/lib/Drupal/system/Plugin/Block/SystemMenuBlock.php

@@ -12,11 +12,11 @@
 use Drupal\Core\Annotation\Translation;
 
 /**
- * Provides a 'System Menu' block.
+ * Provides a generic Menu block.
  *
  * @Block(
  *   id = "system_menu_block",
- *   admin_label = @Translation("System Menu"),
+ *   admin_label = @Translation("Menu"),
  *   category = "menu",
  *   derivative = "Drupal\system\Plugin\Derivative\SystemMenuBlock"
  * )
@@ -27,18 +27,17 @@ class SystemMenuBlock extends BlockBase {
    * Overrides \Drupal\block\BlockBase::access().
    */
   public function access() {
-    // @todo The 'Tools' menu should be available to anonymous users.
+    // @todo Clean up when http://drupal.org/node/1874498 lands.
     list( , $derivative) = explode(':', $this->getPluginId());
-    return ($GLOBALS['user']->isAuthenticated() || in_array($derivative, array('menu-main', 'menu-tools', 'menu-footer')));
+    return ($GLOBALS['user']->isAuthenticated() || in_array($derivative, array('main', 'tools', 'footer')));
   }
 
   /**
    * {@inheritdoc}
    */
   public function build() {
-    list( , $derivative) = explode(':', $this->getPluginId());
-    // Derivatives are prefixed with 'menu-'.
-    $menu = substr($derivative, 5);
+    // @todo Clean up when http://drupal.org/node/1874498 lands.
+    list(, $menu) = explode(':', $this->getPluginId());
     return menu_tree($menu);
   }
 

core/modules/system/lib/Drupal/system/Plugin/Derivative/SystemMenuBlock.php

@@ -8,29 +8,53 @@
 namespace Drupal\system\Plugin\Derivative;
 
 use Drupal\Component\Plugin\Derivative\DerivativeBase;
+use Drupal\Core\Entity\EntityStorageControllerInterface;
+use Drupal\Core\Plugin\Discovery\ContainerDerivativeInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
- * Provides block plugin definitions for system menus.
+ * Provides block plugin definitions for custom menus.
  *
- * @see \Drupal\system\Plugin\block\block\SystemMenuBlock
+ * @see \Drupal\system\Plugin\Block\SystemMenuBlock
  */
-class SystemMenuBlock extends DerivativeBase {
+class SystemMenuBlock extends DerivativeBase implements ContainerDerivativeInterface {
+
+  /**
+   * The menu storage.
+   *
+   * @var \Drupal\Core\Entity\EntityStorageControllerInterface
+   */
+  protected $menuStorage;
+
+  /**
+   * Constructs new SystemMenuBlock.
+   *
+   * @param \Drupal\Core\Entity\EntityStorageControllerInterface $menu_storage
+   *   The menu storage.
+   */
+  public function __construct(EntityStorageControllerInterface $menu_storage) {
+    $this->menuStorage = $menu_storage;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container, $base_plugin_id) {
+    return new static(
+      $container->get('entity.manager')->getStorageController('menu')
+    );
+  }
+
   /**
    * {@inheritdoc}
    */
   public function getDerivativeDefinitions(array $base_plugin_definition) {
-    // Provide a block plugin definition for each system menu.
-    foreach (menu_list_system_menus() as $menu => $name) {
-      // The block deltas need to be prefixed with 'menu-', since the 'main'
-      // menu would otherwise clash with the 'main' page content block.
-      $menu_key = 'menu-' . $menu;
-      $this->derivatives[$menu_key] = $base_plugin_definition;
-      // It is possible that users changed the menu label. Fall back on the
-      // built-in menu label if the entity was not found.
-      $entity = entity_load('menu', $menu);
-      $this->derivatives[$menu_key]['admin_label'] = !empty($entity) ? $entity->label() : $name;
-      $this->derivatives[$menu_key]['cache'] = DRUPAL_NO_CACHE;
+    foreach ($this->menuStorage->loadMultiple() as $menu => $entity) {
+      $this->derivatives[$menu] = $base_plugin_definition;
+      $this->derivatives[$menu]['admin_label'] = $entity->label();
+      $this->derivatives[$menu]['cache'] = DRUPAL_NO_CACHE;
     }
-    return parent::getDerivativeDefinitions($base_plugin_definition);
+    return $this->derivatives;
   }
+
 }

core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.php

@@ -48,9 +48,9 @@ function setUp() {
       'machine_name' => 'system_menu_tools',
       'region' => 'content',
     );
-    $this->drupalPlaceBlock('system_menu_block:menu-tools', $settings);
+    $this->drupalPlaceBlock('system_menu_block:tools', $settings);
     $settings['theme'] = \Drupal::config('system.theme')->get('admin');
-    $this->drupalPlaceBlock('system_menu_block:menu-tools', $settings);
+    $this->drupalPlaceBlock('system_menu_block:tools', $settings);
   }
 
   /**

core/modules/system/lib/Drupal/system/Tests/Menu/MenuRouterTest.php

@@ -65,7 +65,7 @@ function setUp() {
       ->set('admin', $this->admin_theme)
       ->save();
     theme_disable(array($this->alternate_theme));
-    $this->drupalPlaceBlock('system_menu_block:menu-tools');
+    $this->drupalPlaceBlock('system_menu_block:tools');
   }
 
   /**

core/modules/system/lib/Drupal/system/Tests/Menu/TrailTest.php

@@ -35,8 +35,8 @@ function setUp() {
 
     // This test puts menu links in the Tools and Administration menus and then
     // tests for their presence on the page.
-    $this->drupalPlaceBlock('system_menu_block:menu-tools');
-    $this->drupalPlaceBlock('system_menu_block:menu-admin');
+    $this->drupalPlaceBlock('system_menu_block:tools');
+    $this->drupalPlaceBlock('system_menu_block:admin');
   }
 
   /**

core/modules/system/system.module

@@ -9,7 +9,6 @@
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Language\Language;
 use Drupal\Core\Utility\ModuleInfo;
-use Drupal\system\Plugin\Block\SystemMenuBlock;
 use Drupal\user\UserInterface;