Skip to content

Commit

Permalink
Remove delegatesfocus behavior from popup
Browse files Browse the repository at this point in the history
The OpenUI resolved [1] to punt this part of the proposal until
later, when it might be built in a more general-purpose way.

[1] openui/open-ui#368 (comment)

Bug: 1307772
Change-Id: I2346791cf8a4e8ba7b2d1d51073d82cc212a4cf2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3811398
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032067}
NOKEYCHECK=True
GitOrigin-RevId: 68e42f3582d3b1d63d712b5f1ed080fa0f01f6ff
  • Loading branch information
mfreed7 authored and copybara-github committed Aug 5, 2022
1 parent 63891eb commit 91f4459
Showing 2 changed files with 5 additions and 57 deletions.
19 changes: 5 additions & 14 deletions blink/renderer/core/dom/element.cc
Original file line number Diff line number Diff line change
@@ -2815,17 +2815,17 @@ void Element::SetPopupFocusOnShow() {
GetDocument().UpdateStyleAndLayoutTreeForNode(this);

Element* control = nullptr;
if (IsAutofocusable() || hasAttribute(html_names::kDelegatesfocusAttr)) {
// If the popup has autofocus or delegatesfocus, focus it.
if (IsAutofocusable()) {
// If the popup has autofocus, focus it.
control = this;
} else {
// Otherwise, look for a child control that has the autofocus attribute.
control = GetPopupFocusableArea(/*autofocus_only=*/true);
}

// If the popup does not use autofocus or delegatesfocus, then the focus
// should remain on the currently active element.
// https://open-ui.org/components/popup.research.explainer#autofocus-logic
// If the popup does not use autofocus, then the focus should remain on the
// currently active element.
// https://open-ui.org/components/popup.research.explainer#focus-management
if (!control)
return;

@@ -5694,15 +5694,6 @@ void Element::Focus(const FocusParams& params) {
frame_owner_element->contentDocument()->UnloadStarted())
return;

if (HasValidPopupAttribute() &&
hasAttribute(html_names::kDelegatesfocusAttr)) {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
if (auto* node_to_focus = GetPopupFocusableArea(/*autofocus_only=*/false)) {
node_to_focus->Focus(params);
}
return;
}

// Ensure we have clean style (including forced display locks).
GetDocument().UpdateStyleAndLayoutTreeForNode(this);

Original file line number Diff line number Diff line change
@@ -49,49 +49,6 @@
<button autofocus>second autofocus button</button>
</div>

<div popup delegatesfocus data-test='delegatesfocus pop-up'>
<p>This is a pop-up</p>
<button class=should-be-focused>first button should be focused</button>
<button>second button</button>
</div>

<div popup delegatesfocus data-test='delegatesfocus takes precedence over autofocus'>
<p>This is a pop-up</p>
<button class=should-be-focused>first button</button>
<button autofocus>autofocus button should NOT be focused</button>
</div>

<div popup delegatesfocus autofocus data-test='delegatesfocus takes precedence over autofocus 2'>
<p>This is a pop-up</p>
<button class=should-be-focused>first button</button>
<button>autofocus button should NOT be focused</button>
</div>

<div popup delegatesfocus data-test='delegatesfocus on empty pop-up has no effect' data-no-focus></div>

<div popup data-test='delegatesfocus on child has no effect' data-no-focus>
<p>This is a pop-up</p>
<button delegatesfocus>first button</button>
</div>

<div popup delegatesfocus data-test='delegatesfocus skips contained pop-ups'>
<p>This is a pop-up</p>
<div popup>
<button>Contained pop-up button</button>
</div>
<button class=should-be-focused>first button</button>
<button>autofocus button should NOT be focused</button>
</div>

<div popup delegatesfocus data-test='delegatesfocus skips contained dialogs'>
<p>This is a pop-up</p>
<dialog>
<button>Contained dialog button</button>
</dialog>
<button class=should-be-focused>first button</button>
<button>autofocus button should NOT be focused</button>
</div>

<style>
[popup] {
border: 2px solid black;

0 comments on commit 91f4459

Please sign in to comment.