From c866e63b50446b8732ade129044b5299ac2866fb Mon Sep 17 00:00:00 2001 From: Gingeh <39150378+Gingeh@users.noreply.github.com> Date: Tue, 14 Jan 2025 11:06:06 +1100 Subject: [PATCH] LibWeb: Ignore DOM state when hiding removed popovers Based on https://github.com/whatwg/html/pull/9457 with some changes made to catch up with the current spec --- Libraries/LibWeb/HTML/HTMLElement.cpp | 52 +++++++++++++-------------- Libraries/LibWeb/HTML/HTMLElement.h | 9 +++-- 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/Libraries/LibWeb/HTML/HTMLElement.cpp b/Libraries/LibWeb/HTML/HTMLElement.cpp index 579797f5653e..80b75d572648 100644 --- a/Libraries/LibWeb/HTML/HTMLElement.cpp +++ b/Libraries/LibWeb/HTML/HTMLElement.cpp @@ -934,10 +934,10 @@ void HTMLElement::adjust_computed_style(CSS::ComputedProperties& style) } // https://html.spec.whatwg.org/multipage/popover.html#check-popover-validity -WebIDL::ExceptionOr HTMLElement::check_popover_validity(ExpectedToBeShowing expected_to_be_showing, ThrowExceptions throw_exceptions, GC::Ptr expected_document) +WebIDL::ExceptionOr HTMLElement::check_popover_validity(ExpectedToBeShowing expected_to_be_showing, ThrowExceptions throw_exceptions, GC::Ptr expected_document, IgnoreDomState ignore_dom_state) { - // 1. If element's popover attribute is in the no popover state, then: - if (!popover().has_value()) { + // 1. If ignoreDomState is false and element's popover attribute is in the no popover state, then: + if (ignore_dom_state == IgnoreDomState::No && !popover().has_value()) { // 1.1. If throwExceptions is true, then throw a "NotSupportedError" DOMException. if (throw_exceptions == ThrowExceptions::Yes) return WebIDL::NotSupportedError::create(realm(), "Element is not a popover"_string); @@ -954,15 +954,15 @@ WebIDL::ExceptionOr HTMLElement::check_popover_validity(ExpectedToBeShowin } // 3. If any of the following are true: - // - element is not connected; + // - ignoreDomState is false and element is not connected; // - element's node document is not fully active; - // - expectedDocument is not null and element's node document is not expectedDocument; + // - ignoreDomState is false and expectedDocument is not null and element's node document is not expectedDocument; // - element is a dialog element and its is modal flage is set to true; or // - FIXME: element's fullscreen flag is set, // then: // 3.1 If throwExceptions is true, then throw an "InvalidStateError" DOMException. // 3.2 Return false. - if (!is_connected() || !document().is_fully_active() || (expected_document && &document() != expected_document) || (is(*this) && verify_cast(*this).is_modal())) { + if ((ignore_dom_state == IgnoreDomState::No && !is_connected()) || !document().is_fully_active() || (ignore_dom_state == IgnoreDomState::No && expected_document && &document() != expected_document) || (is(*this) && verify_cast(*this).is_modal())) { if (throw_exceptions == ThrowExceptions::Yes) return WebIDL::InvalidStateError::create(realm(), "Element is not in a valid state to show a popover"_string); return false; @@ -984,8 +984,8 @@ WebIDL::ExceptionOr HTMLElement::show_popover_for_bindings(ShowPopoverOpti // https://html.spec.whatwg.org/multipage/popover.html#show-popover WebIDL::ExceptionOr HTMLElement::show_popover(ThrowExceptions throw_exceptions, GC::Ptr invoker) { - // 1. If the result of running check popover validity given element, false, throwExceptions, and null is false, then return. - if (!TRY(check_popover_validity(ExpectedToBeShowing::No, throw_exceptions, nullptr))) + // 1. If the result of running check popover validity given element, false, throwExceptions, null and false is false, then return. + if (!TRY(check_popover_validity(ExpectedToBeShowing::No, throw_exceptions, nullptr, IgnoreDomState::No))) return {}; // 2. Let document be element's node document. @@ -1020,8 +1020,8 @@ WebIDL::ExceptionOr HTMLElement::show_popover(ThrowExceptions throw_except return {}; } - // 9. If the result of running check popover validity given element, false, throwExceptions, and document is false, then run cleanupShowingFlag and return. - if (!TRY(check_popover_validity(ExpectedToBeShowing::No, throw_exceptions, nullptr))) { + // 9. If the result of running check popover validity given element, false, throwExceptions, document, and false is false, then run cleanupShowingFlag and return. + if (!TRY(check_popover_validity(ExpectedToBeShowing::No, throw_exceptions, document, IgnoreDomState::No))) { cleanup_showing_flag(); return {}; } @@ -1036,7 +1036,7 @@ WebIDL::ExceptionOr HTMLElement::show_popover(ThrowExceptions throw_except // FIXME: 11.3. If ancestor is null, then set ancestor to document. // FIXME: 11.4. Run hide all popovers until given ancestor, false, and not nestedShow. // FIXME: 11.5. If originalType is not equal to the value of element's popover attribute, then throw a "InvalidStateError" DOMException. - // FIXME: 11.6. If the result of running check popover validity given element, false, throwExceptions, and document is false, then run cleanupShowingFlag and return. + // FIXME: 11.6. If the result of running check popover validity given element, false, throwExceptions, document, and false is false, then run cleanupShowingFlag and return. // FIXME: 11.7. If the result of running topmost auto popover on document is null, then set shouldRestoreFocus to true. // 11.8. 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()); @@ -1045,7 +1045,7 @@ WebIDL::ExceptionOr HTMLElement::show_popover(ThrowExceptions throw_except // - closeAction being to hide a popover given element, true, true, and false. auto close_callback_function = JS::NativeFunction::create( realm(), [this](JS::VM&) { - MUST(hide_popover(FocusPreviousElement::Yes, FireEvents::Yes, ThrowExceptions::No)); + MUST(hide_popover(FocusPreviousElement::Yes, FireEvents::Yes, ThrowExceptions::No, IgnoreDomState::No)); return JS::js_undefined(); }, @@ -1085,15 +1085,15 @@ WebIDL::ExceptionOr HTMLElement::show_popover(ThrowExceptions throw_except // https://html.spec.whatwg.org/multipage/popover.html#dom-hidepopover WebIDL::ExceptionOr HTMLElement::hide_popover_for_bindings() { - // The hidePopover() method steps are to run the hide popover algorithm given this, true, true, and true. - return hide_popover(FocusPreviousElement::Yes, FireEvents::Yes, ThrowExceptions::Yes); + // The hidePopover() method steps are to run the hide popover algorithm given this, true, true, true, and false. + return hide_popover(FocusPreviousElement::Yes, FireEvents::Yes, ThrowExceptions::Yes, IgnoreDomState::No); } // https://html.spec.whatwg.org/multipage/popover.html#hide-popover-algorithm -WebIDL::ExceptionOr HTMLElement::hide_popover(FocusPreviousElement, FireEvents fire_events, ThrowExceptions throw_exceptions) +WebIDL::ExceptionOr HTMLElement::hide_popover(FocusPreviousElement, FireEvents fire_events, ThrowExceptions throw_exceptions, IgnoreDomState ignore_dom_state) { - // 1. If the result of running check popover validity given element, true, throwExceptions, and null is false, then return. - if (!TRY(check_popover_validity(ExpectedToBeShowing::Yes, throw_exceptions, nullptr))) + // 1. If the result of running check popover validity given element, true, throwExceptions, null and ignoreDomState is false, then return. + if (!TRY(check_popover_validity(ExpectedToBeShowing::Yes, throw_exceptions, nullptr, ignore_dom_state))) return {}; // 2. Let document be element's node document. @@ -1126,7 +1126,7 @@ WebIDL::ExceptionOr HTMLElement::hide_popover(FocusPreviousElement, FireEv // 7. If element's popover attribute is in the auto state, then: if (popover().has_value() && popover().value() == "auto"sv) { // FIXME: 7.1. Run hide all popovers until given element, focusPreviousElement, and fireEvents. - // FIXME: 7.2. If the result of running check popover validity given element, true, and throwExceptions is false, then run cleanupSteps and return. + // FIXME: 7.2. If the result of running check popover validity given element, true, throwExceptions, and ignoreDomState is false, then run cleanupSteps and return. } // FIXME: 8. Let autoPopoverListContainsElement be true if document's showing auto popover list's last item is element, otherwise false. @@ -1143,8 +1143,8 @@ WebIDL::ExceptionOr HTMLElement::hide_popover(FocusPreviousElement, FireEv // FIXME: 10.2. If autoPopoverListContainsElement is true and document's showing auto popover list's last item is not element, then run hide all popovers until given element, focusPreviousElement, and false. - // 10.3. If the result of running check popover validity given element, true, throwExceptions, and null is false, then run cleanupSteps and return. - if (!TRY(check_popover_validity(ExpectedToBeShowing::Yes, throw_exceptions, nullptr))) { + // 10.3. If the result of running check popover validity given element, true, throwExceptions, null, and ignoreDomState is false, then run cleanupSteps and return. + if (!TRY(check_popover_validity(ExpectedToBeShowing::Yes, throw_exceptions, nullptr, ignore_dom_state))) { cleanup_steps(); return {}; } @@ -1193,9 +1193,9 @@ WebIDL::ExceptionOr HTMLElement::toggle_popover(TogglePopoverOptionsOrForc invoker = options.source; }); - // 5. If this's popover visibility state is showing, and force is null or false, then run the hide popover algorithm given this, true, true, and true. + // 5. If this's popover visibility state is showing, and force is null or false, then run the hide popover algorithm given this, true, true, true, and false. if (popover_visibility_state() == PopoverVisibilityState::Showing && (!force.has_value() || !force.value())) - TRY(hide_popover(FocusPreviousElement::Yes, FireEvents::Yes, ThrowExceptions::Yes)); + TRY(hide_popover(FocusPreviousElement::Yes, FireEvents::Yes, ThrowExceptions::Yes, IgnoreDomState::No)); // 6. Otherwise, if force is not present or true, then run show popover given this true, and invoker. else if (!force.has_value() || force.value()) TRY(show_popover(ThrowExceptions::Yes, invoker)); @@ -1203,8 +1203,8 @@ WebIDL::ExceptionOr HTMLElement::toggle_popover(TogglePopoverOptionsOrForc else { // 7.1 Let expectedToBeShowing be true if this's popover visibility state is showing; otherwise false. ExpectedToBeShowing expected_to_be_showing = popover_visibility_state() == PopoverVisibilityState::Showing ? ExpectedToBeShowing::Yes : ExpectedToBeShowing::No; - // 7.2 Run check popover validity given expectedToBeShowing, true, and null. - TRY(check_popover_validity(expected_to_be_showing, ThrowExceptions::Yes, nullptr)); + // 7.2 Run check popover validity given expectedToBeShowing, true, null, and false. + TRY(check_popover_validity(expected_to_be_showing, ThrowExceptions::Yes, nullptr, IgnoreDomState::No)); } // 8. Return true if this's popover visibility state is showing; otherwise false. return popover_visibility_state() == PopoverVisibilityState::Showing; @@ -1281,9 +1281,9 @@ void HTMLElement::removed_from(Node* old_parent) { Element::removed_from(old_parent); - // If removedNode's popover attribute is not in the no popover state, then run the hide popover algorithm given removedNode, false, false, and false. + // If removedNode's popover attribute is not in the no popover state, then run the hide popover algorithm given removedNode, false, false, false, and true. if (popover().has_value()) - MUST(hide_popover(FocusPreviousElement::No, FireEvents::No, ThrowExceptions::No)); + MUST(hide_popover(FocusPreviousElement::No, FireEvents::No, ThrowExceptions::No, IgnoreDomState::Yes)); } // https://html.spec.whatwg.org/multipage/interaction.html#dom-accesskeylabel diff --git a/Libraries/LibWeb/HTML/HTMLElement.h b/Libraries/LibWeb/HTML/HTMLElement.h index 2f7699e60a71..a42aeaac6463 100644 --- a/Libraries/LibWeb/HTML/HTMLElement.h +++ b/Libraries/LibWeb/HTML/HTMLElement.h @@ -60,6 +60,11 @@ enum class ExpectedToBeShowing { No, }; +enum class IgnoreDomState { + Yes, + No, +}; + class HTMLElement : public DOM::Element , public HTML::GlobalEventHandlers @@ -129,7 +134,7 @@ class HTMLElement WebIDL::ExceptionOr toggle_popover(TogglePopoverOptionsOrForceBoolean const&); WebIDL::ExceptionOr show_popover(ThrowExceptions throw_exceptions, GC::Ptr invoker); - WebIDL::ExceptionOr hide_popover(FocusPreviousElement focus_previous_element, FireEvents fire_events, ThrowExceptions throw_exceptions); + WebIDL::ExceptionOr hide_popover(FocusPreviousElement focus_previous_element, FireEvents fire_events, ThrowExceptions throw_exceptions, IgnoreDomState ignore_dom_state); protected: HTMLElement(DOM::Document&, DOM::QualifiedName); @@ -157,7 +162,7 @@ class HTMLElement GC::Ptr m_labels; - WebIDL::ExceptionOr check_popover_validity(ExpectedToBeShowing expected_to_be_showing, ThrowExceptions throw_exceptions, GC::Ptr); + WebIDL::ExceptionOr check_popover_validity(ExpectedToBeShowing expected_to_be_showing, ThrowExceptions throw_exceptions, GC::Ptr, IgnoreDomState ignore_dom_state); void queue_a_popover_toggle_event_task(String old_state, String new_state);