Issue #3359494 by bnjmnm, Spokje, lauriii, hooroomoo: Focus is lost on dialog close if the opener is inside a collapsible element

core/misc/dialog/dialog.ajax.js

@@ -3,7 +3,7 @@
  * Extends the Drupal AJAX functionality to integrate the dialog API.
  */
 
-(function ($, Drupal) {
+(function ($, Drupal, { focusable }) {
   /**
    * Initialize dialogs for Ajax purposes.
    *
@@ -46,6 +46,30 @@
       // Overwrite the close method to remove the dialog on closing.
       settings.dialog.close = function (event, ...args) {
         originalClose.apply(settings.dialog, [event, ...args]);
+        // Check if the opener element is inside an AJAX container.
+        const $element = $(event.target);
+        const ajaxContainer = $element.data('uiDialog')
+          ? $element
+              .data('uiDialog')
+              .opener.closest('[data-drupal-ajax-container]')
+          : [];
+
+        // If the opener element was in an ajax container, and focus is on the
+        // body element, we can assume focus was lost. To recover, focus is
+        // moved to the first focusable element in the container.
+        if (
+          ajaxContainer.length &&
+          (document.activeElement === document.body ||
+            $(document.activeElement).not(':visible'))
+        ) {
+          const focusableChildren = focusable(ajaxContainer[0]);
+          if (focusableChildren.length > 0) {
+            setTimeout(() => {
+              focusableChildren[0].focus();
+            }, 0);
+          }
+        }
+
         $(event.target).remove();
       };
     },
@@ -246,4 +270,4 @@
   $(window).on('dialog:beforeclose', (e, dialog, $element) => {
     $element.off('.dialog');
   });
-})(jQuery, Drupal);
+})(jQuery, Drupal, window.tabbable);

core/modules/block/tests/src/Functional/Views/DisplayBlockTest.php

@@ -393,8 +393,8 @@ class DisplayBlockTest extends ViewTestBase {
     $cached_id_token = Crypt::hmacBase64($cached_id, Settings::getHashSalt() . $this->container->get('private_key')->get());
     // @see \Drupal\contextual\Tests\ContextualDynamicContextTest:assertContextualLinkPlaceHolder()
     // Check existence of the contextual link placeholders.
-    $this->assertSession()->responseContains('<div' . new Attribute(['data-contextual-id' => $id, 'data-contextual-token' => $id_token]) . '></div>');
-    $this->assertSession()->responseContains('<div' . new Attribute(['data-contextual-id' => $cached_id, 'data-contextual-token' => $cached_id_token]) . '></div>');
+    $this->assertSession()->responseContains('<div' . new Attribute(['data-contextual-id' => $id, 'data-contextual-token' => $id_token, 'data-drupal-ajax-container' => '']) . '></div>');
+    $this->assertSession()->responseContains('<div' . new Attribute(['data-contextual-id' => $cached_id, 'data-contextual-token' => $cached_id_token, 'data-drupal-ajax-container' => '']) . '></div>');
 
     // Get server-rendered contextual links.
     // @see \Drupal\contextual\Tests\ContextualDynamicContextTest:renderContextualLinks()

core/modules/contextual/src/Element/ContextualLinksPlaceholder.php

@@ -49,6 +49,7 @@ class ContextualLinksPlaceholder extends RenderElement {
     $attribute = new Attribute([
       'data-contextual-id' => $element['#id'],
       'data-contextual-token' => $token,
+      'data-drupal-ajax-container' => '',
     ]);
     $element['#markup'] = new FormattableMarkup('<div@attributes></div>', ['@attributes' => $attribute]);
 

core/modules/contextual/tests/src/FunctionalJavascript/ContextualLinksTest.php

@@ -85,9 +85,27 @@ class ContextualLinksTest extends WebDriverTestBase {
     $this->assertSession()->assertWaitOnAjaxRequest();
     $current_page_string = 'NOT_RELOADED_IF_ON_PAGE';
     $this->getSession()->executeScript('document.body.appendChild(document.createTextNode("' . $current_page_string . '"));');
-    $this->clickContextualLink('#block-branding', 'Test Link with Ajax');
+
+    // Move the pointer over the branding block so the contextual link appears
+    // as it would with a real user interaction. Otherwise clickContextualLink()
+    // does not open the dialog in a manner that is opener-aware, and it isn't
+    // possible to reliably test focus management.
+    $driver_session = $this->getSession()->getDriver()->getWebDriverSession();
+    $element = $driver_session->element('css selector', '#block-branding');
+    $driver_session->moveto(['element' => $element->getID()]);
+    $this->clickContextualLink('#block-branding', 'Test Link with Ajax', FALSE);
     $this->assertNotEmpty($this->assertSession()->waitForElementVisible('css', '#drupal-modal'));
     $this->assertSession()->elementContains('css', '#drupal-modal', 'Everything is contextual!');
+    $this->getSession()->executeScript('document.querySelector("#block-branding .trigger").addEventListener("focus", (e) => e.target.classList.add("i-am-focused"))');
+    $this->getSession()->getPage()->pressButton('Close');
+    $this->assertSession()->assertNoElementAfterWait('css', 'ui.dialog');
+
+    // When the dialog is closed, the opening contextual link is now inside a
+    // collapsed container, so focus should be routed to the contextual link
+    // toggle button.
+    $this->assertNotNull($this->assertSession()->waitForElement('css', '.trigger.i-am-focused'), $this->getSession()->getPage()->find('css', '#block-branding')->getOuterHtml());
+    $this->assertJsCondition('document.activeElement === document.querySelector("#block-branding button.trigger")', 10000, 'Focus should be on the contextual trigger, but instead is at ' . $this->getSession()->evaluateScript('document.activeElement.outerHTML'));
+
     // Check to make sure that page was not reloaded.
     $this->assertSession()->pageTextContains($current_page_string);
 

core/modules/system/tests/modules/dialog_renderer_test/dialog_renderer_test.routing.yml

@@ -29,3 +29,11 @@ dialog_renderer_test.modal_content_input:
     _title: 'Thing 3'
   requirements:
     _access: 'TRUE'
+
+dialog_renderer_test.collapsed_opener:
+  path: '/dialog_renderer-collapsed-opener'
+  defaults:
+    _controller: '\Drupal\dialog_renderer_test\Controller\TestController::collapsedOpener'
+    _title: 'Collapsed Openers'
+  requirements:
+    _access: 'TRUE'

core/modules/system/tests/modules/dialog_renderer_test/src/Controller/TestController.php

@@ -189,4 +189,39 @@ class TestController {
     ];
   }
 
+  /**
+   * Displays a dropbutton with a link that opens in a modal dialog.
+   *
+   * @return array
+   *   Render array with links.
+   */
+  public function collapsedOpener() {
+    return [
+      '#markup' => '<h2>Honk</h2>',
+      'dropbutton' => [
+        '#type' => 'dropbutton',
+        '#dropbutton_type' => 'small',
+        '#links' => [
+          'front' => [
+            'title' => 'front!',
+            'url' => Url::fromRoute('<front>'),
+          ],
+          'in a dropbutton' => [
+            'title' => 'inside a dropbutton',
+            'url' => Url::fromRoute('dialog_renderer_test.modal_content'),
+            'attributes' => [
+              'class' => ['use-ajax'],
+              'data-dialog-type' => 'modal',
+            ],
+          ],
+        ],
+      ],
+      '#attached' => [
+        'library' => [
+          'core/drupal.ajax',
+        ],
+      ],
+    ];
+  }
+
 }

core/modules/system/tests/src/FunctionalJavascript/ModalRendererTest.php

@@ -76,4 +76,31 @@ class ModalRendererTest extends WebDriverTestBase {
     $this->assertJsCondition('document.activeElement === document.querySelector(".ui-dialog .form-text")');
   }
 
+  /**
+   * Confirm focus management of a dialog openers in a dropbutton.
+   */
+  public function testOpenerInDropbutton() {
+    $assert_session = $this->assertSession();
+    $page = $this->getSession()->getPage();
+
+    $this->drupalGet('dialog_renderer-collapsed-opener');
+
+    // Open a modal using a link inside a dropbutton.
+    $page->find('css', '.dropbutton-toggle button')->click();
+    $modal_link = $assert_session->waitForElementVisible('css', '.secondary-action a');
+    $modal_link->click();
+    $assert_session->waitForElementVisible('css', '.ui-dialog');
+    $assert_session->assertVisibleInViewport('css', '.ui-dialog .ui-dialog-content');
+    $page->pressButton('Close');
+
+    // When the dialog "closes" it is still present, so wait on it switching to
+    // `display: none;`.
+    $assert_session->waitForElement('css', '.ui-dialog[style*="display: none;"]');
+
+    // Confirm that when the modal closes, focus is moved to the first visible
+    // and focusable item in the contextual link container, because the original
+    // opener is not available.
+    $this->assertJsCondition('document.activeElement === document.querySelector(".dropbutton-action a")');
+  }
+
 }