From 3c675e3f25d8d9a5bd8943aef1b40a048a2ee748 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Sun, 17 Sep 2023 17:12:17 +0200 Subject: [PATCH] Userland+Ladybird: Always specify url to be about:srcdoc in load_html() After moving to navigables, we started reusing the code that populates session history entries with the srcdoc attribute value from iframes in `Page::load_html()` for loading HTML. This change addresses a crash in `determine_the_origin` which occurred because this method expected the URL to be `about:srcdoc` if we also provided HTML content (previously, it was the URL passed along with the HTML content into `load_html()`). --- Ladybird/AppKit/UI/LadybirdWebView.h | 2 +- Ladybird/AppKit/UI/LadybirdWebView.mm | 4 ++-- Ladybird/AppKit/UI/TabController.mm | 2 +- Ladybird/Qt/BrowserWindow.cpp | 4 ++-- Ladybird/Qt/BrowserWindow.h | 2 +- Ladybird/Qt/Tab.cpp | 6 +++--- Ladybird/Qt/Tab.h | 2 +- Userland/Applications/Mail/MailWidget.cpp | 2 +- Userland/Applications/Presenter/PresenterWidget.cpp | 2 +- Userland/Applications/TextEditor/MainWidget.cpp | 4 ++-- Userland/DevTools/HackStudio/Editor.cpp | 4 ++-- Userland/Libraries/LibWeb/Page/Page.cpp | 4 ++-- Userland/Libraries/LibWeb/Page/Page.h | 2 +- Userland/Libraries/LibWebView/ViewImplementation.cpp | 9 ++++----- Userland/Libraries/LibWebView/ViewImplementation.h | 2 +- Userland/Services/WebContent/ConnectionFromClient.cpp | 8 ++++---- Userland/Services/WebContent/ConnectionFromClient.h | 2 +- Userland/Services/WebContent/WebContentServer.ipc | 2 +- 18 files changed, 31 insertions(+), 32 deletions(-) diff --git a/Ladybird/AppKit/UI/LadybirdWebView.h b/Ladybird/AppKit/UI/LadybirdWebView.h index fd22836a6d..da7661be44 100644 --- a/Ladybird/AppKit/UI/LadybirdWebView.h +++ b/Ladybird/AppKit/UI/LadybirdWebView.h @@ -40,7 +40,7 @@ - (instancetype)init:(id)observer; - (void)loadURL:(URL const&)url; -- (void)loadHTML:(StringView)html url:(URL const&)url; +- (void)loadHTML:(StringView)html; - (WebView::ViewImplementation&)view; - (String const&)handle; diff --git a/Ladybird/AppKit/UI/LadybirdWebView.mm b/Ladybird/AppKit/UI/LadybirdWebView.mm index 8d4954d514..303490c531 100644 --- a/Ladybird/AppKit/UI/LadybirdWebView.mm +++ b/Ladybird/AppKit/UI/LadybirdWebView.mm @@ -109,9 +109,9 @@ struct HideCursor { m_web_view_bridge->load(url); } -- (void)loadHTML:(StringView)html url:(URL const&)url +- (void)loadHTML:(StringView)html { - m_web_view_bridge->load_html(html, url); + m_web_view_bridge->load_html(html); } - (WebView::ViewImplementation&)view diff --git a/Ladybird/AppKit/UI/TabController.mm b/Ladybird/AppKit/UI/TabController.mm index 1543b73aed..210c712c6c 100644 --- a/Ladybird/AppKit/UI/TabController.mm +++ b/Ladybird/AppKit/UI/TabController.mm @@ -91,7 +91,7 @@ enum class IsHistoryNavigation { - (void)loadHTML:(StringView)html url:(URL const&)url { - [[self tab].web_view loadHTML:html url:url]; + [[self tab].web_view loadHTML:html]; } - (void)onLoadStart:(URL const&)url isRedirect:(BOOL)isRedirect diff --git a/Ladybird/Qt/BrowserWindow.cpp b/Ladybird/Qt/BrowserWindow.cpp index b1812e354a..bf4fe68441 100644 --- a/Ladybird/Qt/BrowserWindow.cpp +++ b/Ladybird/Qt/BrowserWindow.cpp @@ -442,10 +442,10 @@ Tab& BrowserWindow::new_tab(QString const& url, Web::HTML::ActivateTab activate_ return tab; } -Tab& BrowserWindow::new_tab(StringView html, URL const& url, Web::HTML::ActivateTab activate_tab) +Tab& BrowserWindow::new_tab(StringView html, Web::HTML::ActivateTab activate_tab) { auto& tab = create_new_tab(activate_tab); - tab.load_html(html, url); + tab.load_html(html); return tab; } diff --git a/Ladybird/Qt/BrowserWindow.h b/Ladybird/Qt/BrowserWindow.h index cd344425aa..1a72efd7f9 100644 --- a/Ladybird/Qt/BrowserWindow.h +++ b/Ladybird/Qt/BrowserWindow.h @@ -70,7 +70,7 @@ public slots: void tab_title_changed(int index, QString const&); void tab_favicon_changed(int index, QIcon const& icon); Tab& new_tab(QString const&, Web::HTML::ActivateTab); - Tab& new_tab(StringView html, URL const& url, Web::HTML::ActivateTab); + Tab& new_tab(StringView html, Web::HTML::ActivateTab); void activate_tab(int index); void close_tab(int index); void close_current_tab(); diff --git a/Ladybird/Qt/Tab.cpp b/Ladybird/Qt/Tab.cpp index de56d13c10..df9d493915 100644 --- a/Ladybird/Qt/Tab.cpp +++ b/Ladybird/Qt/Tab.cpp @@ -217,7 +217,7 @@ Tab::Tab(BrowserWindow* window, StringView webdriver_content_ipc_path, WebView:: view().on_received_source = [this](auto const& url, auto const& source) { auto html = WebView::highlight_source(url, source); - m_window->new_tab(html, url, Web::HTML::ActivateTab::Yes); + m_window->new_tab(html, Web::HTML::ActivateTab::Yes); }; view().on_navigate_back = [this]() { @@ -550,9 +550,9 @@ void Tab::navigate(QString url_qstring) view().load(url_string); } -void Tab::load_html(StringView html, URL const& url) +void Tab::load_html(StringView html) { - view().load_html(html, url); + view().load_html(html); } void Tab::back() diff --git a/Ladybird/Qt/Tab.h b/Ladybird/Qt/Tab.h index 83d713d1d5..586e41e821 100644 --- a/Ladybird/Qt/Tab.h +++ b/Ladybird/Qt/Tab.h @@ -34,7 +34,7 @@ public: WebContentView& view() { return *m_view; } void navigate(QString); - void load_html(StringView, URL const&); + void load_html(StringView); void back(); void forward(); diff --git a/Userland/Applications/Mail/MailWidget.cpp b/Userland/Applications/Mail/MailWidget.cpp index b5ae365a05..689742582a 100644 --- a/Userland/Applications/Mail/MailWidget.cpp +++ b/Userland/Applications/Mail/MailWidget.cpp @@ -505,5 +505,5 @@ void MailWidget::selected_email_to_load(GUI::ModelIndex const& index) // FIXME: I'm not sure what the URL should be. Just use the default URL "about:blank". // FIXME: It would be nice if we could pass over the charset. // FIXME: Add ability to cancel the load when we switch to another email. Feels very sluggish on heavy emails otherwise - m_web_view->load_html(decoded_data, "about:blank"sv); + m_web_view->load_html(decoded_data); } diff --git a/Userland/Applications/Presenter/PresenterWidget.cpp b/Userland/Applications/Presenter/PresenterWidget.cpp index bc0e4bb328..97003e1a90 100644 --- a/Userland/Applications/Presenter/PresenterWidget.cpp +++ b/Userland/Applications/Presenter/PresenterWidget.cpp @@ -152,7 +152,7 @@ void PresenterWidget::set_file(StringView file_name) m_current_presentation = presentation.release_value(); window()->set_title(DeprecatedString::formatted(title_template, m_current_presentation->title(), m_current_presentation->author())); set_min_size(m_current_presentation->normative_size()); - m_web_view->load_html(MUST(m_current_presentation->render()), "presenter://slide.html"sv); + m_web_view->load_html(MUST(m_current_presentation->render())); update_slides_actions(); } } diff --git a/Userland/Applications/TextEditor/MainWidget.cpp b/Userland/Applications/TextEditor/MainWidget.cpp index fc13be59d9..8fe943ee6d 100644 --- a/Userland/Applications/TextEditor/MainWidget.cpp +++ b/Userland/Applications/TextEditor/MainWidget.cpp @@ -909,7 +909,7 @@ void MainWidget::update_markdown_preview() if (document) { auto html = document->render_to_html(); auto current_scroll_pos = m_page_view->visible_content_rect(); - m_page_view->load_html(html, URL::create_with_file_scheme(m_path)); + m_page_view->load_html(html); m_page_view->scroll_into_view(current_scroll_pos, true, true); } } @@ -917,7 +917,7 @@ void MainWidget::update_markdown_preview() void MainWidget::update_html_preview() { auto current_scroll_pos = m_page_view->visible_content_rect(); - m_page_view->load_html(m_editor->text(), URL::create_with_file_scheme(m_path)); + m_page_view->load_html(m_editor->text()); m_page_view->scroll_into_view(current_scroll_pos, true, true); } diff --git a/Userland/DevTools/HackStudio/Editor.cpp b/Userland/DevTools/HackStudio/Editor.cpp index 4234230fe1..f2b44d795c 100644 --- a/Userland/DevTools/HackStudio/Editor.cpp +++ b/Userland/DevTools/HackStudio/Editor.cpp @@ -229,7 +229,7 @@ void Editor::show_documentation_tooltip_if_available(DeprecatedString const& hov return; } - s_tooltip_page_view->load_html(man_document->render_to_html(""sv), {}); + s_tooltip_page_view->load_html(man_document->render_to_html(""sv)); s_tooltip_window->set_rect(0, 0, 500, 400); s_tooltip_window->move_to(screen_location.translated(4, 4)); @@ -722,7 +722,7 @@ void Editor::handle_function_parameters_hint_request() } html.append(""sv); - s_tooltip_page_view->load_html(html.to_deprecated_string(), {}); + s_tooltip_page_view->load_html(html.to_deprecated_string()); auto cursor_rect = current_editor().cursor_content_rect().location().translated(screen_relative_rect().location()); diff --git a/Userland/Libraries/LibWeb/Page/Page.cpp b/Userland/Libraries/LibWeb/Page/Page.cpp index 313b5c2d29..8540f2e799 100644 --- a/Userland/Libraries/LibWeb/Page/Page.cpp +++ b/Userland/Libraries/LibWeb/Page/Page.cpp @@ -47,9 +47,9 @@ void Page::load(const AK::URL& url) (void)top_level_traversable()->navigate(url, *top_level_traversable()->active_document()); } -void Page::load_html(StringView html, const AK::URL& url) +void Page::load_html(StringView html) { - (void)top_level_traversable()->navigate(url, *top_level_traversable()->active_document(), String::from_utf8(html).release_value_but_fixme_should_propagate_errors()); + (void)top_level_traversable()->navigate("about:srcdoc"sv, *top_level_traversable()->active_document(), String::from_utf8(html).release_value_but_fixme_should_propagate_errors()); } bool Page::has_ongoing_navigation() const diff --git a/Userland/Libraries/LibWeb/Page/Page.h b/Userland/Libraries/LibWeb/Page/Page.h index 002a078747..bf6dbfcbbb 100644 --- a/Userland/Libraries/LibWeb/Page/Page.h +++ b/Userland/Libraries/LibWeb/Page/Page.h @@ -60,7 +60,7 @@ public: void load(const AK::URL&); - void load_html(StringView, const AK::URL&); + void load_html(StringView); bool has_ongoing_navigation() const; diff --git a/Userland/Libraries/LibWebView/ViewImplementation.cpp b/Userland/Libraries/LibWebView/ViewImplementation.cpp index ca906b0986..d855ac28e1 100644 --- a/Userland/Libraries/LibWebView/ViewImplementation.cpp +++ b/Userland/Libraries/LibWebView/ViewImplementation.cpp @@ -86,15 +86,14 @@ void ViewImplementation::load(AK::URL const& url) client().async_load_url(url); } -void ViewImplementation::load_html(StringView html, AK::URL const& url) +void ViewImplementation::load_html(StringView html) { - m_url = url; - client().async_load_html(html, url); + client().async_load_html(html); } void ViewImplementation::load_empty_document() { - load_html(""sv, {}); + load_html(""sv); } void ViewImplementation::zoom_in() @@ -328,7 +327,7 @@ void ViewImplementation::handle_web_content_process_crash() auto escaped_url = escape_html_entities(m_url.to_deprecated_string()); builder.appendff("The web page {} has crashed.

You can reload the page to try again.", escaped_url, escaped_url); builder.append(""sv); - load_html(builder.to_deprecated_string(), m_url); + load_html(builder.to_deprecated_string()); } ErrorOr ViewImplementation::take_screenshot(ScreenshotType type) diff --git a/Userland/Libraries/LibWebView/ViewImplementation.h b/Userland/Libraries/LibWebView/ViewImplementation.h index 0f92a6e4b7..9dd41dcfac 100644 --- a/Userland/Libraries/LibWebView/ViewImplementation.h +++ b/Userland/Libraries/LibWebView/ViewImplementation.h @@ -52,7 +52,7 @@ public: void server_did_change_selection(Badge); void load(AK::URL const&); - void load_html(StringView, AK::URL const&); + void load_html(StringView); void load_empty_document(); void zoom_in(); diff --git a/Userland/Services/WebContent/ConnectionFromClient.cpp b/Userland/Services/WebContent/ConnectionFromClient.cpp index a58c352bcb..7fa4116394 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.cpp +++ b/Userland/Services/WebContent/ConnectionFromClient.cpp @@ -119,10 +119,10 @@ void ConnectionFromClient::load_url(const URL& url) page().load(url); } -void ConnectionFromClient::load_html(DeprecatedString const& html, const URL& url) +void ConnectionFromClient::load_html(DeprecatedString const& html) { - dbgln_if(SPAM_DEBUG, "handle: WebContentServer::LoadHTML: html={}, url={}", html, url); - page().load_html(html, url); + dbgln_if(SPAM_DEBUG, "handle: WebContentServer::LoadHTML: html={}", html); + page().load_html(html); } void ConnectionFromClient::set_viewport_rect(Gfx::IntRect const& rect) @@ -481,7 +481,7 @@ void ConnectionFromClient::debug_request(DeprecatedString const& request, Deprec auto maybe_link = document->query_selector("link[rel=match]"sv); if (maybe_link.is_error() || !maybe_link.value()) { // To make sure that we fail the ref-test if the link is missing, load the error page. - load_html("

Failed to find <link rel="match" /> in ref test page!

Make sure you added it.", "about:blank"sv); + load_html("

Failed to find <link rel="match" /> in ref test page!

Make sure you added it."); } else { auto link = maybe_link.release_value(); auto url = document->parse_url(link->get_attribute(Web::HTML::AttributeNames::href)); diff --git a/Userland/Services/WebContent/ConnectionFromClient.h b/Userland/Services/WebContent/ConnectionFromClient.h index 18d0d4af71..84b8617ced 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.h +++ b/Userland/Services/WebContent/ConnectionFromClient.h @@ -56,7 +56,7 @@ private: virtual void update_system_fonts(DeprecatedString const&, DeprecatedString const&, DeprecatedString const&) override; virtual void update_screen_rects(Vector const&, u32) override; virtual void load_url(URL const&) override; - virtual void load_html(DeprecatedString const&, URL const&) override; + virtual void load_html(DeprecatedString const&) override; virtual void paint(Gfx::IntRect const&, i32) override; virtual void set_viewport_rect(Gfx::IntRect const&) override; virtual void mouse_down(Gfx::IntPoint, Gfx::IntPoint, unsigned, unsigned, unsigned) override; diff --git a/Userland/Services/WebContent/WebContentServer.ipc b/Userland/Services/WebContent/WebContentServer.ipc index 7b9279b7cc..9f71e918d9 100644 --- a/Userland/Services/WebContent/WebContentServer.ipc +++ b/Userland/Services/WebContent/WebContentServer.ipc @@ -19,7 +19,7 @@ endpoint WebContentServer update_screen_rects(Vector rects, u32 main_screen_index) =| load_url(URL url) =| - load_html(DeprecatedString html, URL url) =| + load_html(DeprecatedString html) =| add_backing_store(i32 backing_store_id, Gfx::ShareableBitmap bitmap) =| remove_backing_store(i32 backing_store_id) =|