Issue #2957953 by alexpott, sleitner, pobster, Berdir, mohit_aghera,...

Issue #2957953 by alexpott, sleitner, pobster, Berdir, mohit_aghera, kishor_kolekar, joegraduate, trackleft2, bappa.sarkar, acbramley, ankithashetty, paulocs, batkor, dhirendra.mishra, seanB, priyanka.sahni, Abhijith S, chetanbharambe, vikashsoni, anmolgoyal74, Webbeh, larowlan, hudri, ryan.gibson, amietpatial, ddhuri, capysara, runeasgar, saranya ashokkumar, shimpy, nikitagupta: Editing menus user-experience has regressed

core/modules/menu_ui/src/MenuListBuilder.php

@@ -4,6 +4,7 @@
 
 use Drupal\Core\Config\Entity\ConfigEntityListBuilder;
 use Drupal\Core\Entity\EntityInterface;
+use Drupal\Core\Url;
 
 /**
  * Defines a class to build a listing of menu entities.
@@ -49,6 +50,9 @@ public function getDefaultOperations(EntityInterface $entity) {
         'title' => t('Add link'),
         'weight' => 20,
         'url' => $entity->toUrl('add-link-form'),
+        'query' => [
+          'destination' => $entity->toUrl('edit-form')->toString(),
+        ],
       ];
     }
     if (isset($operations['delete'])) {
@@ -57,6 +61,16 @@ public function getDefaultOperations(EntityInterface $entity) {
     return $operations;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function ensureDestination(Url $url) {
+    // We don't want to add the destination URL here, as it means we get
+    // redirected back to the list-builder after adding/deleting menu links from
+    // a menu.
+    return $url;
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/menu_ui/tests/src/Functional/MenuUiTest.php

@@ -85,6 +85,10 @@ protected function setUp(): void {
 
     $this->drupalPlaceBlock('page_title_block');
     $this->drupalPlaceBlock('system_menu_block:main');
+    $this->drupalPlaceBlock('local_actions_block', [
+      'region' => 'content',
+      'weight' => -100,
+    ]);
 
     $this->drupalCreateContentType(['type' => 'article', 'name' => 'Article']);
 
@@ -101,7 +105,7 @@ protected function setUp(): void {
   /**
    * Tests menu functionality using the admin and user interfaces.
    */
-  public function testMenu() {
+  public function testMenuAdministration() {
     // Log in the user.
     $this->drupalLogin($this->adminUser);
     $this->items = [];
@@ -195,6 +199,16 @@ public function addCustomMenuCRUD() {
     $menu->save();
     $this->drupalGet('admin/structure/menu/manage/' . $menu_name);
     $this->assertSession()->pageTextContains($new_label);
+
+    // Delete the custom menu via the UI to testing destination handling.
+    $this->drupalGet('admin/structure/menu');
+    $this->assertSession()->pageTextContains($new_label);
+    // Click the "Delete menu" operation in the Tools row.
+    $links = $this->xpath('//*/td[contains(text(),:menu_label)]/following::a[normalize-space()=:link_label]', [':menu_label' => $new_label, ':link_label' => 'Delete menu']);
+    $links[0]->click();
+    $this->submitForm([], 'Delete');
+    $this->assertSession()->addressEquals('admin/structure/menu');
+    $this->assertSession()->responseContains("The menu <em class=\"placeholder\">$new_label</em> has been deleted.");
   }
 
   /**
@@ -259,6 +273,7 @@ public function deleteCustomMenu() {
     $this->drupalGet("admin/structure/menu/manage/{$menu_name}/delete");
     $this->submitForm([], 'Delete');
     $this->assertSession()->statusCodeEquals(200);
+    $this->assertSession()->addressEquals("admin/structure/menu");
     $this->assertSession()->pageTextContains("The menu $label has been deleted.");
     $this->assertNull(Menu::load($menu_name), 'Custom menu was deleted');
     // Test if all menu links associated with the menu were removed from
@@ -279,11 +294,37 @@ public function deleteCustomMenu() {
    * Tests menu functionality.
    */
   public function doMenuTests() {
+    // Add a link to the tools menu first, to test cacheability metadata of the
+    // destination query string.
+    $this->drupalGet('admin/structure/menu/manage/tools');
+    $this->clickLink('Add link');
+    $link_title = $this->randomString();
+    $this->submitForm(['link[0][uri]' => '/', 'title[0][value]' => $link_title], 'Save');
+    $this->assertSession()->linkExists($link_title);
+    $this->assertSession()->addressEquals('admin/structure/menu/manage/tools');
+
+    // Test adding a menu link direct from the menus listing page.
+    $this->drupalGet('admin/structure/menu');
+    // Click the "Add link" operation in the Tools row.
+    $links = $this->xpath('//*/td[contains(text(),:menu_label)]/following::a[normalize-space()=:link_label]', [':menu_label' => 'Tools', ':link_label' => 'Add link']);
+    $links[0]->click();
+    $this->assertMatchesRegularExpression('#admin/structure/menu/manage/tools/add\?destination=(/[^/]*)*/admin/structure/menu/manage/tools$#', $this->getSession()->getCurrentUrl());
+    $link_title = $this->randomString();
+    $this->submitForm(['link[0][uri]' => '/', 'title[0][value]' => $link_title], 'Save');
+    $this->assertSession()->linkExists($link_title);
+    $this->assertSession()->addressEquals('admin/structure/menu/manage/tools');
+
     $menu_name = $this->menu->id();
 
-    // Test the 'Add link' local action.
-    $this->drupalGet(Url::fromRoute('entity.menu.edit_form', ['menu' => $menu_name]));
+    // Access the menu via the overview form to ensure it does not add a
+    // destination that breaks the user interface.
+    $this->drupalGet('admin/structure/menu');
 
+    // Select the edit menu link for our menu.
+    $links = $this->xpath('//*/td[contains(text(),:menu_label)]/following::a[normalize-space()=:link_label]', [':menu_label' => (string) $this->menu->label(), ':link_label' => 'Edit menu']);
+    $links[0]->click();
+
+    // Test the 'Add link' local action.
     $this->clickLink('Add link');
     $link_title = $this->randomString();
     $this->submitForm(['link[0][uri]' => '/', 'title[0][value]' => $link_title], 'Save');
@@ -300,6 +341,22 @@ public function doMenuTests() {
     $this->submitForm([], 'Delete');
     $this->assertSession()->addressEquals(Url::fromRoute('entity.menu.edit_form', ['menu' => $menu_name]));
 
+    // Clear the cache to ensure that recent caches aren't preventing us from
+    // seeing a broken add link.
+    $this->resetAll();
+    $this->drupalGet('admin/structure/menu');
+
+    // Select the edit menu link for our menu.
+    $links = $this->xpath('//*/td[contains(text(),:menu_label)]/following::a[normalize-space()=:link_label]', [':menu_label' => (string) $this->menu->label(), ':link_label' => 'Edit menu']);
+    $links[0]->click();
+
+    // Test the 'Add link' local action.
+    $this->clickLink('Add link');
+    $link_title = $this->randomString();
+    $this->submitForm(['link[0][uri]' => '/', 'title[0][value]' => $link_title], 'Save');
+    $this->assertSession()->linkExists($link_title);
+    $this->assertSession()->addressEquals(Url::fromRoute('entity.menu.edit_form', ['menu' => $menu_name]));
+
     // Add nodes to use as links for menu links.
     $node1 = $this->drupalCreateNode(['type' => 'article']);
     $node2 = $this->drupalCreateNode(['type' => 'article']);