Implement dialog initial focus proposal

This has been discussed here: whatwg/html#4184 whatwg/html#8199 The gist of the changes are: 1. Make the dialog focusing steps look at keyboard focusable elements instead of any focusable element. 2. Make the dialog element itself get focus if it has the autofocus attribute set. 3. Make the dialog element itself get focus as a fallback instead of focus being "reset" to the body element. This patch also adds "outline:none" to several WPTs because this patch causes the dialog element to become initially focused in some cases and get a focus ring. I2S: https://groups.google.com/a/chromium.org/g/blink-dev/c/CEL3wWHrTAQ Fixed: 1193482 Change-Id: I1fee5981f72039a4467cbb35b2317832dd31bbea Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3984630 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1100024}
web-platform-tests · Feb 1, 2023 · 4a7eb16 · 4a7eb16
1 parent 531d5a4
commit 4a7eb16
Show file tree

Hide file tree

Showing 40 changed files with 174 additions and 66 deletions.
diff --git a/css/css-anchor-position/anchor-position-top-layer-001.html b/css/css-anchor-position/anchor-position-top-layer-001.html
@@ -22,6 +22,7 @@
   height: 100px;
   background: lime;
   anchor-scroll: --a;
+  outline: none;
 }
 
 body {

diff --git a/css/css-anchor-position/anchor-position-top-layer-002.html b/css/css-anchor-position/anchor-position-top-layer-002.html
@@ -22,6 +22,7 @@
   height: 100px;
   background: lime;
   anchor-scroll: --a;
+  outline: none;
 }
 
 body {

diff --git a/css/css-anchor-position/anchor-position-top-layer-003.html b/css/css-anchor-position/anchor-position-top-layer-003.html
@@ -22,6 +22,7 @@
   height: 100px;
   background: lime;
   anchor-scroll: --a;
+  outline: none;
 }
 
 body {

diff --git a/css/css-anchor-position/anchor-position-top-layer-004.html b/css/css-anchor-position/anchor-position-top-layer-004.html
@@ -22,6 +22,7 @@
   height: 100px;
   background: lime;
   anchor-scroll: --a;
+  outline: none;
 }
 
 body {

diff --git a/css/css-anchor-position/anchor-position-top-layer-005.html b/css/css-anchor-position/anchor-position-top-layer-005.html
@@ -35,6 +35,7 @@
   border: 0;
   padding: 0;
   inset: auto;
+  outline: none;
 }
 
 dialog::backdrop {

diff --git a/css/css-anchor-position/anchor-position-top-layer-006.html b/css/css-anchor-position/anchor-position-top-layer-006.html
@@ -34,6 +34,7 @@
   margin: 0;
   border: 0;
   padding: 0;
+  outline: none;
 }
 
 dialog::backdrop {

diff --git a/css/css-contain/content-visibility/content-visibility-with-top-layer-003.html b/css/css-contain/content-visibility/content-visibility-with-top-layer-003.html
@@ -12,6 +12,7 @@
 <style>
 .box { width: 100px; height: 100px; border: 1px solid black; }
 .hidden { content-visibility: hidden }
+#dialog { outline: none; }
 </style>
 
 <div id=container class=box>

diff --git a/css/css-contain/content-visibility/content-with-top-layer-ref.html b/css/css-contain/content-visibility/content-with-top-layer-ref.html
@@ -4,6 +4,7 @@
 
 <style>
 .box { width: 100px; height: 100px; border: 1px solid black; }
+dialog { outline: none; }
 </style>
 
 <div id=container class=box>

diff --git a/css/css-view-transitions/dialog-in-top-layer-during-transition-new.html b/css/css-view-transitions/dialog-in-top-layer-during-transition-new.html
@@ -18,6 +18,8 @@
 
   view-transition-name: dialog;
   contain: layout;
+
+  outline: none;
 }
 
 #target::backdrop {

diff --git a/css/css-view-transitions/dialog-in-top-layer-during-transition-old.html b/css/css-view-transitions/dialog-in-top-layer-during-transition-old.html
@@ -18,6 +18,8 @@
 
   view-transition-name: dialog;
   contain: layout;
+
+  outline: none;
 }
 
 #target::backdrop {

diff --git a/html/semantics/interactive-elements/the-dialog-element/backdrop-stacking-order.html b/html/semantics/interactive-elements/the-dialog-element/backdrop-stacking-order.html
@@ -6,6 +6,7 @@
     padding: 0px;
     border: none;
     margin: 0px;
+    outline: none;
 }
 
 #bottom::backdrop {

diff --git a/html/semantics/interactive-elements/the-dialog-element/child-sequential-focus.html b/html/semantics/interactive-elements/the-dialog-element/child-sequential-focus.html
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<link rel=author href="mailto:jarhar@chromium.org">
+<link rel=help href="https://github.com/whatwg/html/pull/8199">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<dialog autofocus id=autofocusdialog data-description="dialog element with autofocus should get initial focus.">
+  <button>focusable button</button>
+  <button autofocus class=target>autofocusable button</button>
+</dialog>
+
+<dialog id=keyboardfocusdialog data-description="Only keyboard-focusable elements should get dialog initial focus.">
+  <button tabindex="-1">mouse focusable button</button>
+  <button class=target>keyboard focusable button</button>
+</dialog>
+
+<dialog id=autofocuswithoutkeyboarddialog data-description="Autofocus takes precedence over keyboard-focusable requirement.">
+  <button>keyboard focusable button</button>
+  <button tabindex="-1" autofocus class=target>mouse focusable autofocus button</button>
+</dialog>
+
+<dialog id=subtree data-description="Only keyboard-focusable elements should get dialog initial focus including in subtrees.">
+  <div>
+    <button tabindex="-1">mouse focusable button</button>
+    <button class=target>keyboard focusable button</button>
+  </div>
+</dialog>
+
+<dialog id=nestedbuttons data-description="Only keyboard-focusable elements should get dialog initial focus including in nested buttons.">
+  <button tabindex="-1">
+    <span>mouse focusable button</span>
+    <button tabindex="-1">nested mouse focusable button</button>
+  </button>
+  <button class=target>keyboard focusable button</button>
+</dialog>
+
+<script>
+document.querySelectorAll('dialog').forEach(dialog => {
+  test(t => {
+    const target = dialog.querySelector('.target');
+    t.add_cleanup
+    t.add_cleanup(() => {
+      if (dialog.open)
+        dialog.close();
+    });
+
+    dialog.showModal();
+    assert_equals(document.activeElement,
+      dialog.hasAttribute('autofocus') ? dialog : target,
+      'showModal: the target element did not receive initial focus.');
+    dialog.close();
+
+    dialog.show();
+    assert_equals(document.activeElement, target, 'show: the target element did not receive initial focus.');
+    dialog.close();
+  }, dialog.dataset.description);
+});
+</script>