diff --git a/Libraries/LibWeb/HTML/HTMLElement.cpp b/Libraries/LibWeb/HTML/HTMLElement.cpp index 9bb2c619af5..93207f6f4f1 100644 --- a/Libraries/LibWeb/HTML/HTMLElement.cpp +++ b/Libraries/LibWeb/HTML/HTMLElement.cpp @@ -997,10 +997,11 @@ 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) +// https://whatpr.org/html/9457/popover.html#check-popover-validity +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); @@ -1017,15 +1018,19 @@ 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) && as(*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) && as(*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; @@ -1045,10 +1050,11 @@ WebIDL::ExceptionOr HTMLElement::show_popover_for_bindings(ShowPopoverOpti } // https://html.spec.whatwg.org/multipage/popover.html#show-popover +// https://whatpr.org/html/9457/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. @@ -1085,8 +1091,8 @@ WebIDL::ExceptionOr HTMLElement::show_popover(ThrowExceptions throw_except return {}; } - // 10. 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))) { + // 10. 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 {}; } @@ -1123,7 +1129,7 @@ WebIDL::ExceptionOr HTMLElement::show_popover(ThrowExceptions throw_except // FIXME: 18.2. If originalType is not equal to the value of element's popover attribute, then: // FIXME: 18.2.1. If throwExceptions is true, then throw a "InvalidStateError" DOMException. // FIXME: 18.2.2. Return. - // FIXME: 18.3. If the result of running check popover validity given element, false, throwExceptions, and document is false, then run cleanupShowingFlag and return. + // FIXME: 18.3. If the result of running check popover validity given element, false, throwExceptions, document, and false is false, then run cleanupShowingFlag and return. // FIXME: 18.4. If the result of running topmost auto or hint popover on document is null, then set shouldRestoreFocus to true. // FIXME: 18.5. If stackToAppendTo is "auto": // FIXME: 18.5.1. Assert: document's showing auto popover list does not contain element. @@ -1139,7 +1145,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(); }, @@ -1168,17 +1174,19 @@ WebIDL::ExceptionOr HTMLElement::show_popover(ThrowExceptions throw_except } // https://html.spec.whatwg.org/multipage/popover.html#dom-hidepopover +// https://whatpr.org/html/9457/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) +// https://whatpr.org/html/9457/popover.html#hide-popover-algorithm +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. @@ -1211,7 +1219,7 @@ WebIDL::ExceptionOr HTMLElement::hide_popover(FocusPreviousElement, FireEv // 7. If element's popover attribute is in the auto state FIXME: or the hint 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. @@ -1228,8 +1236,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 {}; } @@ -1262,6 +1270,7 @@ WebIDL::ExceptionOr HTMLElement::hide_popover(FocusPreviousElement, FireEv } // https://html.spec.whatwg.org/multipage/popover.html#dom-togglepopover +// https://whatpr.org/html/9457/popover.html#dom-togglepopover WebIDL::ExceptionOr HTMLElement::toggle_popover(TogglePopoverOptionsOrForceBoolean const& options) { // 1. Let force be null. @@ -1280,9 +1289,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)); @@ -1290,8 +1299,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; @@ -1368,9 +1377,10 @@ void HTMLElement::removed_from(Node* old_parent, Node& old_root) { Element::removed_from(old_parent, old_root); - // If removedNode's popover attribute is not in the no popover state, then run the hide popover algorithm given removedNode, false, false, and false. + // https://whatpr.org/html/9457/infrastructure.html#dom-trees:concept-node-remove-ext + // 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 942a33d1f82..05073a971e6 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 @@ -131,9 +136,9 @@ public: WebIDL::ExceptionOr hide_popover_for_bindings(); WebIDL::ExceptionOr toggle_popover(TogglePopoverOptionsOrForceBoolean const&); - 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); 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); diff --git a/Libraries/LibWeb/HTML/PopoverInvokerElement.cpp b/Libraries/LibWeb/HTML/PopoverInvokerElement.cpp index a2211316646..84883af97e5 100644 --- a/Libraries/LibWeb/HTML/PopoverInvokerElement.cpp +++ b/Libraries/LibWeb/HTML/PopoverInvokerElement.cpp @@ -34,6 +34,7 @@ void PopoverInvokerElement::visit_edges(JS::Cell::Visitor& visitor) } // https://html.spec.whatwg.org/multipage/popover.html#popover-target-attribute-activation-behavior +// https://whatpr.org/html/9457/popover.html#popover-target-attribute-activation-behavior void PopoverInvokerElement::popover_target_activation_behaviour(GC::Ref node, GC::Ref event_target) { // To run the popover target attribute activation behavior given a Node node and a Node eventTarget: @@ -60,14 +61,14 @@ void PopoverInvokerElement::popover_target_activation_behaviour(GC::Refpopover_visibility_state() == HTMLElement::PopoverVisibilityState::Hidden) return; - // 6. If popover's popover visibility state is showing, then run the hide popover algorithm given popover, true, true, and false. + // 6. If popover's popover visibility state is showing, then run the hide popover algorithm given popover, true, true, false, and false. if (popover->popover_visibility_state() == HTMLElement::PopoverVisibilityState::Showing) { - MUST(popover->hide_popover(FocusPreviousElement::Yes, FireEvents::Yes, ThrowExceptions::No)); + MUST(popover->hide_popover(FocusPreviousElement::Yes, FireEvents::Yes, ThrowExceptions::No, IgnoreDomState::No)); } - // 7. Otherwise, if popover's popover visibility state is hidden and the result of running check popover validity given popover, false, false, and null is true, then run show popover given popover, false, and node. + // 7. Otherwise, if popover's popover visibility state is hidden and the result of running check popover validity given popover, false, false, null, and false is true, then run show popover given popover, false, and node. else if (popover->popover_visibility_state() == HTMLElement::PopoverVisibilityState::Hidden - && MUST(popover->check_popover_validity(ExpectedToBeShowing::No, ThrowExceptions::No, nullptr))) { + && MUST(popover->check_popover_validity(ExpectedToBeShowing::No, ThrowExceptions::No, nullptr, IgnoreDomState::No))) { MUST(popover->show_popover(ThrowExceptions::No, as(*node))); } } diff --git a/Tests/LibWeb/Text/expected/popover-crashes.txt b/Tests/LibWeb/Text/expected/popover-crashes.txt index eb284037599..e358a96903c 100644 --- a/Tests/LibWeb/Text/expected/popover-crashes.txt +++ b/Tests/LibWeb/Text/expected/popover-crashes.txt @@ -1 +1,2 @@ Didn't crash when showing recently hidden popover +Didn't crash when removing visible popover diff --git a/Tests/LibWeb/Text/input/popover-crashes.html b/Tests/LibWeb/Text/input/popover-crashes.html index a387b1bf779..4df1906348e 100644 --- a/Tests/LibWeb/Text/input/popover-crashes.html +++ b/Tests/LibWeb/Text/input/popover-crashes.html @@ -8,5 +8,7 @@ pop.hidePopover(); pop.showPopover(); println("Didn't crash when showing recently hidden popover"); + pop.remove(); + println("Didn't crash when removing visible popover"); });