diff --git a/Userland/Applications/Browser/BookmarksBarWidget.cpp b/Userland/Applications/Browser/BookmarksBarWidget.cpp index 6614faebd6..ebe83a5730 100644 --- a/Userland/Applications/Browser/BookmarksBarWidget.cpp +++ b/Userland/Applications/Browser/BookmarksBarWidget.cpp @@ -1,6 +1,7 @@ /* * Copyright (c) 2020, Emanuel Sprung * Copyright (c) 2022, networkException + * Copyright (c) 2023, Cameron Youell * * SPDX-License-Identifier: BSD-2-Clause */ @@ -15,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -151,12 +153,14 @@ BookmarksBarWidget::BookmarksBarWidget(DeprecatedString const& bookmarks_file, b m_context_menu->add_separator(); m_context_menu->add_action(GUI::Action::create( "&Edit...", g_icon_bag.rename, [this](auto&) { - edit_bookmark(m_context_menu_url); + if (auto result = edit_bookmark(m_context_menu_url); result.is_error()) + GUI::MessageBox::show_error(this->window(), MUST(String::formatted("Failed to edit bookmark: {}", result.error()))); }, this)); m_context_menu->add_action(GUI::CommonActions::make_delete_action( [this](auto&) { - remove_bookmark(m_context_menu_url); + if (auto result = remove_bookmark(m_context_menu_url); result.is_error()) + GUI::MessageBox::show_error(this->window(), MUST(String::formatted("Failed to remove bookmark: {}", result.error()))); }, this)); @@ -271,7 +275,7 @@ void BookmarksBarWidget::update_content_size() } } -bool BookmarksBarWidget::contains_bookmark(DeprecatedString const& url) +bool BookmarksBarWidget::contains_bookmark(StringView url) { for (int item_index = 0; item_index < model()->row_count(); ++item_index) { @@ -283,7 +287,7 @@ bool BookmarksBarWidget::contains_bookmark(DeprecatedString const& url) return false; } -bool BookmarksBarWidget::remove_bookmark(DeprecatedString const& url) +ErrorOr BookmarksBarWidget::remove_bookmark(StringView url) { for (int item_index = 0; item_index < model()->row_count(); ++item_index) { @@ -291,46 +295,41 @@ bool BookmarksBarWidget::remove_bookmark(DeprecatedString const& url) if (item_url == url) { auto& json_model = *static_cast(model()); - auto const item_removed = json_model.remove(item_index); - if (!item_removed) - return false; - - json_model.store(); + TRY(json_model.remove(item_index)); + TRY(json_model.store()); if (on_bookmark_change) on_bookmark_change(); - return true; + return {}; } } - return false; + return Error::from_string_view("Bookmark not found"sv); } -bool BookmarksBarWidget::add_bookmark(DeprecatedString const& url, DeprecatedString const& title) +ErrorOr BookmarksBarWidget::add_bookmark(StringView url, StringView title) { Vector values; - values.append(title); - values.append(url); + TRY(values.try_append(title)); + TRY(values.try_append(url)); - auto was_bookmark_added = update_model(values, [](auto& model, auto&& values) { - return model.add(move(values)); - }); - - if (!was_bookmark_added) - return false; + TRY(update_model(values, [](auto& model, auto&& values) -> ErrorOr { + return TRY(model.add(move(values))); + })); if (on_bookmark_change) on_bookmark_change(); - if (edit_bookmark(url, PerformEditOn::NewBookmark)) - return true; + if (auto result = edit_bookmark(url, PerformEditOn::NewBookmark); result.is_error()) { + (void)remove_bookmark(url); + return Error::copy(result.release_error()); + } - remove_bookmark(url); - return false; + return {}; } -bool BookmarksBarWidget::edit_bookmark(DeprecatedString const& url, PerformEditOn perform_edit_on) +ErrorOr BookmarksBarWidget::edit_bookmark(StringView url, PerformEditOn perform_edit_on) { for (int item_index = 0; item_index < model()->row_count(); ++item_index) { auto item_title = model()->index(item_index, 0).data().to_deprecated_string(); @@ -338,33 +337,32 @@ bool BookmarksBarWidget::edit_bookmark(DeprecatedString const& url, PerformEditO if (item_url == url) { auto values = BookmarkEditor::edit_bookmark(window(), item_title, item_url, perform_edit_on); - auto was_bookmark_changed = update_model(values, [item_index](auto& model, auto&& values) { - return model.set(item_index, move(values)); - }); - if (was_bookmark_changed && on_bookmark_change) + TRY(update_model(values, [item_index](auto& model, auto&& values) { + return model.set(item_index, move(values)); + })); + + if (on_bookmark_change) on_bookmark_change(); - return was_bookmark_changed; + return {}; } } - return false; + return Error::from_string_view("Bookmark not found"sv); } -bool BookmarksBarWidget::update_model(Vector& values, Function&& values)> perform_model_change) +ErrorOr BookmarksBarWidget::update_model(Vector& values, Function(GUI::JsonArrayModel& model, Vector&& values)> perform_model_change) { - bool has_model_changed = false; + if (values.is_empty()) + return Error::from_string_view("No values to update model with"sv); - if (!values.is_empty()) { - auto& json_model = *static_cast(model()); - has_model_changed = perform_model_change(json_model, move(values)); + auto& json_model = *static_cast(model()); - if (has_model_changed) - json_model.store(); - } + TRY(perform_model_change(json_model, move(values))); + TRY(json_model.store()); - return has_model_changed; + return {}; } } diff --git a/Userland/Applications/Browser/BookmarksBarWidget.h b/Userland/Applications/Browser/BookmarksBarWidget.h index 9f70e6b162..c4bc426cfe 100644 --- a/Userland/Applications/Browser/BookmarksBarWidget.h +++ b/Userland/Applications/Browser/BookmarksBarWidget.h @@ -32,20 +32,19 @@ public: InNewWindow }; - Function on_bookmark_click; - Function on_bookmark_hover; - Function on_bookmark_change; - - bool contains_bookmark(DeprecatedString const& url); - bool remove_bookmark(DeprecatedString const& url); - bool add_bookmark(DeprecatedString const& url, DeprecatedString const& title); - enum class PerformEditOn { NewBookmark, ExistingBookmark }; - bool edit_bookmark(DeprecatedString const& url, PerformEditOn perform_edit_on = PerformEditOn::ExistingBookmark); + Function on_bookmark_click; + Function on_bookmark_hover; + Function on_bookmark_change; + + bool contains_bookmark(StringView url); + ErrorOr remove_bookmark(StringView url); + ErrorOr add_bookmark(StringView url, StringView title); + ErrorOr edit_bookmark(StringView url, PerformEditOn perform_edit_on = PerformEditOn::ExistingBookmark); virtual Optional calculated_min_size() const override { @@ -64,7 +63,7 @@ private: void update_content_size(); - bool update_model(Vector& values, Function&& values)> perform_model_change); + ErrorOr update_model(Vector& values, Function(GUI::JsonArrayModel& model, Vector&& values)> perform_model_change); RefPtr m_model; RefPtr m_additional; diff --git a/Userland/Applications/Browser/Tab.cpp b/Userland/Applications/Browser/Tab.cpp index 45645c4cb3..5631b46e47 100644 --- a/Userland/Applications/Browser/Tab.cpp +++ b/Userland/Applications/Browser/Tab.cpp @@ -4,6 +4,7 @@ * Copyright (c) 2021, Sam Atkins * Copyright (c) 2022, the SerenityOS developers. * Copyright (c) 2022, networkException + * Copyright (c) 2023, Cameron Youell * * SPDX-License-Identifier: BSD-2-Clause */ @@ -195,7 +196,8 @@ Tab::Tab(BrowserWindow& window) auto bookmark_action = GUI::Action::create( "Bookmark current URL", { Mod_Ctrl, Key_D }, [this](auto&) { - bookmark_current_url(); + if (auto result = bookmark_current_url(); result.is_error()) + GUI::MessageBox::show_error(this->window().main_widget()->window(), MUST(String::formatted("Failed to bookmark URL: {}", result.error()))); }, this); @@ -596,17 +598,19 @@ void Tab::update_actions() window.go_forward_action().set_enabled(m_history.can_go_forward()); } -void Tab::bookmark_current_url() +ErrorOr Tab::bookmark_current_url() { auto url = this->url().to_deprecated_string(); if (BookmarksBarWidget::the().contains_bookmark(url)) { - BookmarksBarWidget::the().remove_bookmark(url); + TRY(BookmarksBarWidget::the().remove_bookmark(url)); } else { - BookmarksBarWidget::the().add_bookmark(url, m_title); + TRY(BookmarksBarWidget::the().add_bookmark(url, m_title)); } + + return {}; } -void Tab::update_bookmark_button(DeprecatedString const& url) +void Tab::update_bookmark_button(StringView url) { if (BookmarksBarWidget::the().contains_bookmark(url)) { m_bookmark_button->set_icon(g_icon_bag.bookmark_filled); diff --git a/Userland/Applications/Browser/Tab.h b/Userland/Applications/Browser/Tab.h index 2d99798caa..b6dc5d17f4 100644 --- a/Userland/Applications/Browser/Tab.h +++ b/Userland/Applications/Browser/Tab.h @@ -108,8 +108,8 @@ private: BrowserWindow& window(); void update_actions(); - void bookmark_current_url(); - void update_bookmark_button(DeprecatedString const& url); + ErrorOr bookmark_current_url(); + void update_bookmark_button(StringView url); void start_download(const URL& url); void view_source(const URL& url, DeprecatedString const& source); void update_status(Optional text_override = {}, i32 count_waiting = 0);