From 0060b8c4e55871b9f82d3c72e7db07d5805b9485 Mon Sep 17 00:00:00 2001 From: Kemal Zebari Date: Wed, 29 Mar 2023 19:36:57 -0700 Subject: [PATCH] Browser: Have `BookmarksBarWidget` signal bookmark changes for `Tab` This fixes an issue with a tab not updating its bookmark button when we either edit or delete a bookmark and the tab happens to be on the same page associated with the bookmark URL. `BookmarksBarWidget` "signals" a `Tab` object of any bookmark changes, where it will update the bookmark button depending on if the current URL is an existing bookmark or not. --- .../Browser/BookmarksBarWidget.cpp | 22 ++++++++++++++----- .../Applications/Browser/BookmarksBarWidget.h | 2 +- Userland/Applications/Browser/Tab.cpp | 7 ++---- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/Userland/Applications/Browser/BookmarksBarWidget.cpp b/Userland/Applications/Browser/BookmarksBarWidget.cpp index f0aeae3014..d51b88ae5f 100644 --- a/Userland/Applications/Browser/BookmarksBarWidget.cpp +++ b/Userland/Applications/Browser/BookmarksBarWidget.cpp @@ -292,10 +292,15 @@ bool BookmarksBarWidget::remove_bookmark(DeprecatedString const& url) auto& json_model = *static_cast(model()); auto const item_removed = json_model.remove(item_index); - if (item_removed) - json_model.store(); + if (!item_removed) + return false; - return item_removed; + json_model.store(); + + if (on_bookmark_change) + on_bookmark_change(); + + return true; } } @@ -315,8 +320,8 @@ bool BookmarksBarWidget::add_bookmark(DeprecatedString const& url, DeprecatedStr if (!was_bookmark_added) return false; - if (on_bookmark_add) - on_bookmark_add(url); + if (on_bookmark_change) + on_bookmark_change(); if (edit_bookmark(url, PerformEditOn::NewBookmark)) return true; @@ -333,9 +338,14 @@ 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); - return update_model(values, [item_index](auto& model, auto&& values) { + 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) + on_bookmark_change(); + + return was_bookmark_changed; } } diff --git a/Userland/Applications/Browser/BookmarksBarWidget.h b/Userland/Applications/Browser/BookmarksBarWidget.h index 6737991577..9f70e6b162 100644 --- a/Userland/Applications/Browser/BookmarksBarWidget.h +++ b/Userland/Applications/Browser/BookmarksBarWidget.h @@ -34,7 +34,7 @@ public: Function on_bookmark_click; Function on_bookmark_hover; - Function on_bookmark_add; + Function on_bookmark_change; bool contains_bookmark(DeprecatedString const& url); bool remove_bookmark(DeprecatedString const& url); diff --git a/Userland/Applications/Browser/Tab.cpp b/Userland/Applications/Browser/Tab.cpp index 7b69d97dfe..02951e512d 100644 --- a/Userland/Applications/Browser/Tab.cpp +++ b/Userland/Applications/Browser/Tab.cpp @@ -606,7 +606,6 @@ void Tab::bookmark_current_url() } else { BookmarksBarWidget::the().add_bookmark(url, m_title); } - update_bookmark_button(url); } void Tab::update_bookmark_button(DeprecatedString const& url) @@ -635,10 +634,8 @@ void Tab::did_become_active() m_statusbar->set_text(url); }; - BookmarksBarWidget::the().on_bookmark_add = [this](auto& url) { - auto current_url = this->url().to_deprecated_string(); - if (current_url == url) - update_bookmark_button(current_url); + BookmarksBarWidget::the().on_bookmark_change = [this]() { + update_bookmark_button(url().to_deprecated_string()); }; BookmarksBarWidget::the().remove_from_parent();