Focus previously focused element when dialog is closed

Fixed: 298078
Change-Id: I19a1936ae2bca6136b403447353315e1e650933f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2827405
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#886339}

content/child/runtime_features.cc

@@ -350,6 +350,10 @@ void SetRuntimeFeaturesFromChromiumFeatures() {
            kSetOnlyIfOverridden},
           {"DeclarativeShadowDOM", blink::features::kDeclarativeShadowDOM},
           {"DocumentTransition", blink::features::kDocumentTransition},
+          // TODO(crbug.com/649162): Remove DialogFocusNewSpecBehavior after
+          // the feature is in stable with no issues.
+          {"DialogFocusNewSpecBehavior",
+           blink::features::kDialogFocusNewSpecBehavior},
           {"FeaturePolicyForClientHints",
            features::kFeaturePolicyForClientHints},
           {"EditingNG", blink::features::kEditingNG},

third_party/blink/common/features.cc

@@ -968,5 +968,12 @@ const base::Feature kMinimizeAudioProcessingForUnusedOutput{
     "MinimizeAudioProcessingForUnusedOutput",
     base::FEATURE_DISABLED_BY_DEFAULT};
 
+// When <dialog>s are closed, this focuses the "previously focused" element
+// which had focus when the <dialog> was first opened.
+// TODO(crbug.com/649162): Remove DialogFocusNewSpecBehavior after
+// the feature is in stable with no issues.
+const base::Feature kDialogFocusNewSpecBehavior{
+    "DialogFocusNewSpecBehavior", base::FEATURE_ENABLED_BY_DEFAULT};
+
 }  // namespace features
 }  // namespace blink

third_party/blink/public/common/features.h

@@ -405,6 +405,12 @@ BLINK_COMMON_EXPORT extern const base::Feature kFledgeInterestGroupAPI;
 BLINK_COMMON_EXPORT extern const base::Feature
     kMinimizeAudioProcessingForUnusedOutput;
 
+// When <dialog>s are closed, this focuses the "previously focused" element
+// which had focus when the <dialog> was first opened.
+// TODO(crbug.com/649162): Remove DialogFocusNewSpecBehavior after
+// the feature is in stable with no issues.
+BLINK_COMMON_EXPORT extern const base::Feature kDialogFocusNewSpecBehavior;
+
 }  // namespace features
 }  // namespace blink
 

third_party/blink/renderer/core/html/html_dialog_element.cc

@@ -33,6 +33,7 @@
 #include "third_party/blink/renderer/core/frame/local_frame_view.h"
 #include "third_party/blink/renderer/core/frame/web_feature.h"
 #include "third_party/blink/renderer/core/fullscreen/fullscreen.h"
+#include "third_party/blink/renderer/core/html/focus_options.h"
 #include "third_party/blink/renderer/core/html/forms/html_form_control_element.h"
 #include "third_party/blink/renderer/core/html/html_frame_owner_element.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
@@ -109,7 +110,8 @@ static void InertSubtreesChanged(Document& document) {
 HTMLDialogElement::HTMLDialogElement(Document& document)
     : HTMLElement(html_names::kDialogTag, document),
       is_modal_(false),
-      return_value_("") {
+      return_value_(""),
+      previously_focused_element_(nullptr) {
   UseCounter::Count(document, WebFeature::kDialogElement);
 }
 
@@ -130,6 +132,17 @@ void HTMLDialogElement::close(const String& return_value) {
     return_value_ = return_value;
 
   ScheduleCloseEvent();
+
+  // We should call focus() last since it will fire a focus event which could
+  // modify this element.
+  if (RuntimeEnabledFeatures::DialogFocusNewSpecBehaviorEnabled() &&
+      previously_focused_element_) {
+    FocusOptions focus_options;
+    focus_options.setPreventScroll(true);
+    Element* previously_focused_element = previously_focused_element_;
+    previously_focused_element_ = nullptr;
+    previously_focused_element->focus(&focus_options);
+  }
 }
 
 void HTMLDialogElement::SetIsModal(bool is_modal) {
@@ -158,6 +171,8 @@ void HTMLDialogElement::show() {
   // Element::isFocusable, which requires an up-to-date layout.
   GetDocument().UpdateStyleAndLayout(DocumentUpdateReason::kJavaScript);
 
+  previously_focused_element_ = GetDocument().FocusedElement();
+
   SetFocusForDialog(this);
 }
 
@@ -197,6 +212,8 @@ void HTMLDialogElement::showModal(ExceptionState& exception_state) {
   // is thrown away.
   InertSubtreesChanged(GetDocument());
 
+  previously_focused_element_ = GetDocument().FocusedElement();
+
   SetFocusForDialog(this);
 }
 
@@ -215,4 +232,9 @@ void HTMLDialogElement::DefaultEventHandler(Event& event) {
   HTMLElement::DefaultEventHandler(event);
 }
 
+void HTMLDialogElement::Trace(Visitor* visitor) const {
+  visitor->Trace(previously_focused_element_);
+  HTMLElement::Trace(visitor);
+}
+
 }  // namespace blink

third_party/blink/renderer/core/html/html_dialog_element.h

@@ -40,6 +40,8 @@ class HTMLDialogElement final : public HTMLElement {
  public:
   explicit HTMLDialogElement(Document&);
 
+  void Trace(Visitor*) const override;
+
   void close(const String& return_value = String());
   void show();
   void showModal(ExceptionState&);
@@ -60,6 +62,7 @@ class HTMLDialogElement final : public HTMLElement {
 
   bool is_modal_;
   String return_value_;
+  WeakMember<Element> previously_focused_element_;
 };
 
 }  // namespace blink

third_party/blink/renderer/platform/runtime_enabled_features.json5

@@ -718,6 +718,10 @@
       name: "DevicePosture",
       status: "experimental",
     },
+    {
+      name: "DialogFocusNewSpecBehavior",
+      status: "stable",
+    },
     {
       name: "DigitalGoods",
       origin_trial_feature_name: "DigitalGoods",

third_party/blink/web_tests/VirtualTestSuites

@@ -1050,5 +1050,12 @@
   "prefix": "playback_speed_button",
   "bases": ["media/controls"],
   "args": [ "--enable-features=PlaybackSpeedButton" ]
+  },
+  {
+    "prefix": "dialogfocus-old-behavior",
+    "bases": [
+      "external/wpt/html/semantics/interactive-elements/the-dialog-element"
+    ],
+    "args": ["--disable-features=DialogFocusNewSpecBehavior"]
   }
 ]

third_party/blink/web_tests/virtual/dialogfocus-old-behavior/README.md

@@ -0,0 +1,6 @@
+This is a virtual test suite for the *old* dialog element focusing steps, to
+ensure it respects the feature flag, in case this behavior needs to be disabled
+via Finch.
+
+Flag: --disable-features=DialogFocusNewSpecBehavior
+Bug: crbug.com/298078