From d80d95199110a302e75abce3921e397d2eead2e1 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Wed, 9 Oct 2024 06:36:15 -0400 Subject: [PATCH] LibWeb: Restore check to prevent closing a traversable twice We removed this check as a workaround for a spec issue that was resolved in: https://github.com/whatwg/html/commit/3a8303ece44ed509928be626a6a65639fd --- .../LibWeb/HTML/TraversableNavigable.cpp | 22 ++++++++++++------- .../LibWeb/HTML/TraversableNavigable.h | 1 + Userland/Libraries/LibWeb/HTML/Window.cpp | 4 ++-- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp index 75c54595ad2..7ffccdebb49 100644 --- a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp +++ b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp @@ -1174,22 +1174,28 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_traverse // https://html.spec.whatwg.org/multipage/document-sequences.html#close-a-top-level-traversable void TraversableNavigable::close_top_level_traversable() +{ + // 1. If traversable's is closing is true, then return. + if (is_closing()) + return; + + // 2. Definitely close traversable. + definitely_close_top_level_traversable(); +} + +// https://html.spec.whatwg.org/multipage/document-sequences.html#definitely-close-a-top-level-traversable +void TraversableNavigable::definitely_close_top_level_traversable() { VERIFY(is_top_level_traversable()); - // 1. If traversable's is closing is true, then return. - // FIXME: Spec-issue: The only place in the spec that sets the `is closing` flag to true is `window.close`, and it - // does so immediately before invoking this method. So it does not make sense to return early here. - // https://github.com/whatwg/html/issues/10678 - - // 2. Let toUnload be traversable's active document's inclusive descendant navigables. + // 1. Let toUnload be traversable's active document's inclusive descendant navigables. auto to_unload = active_document()->inclusive_descendant_navigables(); - // If the result of checking if unloading is canceled for toUnload is true, then return. + // 2. If the result of checking if unloading is canceled for toUnload is true, then return. if (check_if_unloading_is_canceled(to_unload) != CheckIfUnloadingIsCanceledResult::Continue) return; - // 4. Append the following session history traversal steps to traversable: + // 3. Append the following session history traversal steps to traversable: append_session_history_traversal_steps(JS::create_heap_function(heap(), [this] { // 1. Let afterAllUnloads be an algorithm step which destroys traversable. auto after_all_unloads = JS::create_heap_function(heap(), [this] { diff --git a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.h b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.h index 72f2a720523..ebefda9a999 100644 --- a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.h +++ b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.h @@ -78,6 +78,7 @@ public: void traverse_the_history_by_delta(int delta, Optional source_document = {}); void close_top_level_traversable(); + void definitely_close_top_level_traversable(); void destroy_top_level_traversable(); void append_session_history_traversal_steps(JS::NonnullGCPtr> steps) diff --git a/Userland/Libraries/LibWeb/HTML/Window.cpp b/Userland/Libraries/LibWeb/HTML/Window.cpp index 0d90029779d..a6c7b434ce9 100644 --- a/Userland/Libraries/LibWeb/HTML/Window.cpp +++ b/Userland/Libraries/LibWeb/HTML/Window.cpp @@ -826,9 +826,9 @@ void Window::close() // 1. Set thisTraversable's is closing to true. traversable->set_closing(true); - // 2. Queue a task on the DOM manipulation task source to close thisTraversable. + // 2. Queue a task on the DOM manipulation task source to definitely close thisTraversable. HTML::queue_global_task(HTML::Task::Source::DOMManipulation, incumbent_global_object, JS::create_heap_function(heap(), [traversable] { - verify_cast(*traversable).close_top_level_traversable(); + verify_cast(*traversable).definitely_close_top_level_traversable(); })); } }