From 346eb026591cca3dca2a90969d7da6c47252f3c6 Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Wed, 27 Sep 2023 23:23:00 -0600 Subject: [PATCH] LibWeb: Remove last substantial FIXMEs from Navigation API --- Userland/Libraries/LibWeb/HTML/Navigation.cpp | 45 ++++++++++++------- .../LibWeb/HTML/TraversableNavigable.cpp | 33 +++++--------- .../LibWeb/HTML/TraversableNavigable.h | 22 ++++----- 3 files changed, 51 insertions(+), 49 deletions(-) diff --git a/Userland/Libraries/LibWeb/HTML/Navigation.cpp b/Userland/Libraries/LibWeb/HTML/Navigation.cpp index b482e7e3c5..363a4e7506 100644 --- a/Userland/Libraries/LibWeb/HTML/Navigation.cpp +++ b/Userland/Libraries/LibWeb/HTML/Navigation.cpp @@ -259,14 +259,11 @@ WebIDL::ExceptionOr Navigation::navigate(String url, Navigatio // FIXME: Fix spec typo here // NOTE: Unlike location.assign() and friends, which are exposed across origin-domain boundaries, // navigation.navigate() can only be accessed by code with direct synchronous access to the - /// window.navigation property. Thus, we avoid the complications about attributing the source document + // window.navigation property. Thus, we avoid the complications about attributing the source document // of the navigation, and we don't need to deal with the allowed by sandboxing to navigate check and its // acccompanying exceptionsEnabled flag. We just treat all navigations as if they come from the Document // corresponding to this Navigation object itself (i.e., document). - [[maybe_unused]] auto history_handling_behavior = to_history_handling_behavior(options.history); - // FIXME: Actually call navigate once Navigables are implemented enough to guarantee a node navigable on - // an active document that's not being unloaded. - // document.navigable().navigate(url, document, history behavior, state) + TRY(document.navigable()->navigate({ .url = url_record, .source_document = document, .history_handling = options.history, .navigation_api_state = move(serialized_state) })); // 11. If this's upcoming non-traverse API method tracker is apiMethodTracker, then: // NOTE: If the upcoming non-traverse API method tracker is still apiMethodTracker, this means that the navigate @@ -330,9 +327,8 @@ WebIDL::ExceptionOr Navigation::reload(NavigationReloadOptions auto api_method_tracker = maybe_set_the_upcoming_non_traverse_api_method_tracker(info, serialized_state); // 9. Reload document's node navigable with navigationAPIState set to serializedState. - // FIXME: Actually call reload once Navigables are implemented enough to guarantee a node navigable on - // an active document that's not being unloaded. - // document.navigable().reload(state) + // FIXME: Pass serialized_state to reload + document.navigable()->reload(); return navigation_api_method_tracker_derived_result(api_method_tracker); } @@ -653,7 +649,7 @@ WebIDL::ExceptionOr Navigation::perform_a_navigation_api_trave auto source_snapshot_params = document.snapshot_source_snapshot_params(); // 12. Append the following session history traversal steps to traversable: - traversable->append_session_history_traversal_steps([key, api_method_tracker, navigable, source_snapshot_params, this] { + traversable->append_session_history_traversal_steps([key, api_method_tracker, navigable, source_snapshot_params, traversable, this] { // 1. Let navigableSHEs be the result of getting session history entries given navigable. auto navigable_shes = navigable->get_session_history_entries(); @@ -669,6 +665,7 @@ WebIDL::ExceptionOr Navigation::perform_a_navigation_api_trave // to reject the finished promise for apiMethodTracker with an "InvalidStateError" DOMException. queue_global_task(HTML::Task::Source::NavigationAndTraversal, relevant_global_object(*this), [this, api_method_tracker] { auto& reject_realm = relevant_realm(*this); + TemporaryExecutionContext execution_context { relevant_settings_object(*this) }; WebIDL::reject_promise(reject_realm, api_method_tracker->finished_promise, WebIDL::InvalidStateError::create(reject_realm, "Cannot traverse with stale session history entry"_fly_string)); }); @@ -684,22 +681,36 @@ WebIDL::ExceptionOr Navigation::perform_a_navigation_api_trave if (target_she == navigable->active_session_history_entry()) return; - // FIXME: 4. Let result be the result of applying the traverse history step given by targetSHE's step to traversable, + // 4. Let result be the result of applying the traverse history step given by targetSHE's step to traversable, // given sourceSnapshotParams, navigable, and "none". - (void)source_snapshot_params; + auto result = traversable->apply_the_traverse_history_step(target_she->step.get(), source_snapshot_params, navigable, UserNavigationInvolvement::None); // NOTE: When result is "canceled-by-beforeunload" or "initiator-disallowed", the navigate event was never fired, // aborting the ongoing navigation would not be correct; it would result in a navigateerror event without a // preceding navigate event. In the "canceled-by-navigate" case, navigate is fired, but the inner navigate event // firing algorithm will take care of aborting the ongoing navigation. - // FIXME: 5. If result is "canceled-by-beforeunload", then queue a global task on the navigation and traversal task source - // given navigation's relevant global object to reject the finished promise for apiMethodTracker with a - // new "AbortError"DOMException created in navigation's relevant realm. + // 5. If result is "canceled-by-beforeunload", then queue a global task on the navigation and traversal task source + // given navigation's relevant global object to reject the finished promise for apiMethodTracker with a + // new "AbortError" DOMException created in navigation's relevant realm. + auto& realm = relevant_realm(*this); + auto& global = relevant_global_object(*this); + if (result == TraversableNavigable::HistoryStepResult::CanceledByBeforeUnload) { + queue_global_task(Task::Source::NavigationAndTraversal, global, [this, api_method_tracker, &realm] { + TemporaryExecutionContext execution_context { relevant_settings_object(*this) }; + reject_the_finished_promise(api_method_tracker, WebIDL::AbortError::create(realm, "Navigation cancelled by beforeunload"_fly_string)); + }); + } - // FIXME: 6. If result is "initiator-disallowed", then queue a global task on the navigation and traversal task source - // given navigation's relevant global object to reject the finished promise for apiMethodTracker with a - // new "SecurityError" DOMException created in navigation's relevant realm. + // 6. If result is "initiator-disallowed", then queue a global task on the navigation and traversal task source + // given navigation's relevant global object to reject the finished promise for apiMethodTracker with a + // new "SecurityError" DOMException created in navigation's relevant realm. + if (result == TraversableNavigable::HistoryStepResult::InitiatorDisallowed) { + queue_global_task(Task::Source::NavigationAndTraversal, global, [this, api_method_tracker, &realm] { + TemporaryExecutionContext execution_context { relevant_settings_object(*this) }; + reject_the_finished_promise(api_method_tracker, WebIDL::SecurityError::create(realm, "Navigation disallowed from this origin"_fly_string)); + }); + } }); // 13. Return a navigation API method tracker-derived result for apiMethodTracker. diff --git a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp index 3e7b0f26d4..eefb0bf075 100644 --- a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp +++ b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp @@ -320,16 +320,11 @@ Vector> TraversableNavigable::get_all_navigables_that_migh TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_step( int step, bool check_for_cancelation, - bool fire_navigate_event_on_commit, Optional source_snapshot_params, JS::GCPtr initiator_to_check, Optional user_involvement_for_navigate_events) { - // FIXME: fireNavigateEventOnCommit is unused in this algorithm (https://github.com/whatwg/html/issues/9800) - (void)fire_navigate_event_on_commit; - auto& vm = this->vm(); - // FIXME: 1. Assert: This is running within traversable's session history traversal queue. // 2. Let targetStep be the result of getting the used step given traversable and step. @@ -765,39 +760,35 @@ void TraversableNavigable::traverse_the_history_by_delta(int delta, Optional source_snapshot_params, JS::GCPtr initiator_to_check, UserNavigationInvolvement user_involvement) +TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_traverse_history_step(int step, Optional source_snapshot_params, JS::GCPtr initiator_to_check, UserNavigationInvolvement user_involvement) { - // 1. Return the result of applying the history step step to traversable given true, true, sourceSnapshotParams, initiatorToCheck, and userInvolvement. - // FIXME: Return result of history application. - apply_the_history_step(step, true, true, move(source_snapshot_params), initiator_to_check, user_involvement); + // 1. Return the result of applying the history step step to traversable given true, sourceSnapshotParams, initiatorToCheck, and userInvolvement. + return apply_the_history_step(step, true, move(source_snapshot_params), initiator_to_check, user_involvement); } // https://html.spec.whatwg.org/multipage/document-sequences.html#close-a-top-level-traversable diff --git a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.h b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.h index b51598e689..6fa5d46345 100644 --- a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.h +++ b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.h @@ -40,10 +40,17 @@ public: }; HistoryObjectLengthAndIndex get_the_history_object_length_and_index(int) const; - void apply_the_traverse_history_step(int, Optional, JS::GCPtr, UserNavigationInvolvement); - void apply_the_reload_history_step(); - void apply_the_push_or_replace_history_step(int step); - void update_for_navigable_creation_or_destruction(); + enum class HistoryStepResult { + InitiatorDisallowed, + CanceledByBeforeUnload, + CanceledByNavigate, + Applied, + }; + + HistoryStepResult apply_the_traverse_history_step(int, Optional, JS::GCPtr, UserNavigationInvolvement); + HistoryStepResult apply_the_reload_history_step(); + HistoryStepResult apply_the_push_or_replace_history_step(int step); + HistoryStepResult update_for_navigable_creation_or_destruction(); int get_the_used_step(int step) const; Vector> get_all_navigables_whose_current_session_history_entry_will_change_or_reload(int) const; @@ -75,17 +82,10 @@ private: virtual void visit_edges(Cell::Visitor&) override; - enum class HistoryStepResult { - InitiatorDisallowed, - CanceledByBeforeUnload, - CanceledByNavigate, - Applied, - }; // FIXME: Fix spec typo cancelation --> cancellation HistoryStepResult apply_the_history_step( int step, bool check_for_cancelation, - bool fire_navigate_event_on_commit, Optional, JS::GCPtr initiator_to_check, Optional user_involvement_for_navigate_events);