From 44f7d7406c7aff4a300a71706eb469437d468b2d Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Tue, 10 Oct 2023 16:05:38 +0200 Subject: [PATCH] LibWeb: Use struct to pass Navigable::navigate() params Using structs makes the navigate() calls looks cleaner. No change in behavior is intended. --- Userland/Libraries/LibWeb/DOM/Document.cpp | 2 +- .../Libraries/LibWeb/HTML/HTMLFormElement.cpp | 8 +++++- .../LibWeb/HTML/HTMLHyperlinkElementUtils.cpp | 2 +- .../LibWeb/HTML/HTMLObjectElement.cpp | 4 ++- Userland/Libraries/LibWeb/HTML/Location.cpp | 5 +++- Userland/Libraries/LibWeb/HTML/Navigable.cpp | 22 ++++++++-------- Userland/Libraries/LibWeb/HTML/Navigable.h | 25 +++++++++++-------- .../LibWeb/HTML/NavigableContainer.cpp | 6 ++++- .../LibWeb/HTML/TraversableNavigable.cpp | 4 ++- Userland/Libraries/LibWeb/HTML/Window.cpp | 4 +-- .../Libraries/LibWeb/Page/EventHandler.cpp | 2 +- Userland/Libraries/LibWeb/Page/Page.cpp | 6 +++-- 12 files changed, 56 insertions(+), 34 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index a8be077ecc..74780f684a 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -3492,7 +3492,7 @@ void Document::shared_declarative_refresh_steps(StringView input, JS::GCPtrnavigate(url_record, *this)); + MUST(navigable()->navigate({ .url = url_record, .source_document = *this })); }).release_value_but_fixme_should_propagate_errors(); // For the purposes of the previous paragraph, a refresh is said to have come due as soon as the later of the diff --git a/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp index d5de63e007..6c242049c2 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp @@ -799,7 +799,13 @@ void HTMLFormElement::plan_to_navigate_to(AK::URL url, Variantnavigate(url, this->document(), post_resource, nullptr, false, to_navigation_history_behavior(history_handling), {}, {}, referrer_policy)); + MUST(target_navigable->navigate({ .url = url, + .source_document = this->document(), + .document_resource = post_resource, + .response = nullptr, + .exceptions_enabled = false, + .history_handling = to_navigation_history_behavior(history_handling), + .referrer_policy = referrer_policy })); }); // 5. Set the form's planned navigation to the just-queued task. diff --git a/Userland/Libraries/LibWeb/HTML/HTMLHyperlinkElementUtils.cpp b/Userland/Libraries/LibWeb/HTML/HTMLHyperlinkElementUtils.cpp index 0e45f7c43a..353860d18a 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLHyperlinkElementUtils.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLHyperlinkElementUtils.cpp @@ -519,7 +519,7 @@ void HTMLHyperlinkElementUtils::follow_the_hyperlink(Optional // FIXME: 12. If subject's link types includes the noreferrer keyword, then set referrerPolicy to "no-referrer". // 13. Navigate targetNavigable to url using subject's node document, with referrerPolicy set to referrerPolicy. - MUST(target_navigable->navigate(url, hyperlink_element_utils_document())); + MUST(target_navigable->navigate({ .url = url, .source_document = hyperlink_element_utils_document() })); } } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp index ad3e73c602..7f483304ca 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp @@ -258,7 +258,9 @@ void HTMLObjectElement::run_object_representation_handler_steps(Optionalurl(); url != "about:blank"sv) - MUST(m_content_navigable->navigate(url, document(), Empty {}, nullptr, false, Bindings::NavigationHistoryBehavior::Replace)); + MUST(m_content_navigable->navigate({ .url = url, + .source_document = document(), + .history_handling = Bindings::NavigationHistoryBehavior::Replace })); // The object element represents its nested browsing context. run_object_representation_completed_steps(Representation::NestedBrowsingContext); diff --git a/Userland/Libraries/LibWeb/HTML/Location.cpp b/Userland/Libraries/LibWeb/HTML/Location.cpp index 78e3a4126a..f55cdc2055 100644 --- a/Userland/Libraries/LibWeb/HTML/Location.cpp +++ b/Userland/Libraries/LibWeb/HTML/Location.cpp @@ -86,7 +86,10 @@ WebIDL::ExceptionOr Location::navigate(AK::URL url, HistoryHandlingBehavio } // 4. Navigate navigable to url using sourceDocument, with exceptionsEnabled set to true and historyHandling set to historyHandling. - TRY(navigable->navigate(url, source_document, {}, nullptr, true, to_navigation_history_behavior(history_handling))); + TRY(navigable->navigate({ .url = url, + .source_document = source_document, + .exceptions_enabled = true, + .history_handling = to_navigation_history_behavior(history_handling) })); return {}; } diff --git a/Userland/Libraries/LibWeb/HTML/Navigable.cpp b/Userland/Libraries/LibWeb/HTML/Navigable.cpp index 52a61723d7..b451231311 100644 --- a/Userland/Libraries/LibWeb/HTML/Navigable.cpp +++ b/Userland/Libraries/LibWeb/HTML/Navigable.cpp @@ -1075,18 +1075,18 @@ WebIDL::ExceptionOr Navigable::populate_session_history_entry_document( // and an optional user navigation involvement userInvolvement (default "none"): // https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate -WebIDL::ExceptionOr Navigable::navigate( - AK::URL const& url, - JS::NonnullGCPtr source_document, - Variant document_resource, - JS::GCPtr response, - bool exceptions_enabled, - Bindings::NavigationHistoryBehavior history_handling, - Optional navigation_api_state, - Optional&> form_data_entry_list, - ReferrerPolicy::ReferrerPolicy referrer_policy, - UserNavigationInvolvement user_involvement) +WebIDL::ExceptionOr Navigable::navigate(NavigateParams params) { + auto const& url = params.url; + auto source_document = params.source_document; + auto const& document_resource = params.document_resource; + auto response = params.response; + auto exceptions_enabled = params.exceptions_enabled; + auto history_handling = params.history_handling; + auto const& navigation_api_state = params.navigation_api_state; + auto const& form_data_entry_list = params.form_data_entry_list; + auto referrer_policy = params.referrer_policy; + auto user_involvement = params.user_involvement; auto& active_document = *this->active_document(); auto& realm = active_document.realm(); auto& vm = this->vm(); diff --git a/Userland/Libraries/LibWeb/HTML/Navigable.h b/Userland/Libraries/LibWeb/HTML/Navigable.h index 08312ae59a..3541ca862d 100644 --- a/Userland/Libraries/LibWeb/HTML/Navigable.h +++ b/Userland/Libraries/LibWeb/HTML/Navigable.h @@ -122,17 +122,20 @@ public: bool allow_POST = false, Function completion_steps = [] {}); - WebIDL::ExceptionOr navigate( - AK::URL const&, - JS::NonnullGCPtr source_document, - Variant document_resource = Empty {}, - JS::GCPtr = nullptr, - bool exceptions_enabled = false, - Bindings::NavigationHistoryBehavior = Bindings::NavigationHistoryBehavior::Auto, - Optional navigation_api_state = {}, - Optional&> form_data_entry_list = {}, - ReferrerPolicy::ReferrerPolicy = ReferrerPolicy::ReferrerPolicy::EmptyString, - UserNavigationInvolvement = UserNavigationInvolvement::None); + struct NavigateParams { + AK::URL const& url; + JS::NonnullGCPtr source_document; + Variant document_resource = Empty {}; + JS::GCPtr response = nullptr; + bool exceptions_enabled = false; + Bindings::NavigationHistoryBehavior history_handling = Bindings::NavigationHistoryBehavior::Auto; + Optional navigation_api_state = {}; + Optional&> form_data_entry_list = {}; + ReferrerPolicy::ReferrerPolicy referrer_policy = ReferrerPolicy::ReferrerPolicy::EmptyString; + UserNavigationInvolvement user_involvement = UserNavigationInvolvement::None; + }; + + WebIDL::ExceptionOr navigate(NavigateParams); WebIDL::ExceptionOr navigate_to_a_fragment(AK::URL const&, HistoryHandlingBehavior, String navigation_id); diff --git a/Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp b/Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp index ef108fe8b5..cee0ec0562 100644 --- a/Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp +++ b/Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp @@ -242,7 +242,11 @@ void NavigableContainer::navigate_an_iframe_or_frame(AK::URL url, ReferrerPolicy Variant document_resource = Empty {}; if (srcdoc_string.has_value()) document_resource = srcdoc_string.value(); - MUST(m_content_navigable->navigate(url, document(), document_resource, nullptr, false, history_handling, {}, {}, referrer_policy)); + MUST(m_content_navigable->navigate({ .url = url, + .source_document = document(), + .document_resource = document_resource, + .history_handling = history_handling, + .referrer_policy = referrer_policy })); } // https://html.spec.whatwg.org/multipage/document-sequences.html#destroy-a-child-navigable diff --git a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp index 274d134846..32f25f0322 100644 --- a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp +++ b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp @@ -116,7 +116,9 @@ WebIDL::ExceptionOr> TraversableNavigable auto traversable = TRY(create_a_new_top_level_traversable(page, nullptr, {})); // 2. Navigate traversable to initialNavigationURL using traversable's active document, with documentResource set to initialNavigationPostResource. - TRY(traversable->navigate(initial_navigation_url, *traversable->active_document(), initial_navigation_post_resource)); + TRY(traversable->navigate({ .url = initial_navigation_url, + .source_document = *traversable->active_document(), + .document_resource = initial_navigation_post_resource })); // 3. Return traversable. return traversable; diff --git a/Userland/Libraries/LibWeb/HTML/Window.cpp b/Userland/Libraries/LibWeb/HTML/Window.cpp index ac4a4beb17..d7ba5e6101 100644 --- a/Userland/Libraries/LibWeb/HTML/Window.cpp +++ b/Userland/Libraries/LibWeb/HTML/Window.cpp @@ -376,7 +376,7 @@ WebIDL::ExceptionOr> Window::open_impl(StringView url, St // FIXME: 5. If urlRecord matches about:blank, then perform the URL and history update steps given target browsing context's active document and urlRecord. // 6. Otherwise, navigate targetNavigable to urlRecord using sourceDocument, with referrerPolicy set to referrerPolicy and exceptionsEnabled set to true. - TRY(target_navigable->navigate(url_record, source_document)); + TRY(target_navigable->navigate({ .url = url_record, .source_document = source_document })); } // 13. Otherwise: @@ -392,7 +392,7 @@ WebIDL::ExceptionOr> Window::open_impl(StringView url, St return WebIDL::SyntaxError::create(realm(), "URL is not valid"_fly_string); // 3. Navigate targetNavigable to urlRecord using sourceDocument, with referrerPolicy set to referrerPolicy and exceptionsEnabled set to true. - TRY(target_navigable->navigate(url_record, source_document)); + TRY(target_navigable->navigate({ .url = url_record, .source_document = source_document })); } // 2. If noopener is false, then set target browsing context's opener browsing context to source browsing context. diff --git a/Userland/Libraries/LibWeb/Page/EventHandler.cpp b/Userland/Libraries/LibWeb/Page/EventHandler.cpp index f93c78b214..0e7e579c32 100644 --- a/Userland/Libraries/LibWeb/Page/EventHandler.cpp +++ b/Userland/Libraries/LibWeb/Page/EventHandler.cpp @@ -285,7 +285,7 @@ bool EventHandler::handle_mouseup(CSSPixelPoint position, CSSPixelPoint screen_p auto url = document->parse_url(href); dbgln("Web::EventHandler: Clicking on a link to {}", url); if (button == GUI::MouseButton::Primary) { - MUST(document->navigable()->navigate(url, document)); + MUST(document->navigable()->navigate({ .url = url, .source_document = document })); } else if (button == GUI::MouseButton::Middle) { if (auto* page = m_browsing_context->page()) page->client().page_did_middle_click_link(url, link->target(), modifiers); diff --git a/Userland/Libraries/LibWeb/Page/Page.cpp b/Userland/Libraries/LibWeb/Page/Page.cpp index 5a30ba4dfd..b85393a672 100644 --- a/Userland/Libraries/LibWeb/Page/Page.cpp +++ b/Userland/Libraries/LibWeb/Page/Page.cpp @@ -45,12 +45,14 @@ void Page::set_focused_browsing_context(Badge, HTML::BrowsingConte void Page::load(const AK::URL& url) { - (void)top_level_traversable()->navigate(url, *top_level_traversable()->active_document()); + (void)top_level_traversable()->navigate({ .url = url, .source_document = *top_level_traversable()->active_document() }); } void Page::load_html(StringView html) { - (void)top_level_traversable()->navigate("about:srcdoc"sv, *top_level_traversable()->active_document(), String::from_utf8(html).release_value_but_fixme_should_propagate_errors()); + (void)top_level_traversable()->navigate({ .url = "about:srcdoc"sv, + .source_document = *top_level_traversable()->active_document(), + .document_resource = String::from_utf8(html).release_value_but_fixme_should_propagate_errors() }); } Gfx::Palette Page::palette() const