Return focus to previously-focused element on popup hide

Akin to <dialog>'s behavior, the desired behavior is that focus
is returned to the previously-focused element when a popup is
hidden:

  openui/open-ui#327

Bug: 1307772
Change-Id: Ia6ae1981612a0c0150b8b5f51b485a00a9a90de9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3627159
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1006180}
NOKEYCHECK=True
GitOrigin-RevId: f1e305a9f30be10bebbe48a83aa104bbcd85db7f

blink/renderer/core/dom/document.cc

@@ -7199,33 +7199,35 @@ bool Document::HintShowing() const {
          (popup_and_hint_stack_.back()->PopupType() == PopupValueType::kHint);
 }
 
-void Document::HideTopmostPopupOrHint() {
+void Document::HideTopmostPopupOrHint(HidePopupFocusBehavior focus_behavior) {
   DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
   if (popup_and_hint_stack_.IsEmpty())
     return;
-  popup_and_hint_stack_.back()->hidePopup(ASSERT_NO_EXCEPTION);
+  popup_and_hint_stack_.back()->hidePopupInternal(focus_behavior);
 }
 
-void Document::HideAllPopupsUntil(const Element* endpoint) {
+void Document::HideAllPopupsUntil(const Element* endpoint,
+                                  HidePopupFocusBehavior focus_behavior) {
   DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
   while (!popup_and_hint_stack_.IsEmpty() &&
          popup_and_hint_stack_.back() != endpoint) {
-    popup_and_hint_stack_.back()->hidePopup(ASSERT_NO_EXCEPTION);
+    popup_and_hint_stack_.back()->hidePopupInternal(focus_behavior);
   }
 }
 
-void Document::HidePopupIfShowing(Element* popup) {
+void Document::HidePopupIfShowing(Element* popup,
+                                  HidePopupFocusBehavior focus_behavior) {
   DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
   DCHECK(popup->HasValidPopupAttribute());
   if (!popup->popupOpen())
     return;
   if (popup->PopupType() == PopupValueType::kAsync) {
-    popup->hidePopup(ASSERT_NO_EXCEPTION);
+    popup->hidePopupInternal(focus_behavior);
   } else {
-    HideAllPopupsUntil(popup);
+    HideAllPopupsUntil(popup, focus_behavior);
     DCHECK(!popup_and_hint_stack_.IsEmpty() &&
            popup_and_hint_stack_.back() == popup);
-    HideTopmostPopupOrHint();
+    HideTopmostPopupOrHint(focus_behavior);
   }
 }
 

blink/renderer/core/dom/document.h

@@ -59,6 +59,7 @@
 #include "third_party/blink/renderer/core/dom/document_encoding_data.h"
 #include "third_party/blink/renderer/core/dom/document_lifecycle.h"
 #include "third_party/blink/renderer/core/dom/document_timing.h"
+#include "third_party/blink/renderer/core/dom/element.h"
 #include "third_party/blink/renderer/core/dom/live_node_list_registry.h"
 #include "third_party/blink/renderer/core/dom/qualified_name.h"
 #include "third_party/blink/renderer/core/dom/synchronous_mutation_observer.h"
@@ -239,6 +240,7 @@ class VisitedLinkState;
 class WebMouseEvent;
 class WorkletAnimationController;
 enum class CSSPropertyID;
+enum class HidePopupFocusBehavior;
 struct AnnotatedRegionValue;
 struct FocusParams;
 struct IconURL;
@@ -1506,13 +1508,15 @@ class CORE_EXPORT Document : public ContainerNode,
   }
   bool PopupOrHintShowing() const;
   bool HintShowing() const;
-  void HideTopmostPopupOrHint();
+  void HideTopmostPopupOrHint(HidePopupFocusBehavior focus_behavior);
   // This hides all visible popups up to, but not including,
   // |endpoint|. If |endpoint| is nullptr, all popups are hidden.
-  void HideAllPopupsUntil(const Element* endpoint);
+  void HideAllPopupsUntil(const Element* endpoint,
+                          HidePopupFocusBehavior focus_behavior);
   // This hides the provided popup, if it is showing. This will also
   // hide all popups above |popup| in the popup stack.
-  void HidePopupIfShowing(Element* popup);
+  void HidePopupIfShowing(Element* popup,
+                          HidePopupFocusBehavior focus_behavior);
 
   // A non-null template_document_host_ implies that |this| was created by
   // EnsureTemplateDocument().

blink/renderer/core/dom/element.cc

@@ -2409,7 +2409,7 @@ void Element::UpdatePopupAttribute(String value) {
       return;
     // If the popup type is changing, hide it.
     if (popupOpen())
-      hidePopup(ASSERT_NO_EXCEPTION);
+      hidePopupInternal(HidePopupFocusBehavior::kFocusPreviousElement);
   }
   if (type == PopupValueType::kNone) {
     if (HasValidPopupAttribute()) {
@@ -2460,23 +2460,30 @@ void Element::showPopup(ExceptionState& exception_state) {
         DOMExceptionCode::kInvalidStateError,
         "Invalid on already-showing or disconnected popup elements");
   }
+  bool should_restore_focus = false;
   if (PopupType() == PopupValueType::kPopup ||
       PopupType() == PopupValueType::kHint) {
     if (GetDocument().HintShowing()) {
-      GetDocument().HideTopmostPopupOrHint();
+      GetDocument().HideTopmostPopupOrHint(HidePopupFocusBehavior::kNone);
     }
     if (PopupType() == PopupValueType::kPopup) {
       // Only hide other popups up to this popup's ancestral popup.
-      GetDocument().HideAllPopupsUntil(NearestOpenAncestralPopup(this));
+      GetDocument().HideAllPopupsUntil(NearestOpenAncestralPopup(this),
+                                       HidePopupFocusBehavior::kNone);
     }
     // Add this popup to the stack.
     auto& stack = GetDocument().PopupAndHintStack();
     DCHECK(!stack.Contains(this));
+    // We only restore focus for popup/hint, and only for the first popup in
+    // the stack.
+    should_restore_focus = stack.IsEmpty();
     stack.push_back(this);
   }
   GetPopupData()->setOpen(true);
   GetDocument().AddToTopLayer(this);
   PseudoStateChanged(CSSSelector::kPseudoPopupOpen);
+  GetPopupData()->setPreviouslyFocusedElement(
+      should_restore_focus ? GetDocument().FocusedElement() : nullptr);
   SetPopupFocusOnShow();
   // Queue the show event.
   Event* event = Event::CreateBubble(event_type_names::kShow);
@@ -2496,24 +2503,42 @@ void Element::hidePopup(ExceptionState& exception_state) {
         DOMExceptionCode::kInvalidStateError,
         "Invalid on already-hidden popup elements");
   }
+  hidePopupInternal(HidePopupFocusBehavior::kFocusPreviousElement);
+}
+
+void Element::hidePopupInternal(HidePopupFocusBehavior focus_behavior) {
   DCHECK(isConnected());
-  GetPopupData()->setOpen(false);
-  GetPopupData()->setInvoker(nullptr);
-  GetPopupData()->setNeedsRepositioningForSelectMenu(false);
+  DCHECK(HasValidPopupAttribute());
+  DCHECK(popupOpen());
   if (PopupType() == PopupValueType::kPopup ||
       PopupType() == PopupValueType::kHint) {
-    GetDocument().HideAllPopupsUntil(this);
-    // Remove this popup from the stack and the top layer.
+    // Hide any popups/hints above us in the stack.
+    GetDocument().HideAllPopupsUntil(this, focus_behavior);
+    // Then remove this popup/hint from the stack.
     auto& stack = GetDocument().PopupAndHintStack();
     DCHECK(stack.back() == this);
     stack.pop_back();
   }
+  GetPopupData()->setOpen(false);
+  GetPopupData()->setInvoker(nullptr);
+  GetPopupData()->setNeedsRepositioningForSelectMenu(false);
   GetDocument().RemoveFromTopLayer(this);
   PseudoStateChanged(CSSSelector::kPseudoPopupOpen);
   // Queue the hide event.
   Event* event = Event::CreateBubble(event_type_names::kHide);
   event->SetTarget(this);
   GetDocument().EnqueueAnimationFrameEvent(event);
+  if (Element* previously_focused_element =
+          GetPopupData()->previouslyFocusedElement()) {
+    GetPopupData()->setPreviouslyFocusedElement(nullptr);
+    if (focus_behavior == HidePopupFocusBehavior::kFocusPreviousElement) {
+      FocusOptions* focus_options = FocusOptions::Create();
+      focus_options->setPreventScroll(true);
+      // Call Focus() last, since it will fire a focus event which could modify
+      // this element. Focusing this element may also hide other popups.
+      previously_focused_element->Focus(focus_options);
+    }
+  }
 }
 
 void Element::SetPopupFocusOnShow() {
@@ -2702,16 +2727,19 @@ void Element::HandlePopupLightDismiss(const Event& event) {
     //    1. This mirrors typical platform popups, which dismiss on mousedown.
     //    2. This allows a mouse-drag that starts on a popup and finishes off
     //       the popup, without light-dismissing the popup.
-    document.HideAllPopupsUntil(NearestOpenAncestralPopup(target_node));
+    document.HideAllPopupsUntil(NearestOpenAncestralPopup(target_node),
+                                HidePopupFocusBehavior::kNone);
   } else if (event_type == event_type_names::kKeydown) {
     const KeyboardEvent* key_event = DynamicTo<KeyboardEvent>(event);
     if (key_event && key_event->key() == "Escape") {
       // Escape key just pops the topmost popup or hint off the stack.
-      document.HideTopmostPopupOrHint();
+      document.HideTopmostPopupOrHint(
+          HidePopupFocusBehavior::kFocusPreviousElement);
     }
   } else if (event_type == event_type_names::kFocusin) {
     // If we focus an element, hide all popups that don't contain that element.
-    document.HideAllPopupsUntil(NearestOpenAncestralPopup(target_node));
+    document.HideAllPopupsUntil(NearestOpenAncestralPopup(target_node),
+                                HidePopupFocusBehavior::kNone);
   }
 }
 
@@ -3115,7 +3143,9 @@ void Element::RemovedFrom(ContainerNode& insertion_point) {
   // If a popup is removed from the document, make sure it gets
   // removed from the popup element stack and the top layer.
   if (was_in_document && HasValidPopupAttribute()) {
-    insertion_point.GetDocument().HidePopupIfShowing(this);
+    // We can't run focus event handlers while removing elements.
+    insertion_point.GetDocument().HidePopupIfShowing(
+        this, HidePopupFocusBehavior::kNone);
   }
 
   if (GetDocument().GetPage())

blink/renderer/core/dom/element.h

@@ -174,6 +174,11 @@ enum class PopupTriggerAction {
   kHide,
 };
 
+enum class HidePopupFocusBehavior {
+  kNone,
+  kFocusPreviousElement,
+};
+
 typedef HeapVector<Member<Attr>> AttrNodeList;
 
 typedef HashMap<AtomicString, SpecificTrustedType> AttrNameToTrustedType;
@@ -553,6 +558,7 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable {
   bool popupOpen() const;
   void showPopup(ExceptionState& exception_state);
   void hidePopup(ExceptionState& exception_state);
+  void hidePopupInternal(HidePopupFocusBehavior focus_behavior);
   static const Element* NearestOpenAncestralPopup(Node* start_node);
   static void HandlePopupLightDismiss(const Event& event);
   void InvokePopup(Element* invoker);

blink/renderer/core/dom/popup_data.h

@@ -43,6 +43,13 @@ class PopupData final : public GarbageCollected<PopupData> {
     return needs_repositioning_for_select_menu_;
   }
 
+  Element* previouslyFocusedElement() const {
+    return previously_focused_element_;
+  }
+  void setPreviouslyFocusedElement(Element* element) {
+    previously_focused_element_ = element;
+  }
+
   HTMLSelectMenuElement* ownerSelectMenuElement() const {
     return owner_select_menu_element_;
   }
@@ -52,6 +59,7 @@ class PopupData final : public GarbageCollected<PopupData> {
 
   void Trace(Visitor* visitor) const {
     visitor->Trace(invoker_);
+    visitor->Trace(previously_focused_element_);
     visitor->Trace(owner_select_menu_element_);
   }
 
@@ -60,6 +68,7 @@ class PopupData final : public GarbageCollected<PopupData> {
   bool had_defaultopen_when_parsed_ = false;
   PopupValueType type_ = PopupValueType::kNone;
   WeakMember<Element> invoker_;
+  WeakMember<Element> previously_focused_element_;
 
   // TODO(crbug.com/1197720): The popup position should be provided by the new
   // anchored positioning scheme.

blink/renderer/core/fullscreen/fullscreen.cc

@@ -203,7 +203,7 @@ void GoFullscreen(Element& element,
   // If there are any open popups, close them, unless this fullscreen
   // element is a descendant of an open popup.
   if (RuntimeEnabledFeatures::HTMLPopupAttributeEnabled())
-    document.HideAllPopupsUntil(&element);
+    document.HideAllPopupsUntil(&element, HidePopupFocusBehavior::kNone);
 
   // To fullscreen an |element| within a |document|, set the |element|'s
   // fullscreen flag and add it to |document|'s top layer.

blink/renderer/core/html/forms/html_button_element.cc

@@ -117,7 +117,19 @@ void HTMLButtonElement::DefaultEventHandler(Event& event) {
       if (popup.element->popupOpen() &&
           (popup.action == PopupTriggerAction::kToggle ||
            popup.action == PopupTriggerAction::kHide)) {
-        popup.element->hidePopup(ASSERT_NO_EXCEPTION);
+        // Note that the order is: `mousedown` which runs popup light dismiss
+        // code, then (for clicked elements) focus is set to the clicked
+        // element, then |DOMActivate| runs here. Also note that the light
+        // dismiss code will not hide popups when an activating element is
+        // clicked. Taking that together, if the clicked control is a triggering
+        // element for a popup, light dismiss will do nothing, focus will be set
+        // to the triggering element, then this code will run and will set focus
+        // to the previously focused element. If instead the clicked control is
+        // not a triggering element, then the light dismiss code will hide the
+        // popup and set focus to the previously focused element, then the
+        // normal focus management code will reset focus to the clicked control.
+        popup.element->hidePopupInternal(
+            HidePopupFocusBehavior::kFocusPreviousElement);
       } else if (!popup.element->popupOpen() &&
                  (popup.action == PopupTriggerAction::kToggle ||
                   popup.action == PopupTriggerAction::kShow)) {

blink/renderer/core/html/forms/html_select_menu_element.cc

@@ -338,7 +338,8 @@ void HTMLSelectMenuElement::OpenListbox() {
 void HTMLSelectMenuElement::CloseListbox() {
   if (listbox_part_ && open()) {
     if (listbox_part_->HasValidPopupAttribute()) {
-      listbox_part_->hidePopup(ASSERT_NO_EXCEPTION);
+      // We will handle focus directly.
+      listbox_part_->hidePopupInternal(HidePopupFocusBehavior::kNone);
     }
     if (button_part_) {
       button_part_->Focus();

blink/renderer/core/html/html_dialog_element.cc

@@ -185,7 +185,7 @@ void HTMLDialogElement::show() {
 
   // Showing a <dialog> should hide all open popups.
   if (RuntimeEnabledFeatures::HTMLPopupAttributeEnabled()) {
-    GetDocument().HideAllPopupsUntil(nullptr);
+    GetDocument().HideAllPopupsUntil(nullptr, HidePopupFocusBehavior::kNone);
   }
 
   // The layout must be updated here because setFocusForDialog calls
@@ -243,7 +243,7 @@ void HTMLDialogElement::showModal(ExceptionState& exception_state) {
 
   // Showing a <dialog> should hide all open popups.
   if (RuntimeEnabledFeatures::HTMLPopupAttributeEnabled()) {
-    document.HideAllPopupsUntil(nullptr);
+    document.HideAllPopupsUntil(nullptr, HidePopupFocusBehavior::kNone);
   }
 
   document.AddToTopLayer(this);