Skip to content

Commit

Permalink
Implement new dialog initial focus algorithm
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=250795

Reviewed by NOBODY (OOPS!).

Updates the dialog focusing steps and the focus delegate steps to match latest spec.

Spec PR: whatwg/html#8199

* LayoutTests/fast/layers/layer-order-after-top-layer.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/child-sequential-focus-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-focus-shadow-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/dialog-showModal-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/focus-previous-iframe.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/show-modal-focusing-steps-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-focus-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-dialog-initial-focus-expected.txt: Removed.
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/popovers/popover-dialog-initial-focus.html: Removed.

These tests no longer exist in upstream WPT repo.

* Source/WebCore/dom/Element.cpp:
(WebCore::Element::findFocusDelegateForTarget):
* Source/WebCore/html/HTMLDialogElement.cpp:
(WebCore::HTMLDialogElement::runFocusingSteps):
(WebCore::HTMLDialogElement::supportsFocus const):
* Source/WebCore/html/HTMLDialogElement.h:
  • Loading branch information
lukewarlow authored and nt1m committed Jul 22, 2024
1 parent b6dc732 commit 0ebb3a3
Show file tree
Hide file tree
Showing 12 changed files with 66 additions and 61 deletions.
3 changes: 1 addition & 2 deletions LayoutTests/fast/layers/layer-order-after-top-layer.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<!DOCTYPE html>
<html>
<style>

dialog, .above {
position: absolute;
left: 20px;
Expand All @@ -20,6 +19,7 @@
display: block;
padding: 0;
margin: 0;
outline: none;
}

.above {
Expand All @@ -30,7 +30,6 @@
height: 300px;
padding: 0;
margin: 0;
background-color: Canvas;
color: white;
background-color: green;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

FAIL dialog element with autofocus should get initial focus. assert_equals: showModal: the target element did not receive initial focus. expected Element node <dialog autofocus="" id="autofocusdialog" data-descriptio... but got Element node <body><dialog autofocus="" id="autofocusdialog" data-desc...
PASS dialog element with autofocus should get initial focus.
FAIL Only keyboard-focusable elements should get dialog initial focus. assert_equals: showModal: the target element did not receive initial focus. expected Element node <button class="target">keyboard focusable button</button> but got Element node <button tabindex="-1">mouse focusable button</button>
PASS Autofocus takes precedence over keyboard-focusable requirement.
FAIL Only keyboard-focusable elements should get dialog initial focus including in subtrees. assert_equals: showModal: the target element did not receive initial focus. expected Element node <button class="target">keyboard focusable button</button> but got Element node <button tabindex="-1">mouse focusable button</button>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
Focus between tests

FAIL show: No autofocus, no delegatesFocus, no siblings assert_equals: expected Element node <dialog data-description="No autofocus, no delegatesFocus... but got Element node <button id="focus-between-tests">Focus between tests</but...
FAIL showModal: No autofocus, no delegatesFocus, no siblings assert_equals: expected Element node <dialog data-description="No autofocus, no delegatesFocus... but got Element node <body>

<!--
We focus this one between each test, to en...
PASS show: No autofocus, no delegatesFocus, no siblings
PASS showModal: No autofocus, no delegatesFocus, no siblings
PASS show: No autofocus, no delegatesFocus, sibling before
PASS showModal: No autofocus, no delegatesFocus, sibling before
PASS show: No autofocus, no delegatesFocus, sibling after
Expand All @@ -29,11 +26,8 @@ PASS show: Autofocus on shadow host, yes delegatesFocus, sibling before
PASS showModal: Autofocus on shadow host, yes delegatesFocus, sibling before
PASS show: Autofocus on shadow host, yes delegatesFocus, sibling after
PASS showModal: Autofocus on shadow host, yes delegatesFocus, sibling after
FAIL show: Autofocus on shadow host, no delegatesFocus, no siblings assert_equals: expected Element node <dialog data-description="Autofocus on shadow host, no de... but got Element node <button id="focus-between-tests">Focus between tests</but...
FAIL showModal: Autofocus on shadow host, no delegatesFocus, no siblings assert_equals: expected Element node <dialog data-description="Autofocus on shadow host, no de... but got Element node <body>

<!--
We focus this one between each test, to en...
PASS show: Autofocus on shadow host, no delegatesFocus, no siblings
PASS showModal: Autofocus on shadow host, no delegatesFocus, no siblings
PASS show: Autofocus on shadow host, no delegatesFocus, sibling before
PASS showModal: Autofocus on shadow host, no delegatesFocus, sibling before
PASS show: Autofocus on shadow host, no delegatesFocus, sibling after
Expand All @@ -44,11 +38,8 @@ PASS show: Autofocus inside shadow tree, yes delegatesFocus, sibling before
PASS showModal: Autofocus inside shadow tree, yes delegatesFocus, sibling before
PASS show: Autofocus inside shadow tree, yes delegatesFocus, sibling after
PASS showModal: Autofocus inside shadow tree, yes delegatesFocus, sibling after
FAIL show: Autofocus inside shadow tree, no delegatesFocus, no siblings assert_equals: expected Element node <dialog data-description="Autofocus inside shadow tree, n... but got Element node <button id="focus-between-tests">Focus between tests</but...
FAIL showModal: Autofocus inside shadow tree, no delegatesFocus, no siblings assert_equals: expected Element node <dialog data-description="Autofocus inside shadow tree, n... but got Element node <body>

<!--
We focus this one between each test, to en...
PASS show: Autofocus inside shadow tree, no delegatesFocus, no siblings
PASS showModal: Autofocus inside shadow tree, no delegatesFocus, no siblings
PASS show: Autofocus inside shadow tree, no delegatesFocus, sibling before
PASS showModal: Autofocus inside shadow tree, no delegatesFocus, sibling before
PASS show: Autofocus inside shadow tree, no delegatesFocus, sibling after
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ PASS showModal() on a <dialog> that already has an open attribute throws an Inva
PASS showModal() on a <dialog> after initial showModal() and removing the open attribute
PASS showModal() on a <dialog> not in a Document throws an InvalidStateError exception
PASS when opening multiple dialogs, only the newest one is non-inert
FAIL opening dialog without focusable children assert_equals: expected Element node <dialog id="d6" open=""></dialog> but got Element node <body><div id="log"></div>
<button id="b0">OK</button>
<d...
PASS opening dialog without focusable children
PASS opening dialog with multiple focusable children
PASS opening dialog with multiple focusable children, one having the autofocus attribute
FAIL when opening multiple dialogs, the most recently opened is rendered on top assert_equals: expected Element node <dialog id="d10" open=""></dialog> but got Element node <dialog id="d11" open=""></dialog>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
CONSOLE MESSAGE: Error: assert_equals: expected Element node <input></input> but got Element node <iframe srcdoc="<input><dialog> Dialog in child </dialog>...


PASS Focus should move back from parent document to child document
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@


FAIL focus when a modal dialog is opened assert_equals: expected Element node <dialog id="outer-dialog" open="">
<dialog id="dialog... but got Element node <body>
<button id="outer-button" autofocus=""></button>
<...
PASS focus when a modal dialog is opened

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,50 @@ PASS Popover corner cases test: autofocus multiple children
PASS Popover focus test: autofocus popover and multiple autofocus children
PASS Popover button click focus test: autofocus popover and multiple autofocus children
PASS Popover corner cases test: autofocus popover and multiple autofocus children
FAIL Popover focus test: Opening dialogs as popovers should use dialog initial focus algorithm. assert_equals: Opening dialogs as popovers should use dialog initial focus algorithm. directly focus with popover.focus() expected Element node <dialog popover="auto" data-test="Opening dialogs as popo... but got Element node <button class="should-be-focused" tabindex="0">button</bu...
FAIL Popover button click focus test: Opening dialogs as popovers should use dialog initial focus algorithm. assert_false: popover should start out hidden expected false got true
FAIL Popover corner cases test: Opening dialogs as popovers should use dialog initial focus algorithm. assert_false: popover should start out hidden expected false got true
FAIL Popover focus test: Opening dialogs as popovers which have autofocus should focus the dialog. assert_equals: expected Element node <button tabindex="0" id="priorFocus"></button> but got Element node <button class="should-be-focused" tabindex="0">button</bu...
FAIL Popover button click focus test: Opening dialogs as popovers which have autofocus should focus the dialog. assert_equals: expected Element node <button tabindex="0" id="priorFocus"></button> but got Element node <button class="should-be-focused" tabindex="0">button</bu...
PASS Popover focus test: Opening dialogs as popovers should use dialog initial focus algorithm.
FAIL Popover button click focus test: Opening dialogs as popovers should use dialog initial focus algorithm. assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button tabindex="0" popovertarget="popover-id">Click me<... but got Element node <body>

















<dialog popover="auto" autofocus=...
PASS Popover corner cases test: Opening dialogs as popovers should use dialog initial focus algorithm.
PASS Popover focus test: Opening dialogs as popovers which have autofocus should focus the dialog.
FAIL Popover button click focus test: Opening dialogs as popovers which have autofocus should focus the dialog. assert_equals: focus should move to the button when clicked, and should stay there when the popover closes expected Element node <button tabindex="0" popovertarget="popover-id">Click me<... but got Element node <body>



















<style>
[popover] {
borde...
PASS Popover corner cases test: Opening dialogs as popovers which have autofocus should focus the dialog.

3 changes: 3 additions & 0 deletions Source/WebCore/dom/Element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3682,6 +3682,9 @@ RefPtr<Element> Element::findFocusDelegateForTarget(ContainerNode& target, Focus
if (RefPtr element = autoFocusDelegate(target, trigger))
return element;
for (Ref element : descendantsOfType<Element>(target)) {
if (is<HTMLDialogElement>(&target) && element->isKeyboardFocusable(nullptr))
return element;

switch (trigger) {
case FocusTrigger::Click:
if (element->isMouseFocusable())
Expand Down
7 changes: 6 additions & 1 deletion Source/WebCore/html/HTMLDialogElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ void HTMLDialogElement::queueCancelTask()
void HTMLDialogElement::runFocusingSteps()
{
RefPtr<Element> control;
if (m_isModal && hasAttributeWithoutSynchronization(HTMLNames::autofocusAttr))
if (hasAttributeWithoutSynchronization(HTMLNames::autofocusAttr))
control = this;
if (!control)
control = findFocusDelegate();
Expand All @@ -205,6 +205,11 @@ void HTMLDialogElement::runFocusingSteps()
topDocument->setAutofocusProcessed();
}

bool HTMLDialogElement::supportsFocus() const
{
return true;
}

void HTMLDialogElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
{
HTMLElement::removedFromAncestor(removalType, oldParentOfRemovedTree);
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/html/HTMLDialogElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class HTMLDialogElement final : public HTMLElement {

void removedFromAncestor(RemovalType, ContainerNode& oldParentOfRemovedTree) final;
void setIsModal(bool newValue);
bool supportsFocus() const final;

String m_returnValue;
bool m_isModal { false };
Expand Down

0 comments on commit 0ebb3a3

Please sign in to comment.