LibWeb/HTML: Implement CloseWatcher::getEnabledState closer to spec

We previously had this implemented in an ad-hoc way, where we used a
boolean on the CloseWatcher instead of a proper function with steps.
This worked at the time, but causes problems with the current version
of the spec, so let's just implement it properly.

This commit consciously does not update the spec text, because it's
diverted quite a lot. That will happen in a subsequent commit.

Libraries/LibWeb/HTML/CloseWatcher.cpp

@@ -23,13 +23,19 @@ namespace Web::HTML {
 GC_DEFINE_ALLOCATOR(CloseWatcher);
 
 // https://html.spec.whatwg.org/multipage/interaction.html#establish-a-close-watcher
-GC::Ref<CloseWatcher> CloseWatcher::establish(HTML::Window& window)
+GC::Ref<CloseWatcher> CloseWatcher::establish(HTML::Window& window, GetEnabledState get_enabled_state)
 {
     // 1. Assert: window's associated Document is fully active.
     VERIFY(window.associated_document().is_fully_active());
 
-    // 2. Let closeWatcher be a new close watcher
-    auto close_watcher = window.realm().create<CloseWatcher>(window.realm());
+    // 2. Let closeWatcher be a new close watcher, with
+    //    window: window
+    //    cancel action: cancelAction
+    //    close action: closeAction
+    //    is running cancel action: false
+    //    get enabled state: getEnabledState
+    auto close_watcher = window.realm().create<CloseWatcher>(window.realm(), move(get_enabled_state));
+    // FIXME: cancelAction and closeAction are both set by the caller currently.
 
     // 3. Let manager be window's associated close watcher manager
     auto manager = window.close_watcher_manager();
@@ -55,8 +61,11 @@ WebIDL::ExceptionOr<GC::Ref<CloseWatcher>> CloseWatcher::construct_impl(JS::Real
     if (!window.associated_document().is_fully_active())
         return WebIDL::InvalidStateError::create(realm, "The document is not fully active."_utf16);
 
-    // 2. Let close_watcher be the result of establishing a close watcher
-    auto close_watcher = establish(window);
+    // 2. Let closeWatcher be the result of establishing a close watcher given this's relevant global object, with:
+    //    - cancelAction given canPreventClose being to return the result of firing an event named cancel at this, with the cancelable attribute initialized to canPreventClose.
+    //    - closeAction being to fire an event named close at this.
+    //    - getEnabledState being to return true.
+    auto close_watcher = establish(window, GC::create_function(realm.heap(), [] { return true; }));
 
     // 3. If options["signal"] exists, then:
     if (auto signal = options.signal) {
@@ -75,8 +84,9 @@ WebIDL::ExceptionOr<GC::Ref<CloseWatcher>> CloseWatcher::construct_impl(JS::Real
     return close_watcher;
 }
 
-CloseWatcher::CloseWatcher(JS::Realm& realm)
+CloseWatcher::CloseWatcher(JS::Realm& realm, GetEnabledState get_enabled_state)
     : DOM::EventTarget(realm)
+    , m_get_enabled_state(move(get_enabled_state))
 {
 }
 
@@ -136,6 +146,11 @@ bool CloseWatcher::request_close(bool require_history_action_activation)
     return true;
 }
 
+bool CloseWatcher::get_enabled_state() const
+{
+    return m_get_enabled_state->function().operator()();
+}
+
 // https://html.spec.whatwg.org/multipage/interaction.html#close-watcher-close
 void CloseWatcher::close()
 {
@@ -196,4 +211,10 @@ WebIDL::CallbackType* CloseWatcher::onclose()
     return event_handler_attribute(HTML::EventNames::close);
 }
 
+void CloseWatcher::visit_edges(Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_get_enabled_state);
+}
+
 }

Libraries/LibWeb/HTML/CloseWatcher.h

@@ -22,8 +22,10 @@ class CloseWatcher final : public DOM::EventTarget {
     GC_DECLARE_ALLOCATOR(CloseWatcher);
 
 public:
+    using GetEnabledState = GC::Ref<GC::Function<bool()>>;
+
     static WebIDL::ExceptionOr<GC::Ref<CloseWatcher>> construct_impl(JS::Realm&, CloseWatcherOptions const& = {});
-    [[nodiscard]] static GC::Ref<CloseWatcher> establish(HTML::Window&);
+    [[nodiscard]] static GC::Ref<CloseWatcher> establish(HTML::Window&, GetEnabledState);
 
     void request_close_for_bindings();
     void close();
@@ -31,8 +33,7 @@ public:
 
     bool request_close(bool require_history_action_activation);
 
-    bool get_enabled_state() const { return m_is_enabled; }
-    void set_enabled(bool enabled) { m_is_enabled = enabled; }
+    bool get_enabled_state() const;
 
     virtual ~CloseWatcher() override = default;
 
@@ -43,13 +44,14 @@ public:
     WebIDL::CallbackType* onclose();
 
 private:
-    CloseWatcher(JS::Realm&);
+    CloseWatcher(JS::Realm&, GetEnabledState);
 
     virtual void initialize(JS::Realm&) override;
+    virtual void visit_edges(Visitor&) override;
 
     bool m_is_running_cancel_action { false };
     bool m_is_active { true };
-    bool m_is_enabled { true };
+    GetEnabledState m_get_enabled_state;
 };
 
 }

Libraries/LibWeb/HTML/HTMLDialogElement.cpp

@@ -309,8 +309,7 @@ void HTMLDialogElement::request_close_the_dialog(Optional<String> return_value,
         return;
     }
     // 3. Set dialog's enable close watcher for requestClose() to true.
-    // AD-HOC: Implemented slightly differently to the spec, as the spec is unnecessarily complex.
-    m_close_watcher->set_enabled(true);
+    m_enable_close_watcher_for_request_close = true;
     // 4. If returnValue is not given, then set it to null.
     // 5. Set this's request close return value to returnValue.
     m_request_close_return_value = return_value;
@@ -319,10 +318,7 @@ void HTMLDialogElement::request_close_the_dialog(Optional<String> return_value,
     // 6. Request to close dialog's close watcher with false.
     m_close_watcher->request_close(false);
     // 7. Set dialog's enable close watcher for requestClose() to false.
-    // AD-HOC: Implemented slightly differently to the spec, as the spec is unnecessarily complex.
-    // FIXME: This should be set based on dialog closedby state, when implemented.
-    if (m_close_watcher)
-        m_close_watcher->set_enabled(m_is_modal);
+    m_enable_close_watcher_for_request_close = false;
 }
 
 // https://html.spec.whatwg.org/multipage/interactive-elements.html#dom-dialog-returnvalue
@@ -427,7 +423,6 @@ void HTMLDialogElement::set_close_watcher()
 
     // 3. Set dialog's close watcher to the result of establishing a close watcher given dialog's relevant global
     //    object, with:
-    m_close_watcher = CloseWatcher::establish(*document().window());
     //    - cancelAction given canPreventClose being to return the result of firing an event named cancel at dialog,
     //      with the cancelable attribute initialized to canPreventClose.
     auto cancel_callback_function = JS::NativeFunction::create(
@@ -441,7 +436,6 @@ void HTMLDialogElement::set_close_watcher()
         },
         0, Utf16FlyString {}, &realm());
     auto cancel_callback = realm().heap().allocate<WebIDL::CallbackType>(*cancel_callback_function, realm());
-    m_close_watcher->add_event_listener_without_options(HTML::EventNames::cancel, DOM::IDLEventListener::create(realm(), cancel_callback));
     //    - closeAction being to close the dialog given dialog, dialog's request close return value, and dialog's
     //      request close source element.
     auto close_callback_function = JS::NativeFunction::create(
@@ -452,12 +446,18 @@ void HTMLDialogElement::set_close_watcher()
         },
         0, Utf16FlyString {}, &realm());
     auto close_callback = realm().heap().allocate<WebIDL::CallbackType>(*close_callback_function, realm());
-    m_close_watcher->add_event_listener_without_options(HTML::EventNames::close, DOM::IDLEventListener::create(realm(), close_callback));
     //    - getEnabledState being to return true if dialog's enable close watcher for request close is true or dialog's
     //      computed closed-by state is not None; otherwise false.
-    // AD-HOC: Implemented slightly differently to the spec, as the spec is unnecessarily complex.
-    // FIXME: This should be set based on dialog closedby state, when implemented.
-    m_close_watcher->set_enabled(m_is_modal);
+    auto get_enabled_state = GC::create_function(heap(), [dialog = this] {
+        if (dialog->m_enable_close_watcher_for_request_close)
+            return true;
+        // FIXME: dialog's computed closed-by state is not None
+        return false;
+    });
+
+    m_close_watcher = CloseWatcher::establish(*document().window(), move(get_enabled_state));
+    m_close_watcher->add_event_listener_without_options(HTML::EventNames::cancel, DOM::IDLEventListener::create(realm(), cancel_callback));
+    m_close_watcher->add_event_listener_without_options(HTML::EventNames::close, DOM::IDLEventListener::create(realm(), close_callback));
 }
 
 // https://html.spec.whatwg.org/multipage/interactive-elements.html#dialog-focusing-steps

Libraries/LibWeb/HTML/HTMLDialogElement.h

@@ -70,6 +70,9 @@ private:
     GC::Ptr<DOM::Element> m_request_close_source_element;
     GC::Ptr<CloseWatcher> m_close_watcher;
 
+    // https://html.spec.whatwg.org/multipage/interactive-elements.html#enable-close-watcher-for-requestclose()
+    bool m_enable_close_watcher_for_request_close { false };
+
     // https://html.spec.whatwg.org/multipage/interactive-elements.html#dialog-toggle-task-tracker
     Optional<ToggleTaskTracker> m_dialog_toggle_task_tracker;
 

Libraries/LibWeb/HTML/HTMLElement.cpp

@@ -1409,9 +1409,8 @@ WebIDL::ExceptionOr<void> HTMLElement::show_popover(ThrowExceptions throw_except
         }
 
         // 6. Set element's popover close watcher to the result of establishing a close watcher given element's relevant global object, with:
-        m_popover_close_watcher = CloseWatcher::establish(*document.window());
         // - cancelAction being to return true.
-        // We simply don't add an event listener for the cancel action.
+        // NB: We simply don't add an event listener for the cancel action.
         // - closeAction being to hide a popover given element, true, true, false, and null.
         auto close_callback_function = JS::NativeFunction::create(
             realm(), [this](JS::VM&) {
@@ -1421,9 +1420,11 @@ WebIDL::ExceptionOr<void> HTMLElement::show_popover(ThrowExceptions throw_except
             },
             0, Utf16FlyString {}, &realm());
         auto close_callback = realm().heap().allocate<WebIDL::CallbackType>(*close_callback_function, realm());
-        m_popover_close_watcher->add_event_listener_without_options(HTML::EventNames::close, DOM::IDLEventListener::create(realm(), close_callback));
         // - getEnabledState being to return true.
-        m_popover_close_watcher->set_enabled(true);
+        auto get_enabled_state = GC::create_function(heap(), [] { return true; });
+
+        m_popover_close_watcher = CloseWatcher::establish(*document.window(), move(get_enabled_state));
+        m_popover_close_watcher->add_event_listener_without_options(HTML::EventNames::close, DOM::IDLEventListener::create(realm(), close_callback));
     }
     // FIXME: 19. Set element's previously focused element to null.
     // FIXME: 20. Let originallyFocusedElement be document's focused area of the document's DOM anchor.