From e96c4c3d5c6510202598f558ea520b80f9507b31 Mon Sep 17 00:00:00 2001
From: Mason Freed <masonf@chromium.org>
Date: Wed, 4 Dec 2024 11:53:05 -0800
Subject: [PATCH] Update dialog closedby behavior to only close one at a time

Per the discussions at:
- https://github.com/openui/open-ui/issues/1128
- https://github.com/whatwg/html/issues/9373#issuecomment-2490229365

both developer and editor feedback is that the prior behavior was
a bit odd. In the case where there are "nested" dialogs, the desired
behavior is for light dismiss clicks to just pop the topmost dialog
off the stack, rather than trying to close all non-clicked dialogs.

This CL implements that behavior and updates the test. After
implementing this behavior and playing with it a bit, I agree
with the feedback - this feels more like what users would expect.

Once this lands and I get a bit of feedback, I'll incorporate this
into the spec PR.

Bug: 376516550
Change-Id: I4b5cfb7dd4a27fc304e52759d882b47394e55524
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6068807
Commit-Queue: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1391794}
---
 ...og-popover-closedby-complex.tentative.html | 32 +++++++++++--------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/html/semantics/interactive-elements/the-dialog-element/dialog-popover-closedby-complex.tentative.html b/html/semantics/interactive-elements/the-dialog-element/dialog-popover-closedby-complex.tentative.html
index aa45ecaad4ff8a..0d300595287a14 100644
--- a/html/semantics/interactive-elements/the-dialog-element/dialog-popover-closedby-complex.tentative.html
+++ b/html/semantics/interactive-elements/the-dialog-element/dialog-popover-closedby-complex.tentative.html
@@ -24,6 +24,7 @@
   #popoverA { top: 150px; bottom: auto; padding:0; }
   #dialogB { top: 200px; bottom: auto; padding:0; }
   #popoverB { top: 250px; bottom: auto; padding:0; }
+  dialog { position: fixed; }
 </style>
 
 <script>
@@ -71,38 +72,43 @@
       await clickOn(unrelated);
       // Clicking outside all is actually a click on a dialog backdrop.
       // If dialogB is modal, it'll be dialogB, which is nested inside popoverA.
-      assertStates(false,modalB,false,false);
+      // Either way, both popoverB and dialogB should close.
+      assertStates(true,modalB,false,false);
+      await clickOn(unrelated);
+      // Clicking outside again should close the remaining two.
+      assertStates(false,false,false,false);
     },`clicking outside all with ${modalAString} and ${modalBString}`);
 
     promise_test(async (t) => {
       openDialogPopoverStack(t,modalA,modalB);
       await clickOn(popoverB);
-      // Clicking popoverB will keep both popovers plus the intervening dialogB
-      // open, because they're a stack.
-      assertStates(false,true,true,true);
+      // Clicking popoverB should keep everything open.
+      assertStates(true,true,true,true);
     },`clicking popoverB with ${modalAString} and ${modalBString}`);
 
     promise_test(async (t) => {
       openDialogPopoverStack(t,modalA,modalB);
       await clickOn(dialogB);
-      // dialogB is nested inside popoverA.
-      assertStates(false,true,true,false);
+      // Only popoverB should be light dismissed.
+      assertStates(true,true,true,false);
     },`clicking dialogB with ${modalAString} and ${modalBString}`);
 
     promise_test(async (t) => {
       openDialogPopoverStack(t,modalA,modalB);
       await clickOn(popoverA);
-      // If dialogB is modal, then clicking popoverA is actually a backdrop
-      // click on dialogB, which will close it. PopoverA stays open because
-      // dialogB is nested inside popoverA.
-      assertStates(false,true,!modalB,false);
+      // Both dialogB and popoverB should be light dismissed.
+      assertStates(true,true,false,false);
     },`clicking popoverA with ${modalAString} and ${modalBString}`);
 
     promise_test(async (t) => {
       openDialogPopoverStack(t,modalA,modalB);
-      await clickOn(dialogB);
-      // Again, this is a backdrop click on dialogB.
-      assertStates(false,true,true,false);
+      await clickOn(dialogA);
+      // If dialogB is modal, clicking on dialogA is actually clicking on dialogB,
+      // which means popoverB will stay open.
+      assertStates(true,modalB,false,false);
+      await clickOn(dialogA);
+      // The next click on dialogA should light dismiss popoverA.
+      assertStates(true,false,false,false);
     },`clicking dialogA with ${modalAString} and ${modalBString}`);
   });
 });