Skip to content

Commit 0060b8c

Browse files
kemzebADKaster
authored andcommitted
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.
1 parent 86781f0 commit 0060b8c

File tree

3 files changed

+19
-12
lines changed

3 files changed

+19
-12
lines changed

Userland/Applications/Browser/BookmarksBarWidget.cpp

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -292,10 +292,15 @@ bool BookmarksBarWidget::remove_bookmark(DeprecatedString const& url)
292292
auto& json_model = *static_cast<GUI::JsonArrayModel*>(model());
293293

294294
auto const item_removed = json_model.remove(item_index);
295-
if (item_removed)
296-
json_model.store();
295+
if (!item_removed)
296+
return false;
297297

298-
return item_removed;
298+
json_model.store();
299+
300+
if (on_bookmark_change)
301+
on_bookmark_change();
302+
303+
return true;
299304
}
300305
}
301306

@@ -315,8 +320,8 @@ bool BookmarksBarWidget::add_bookmark(DeprecatedString const& url, DeprecatedStr
315320
if (!was_bookmark_added)
316321
return false;
317322

318-
if (on_bookmark_add)
319-
on_bookmark_add(url);
323+
if (on_bookmark_change)
324+
on_bookmark_change();
320325

321326
if (edit_bookmark(url, PerformEditOn::NewBookmark))
322327
return true;
@@ -333,9 +338,14 @@ bool BookmarksBarWidget::edit_bookmark(DeprecatedString const& url, PerformEditO
333338

334339
if (item_url == url) {
335340
auto values = BookmarkEditor::edit_bookmark(window(), item_title, item_url, perform_edit_on);
336-
return update_model(values, [item_index](auto& model, auto&& values) {
341+
auto was_bookmark_changed = update_model(values, [item_index](auto& model, auto&& values) {
337342
return model.set(item_index, move(values));
338343
});
344+
345+
if (was_bookmark_changed && on_bookmark_change)
346+
on_bookmark_change();
347+
348+
return was_bookmark_changed;
339349
}
340350
}
341351

Userland/Applications/Browser/BookmarksBarWidget.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class BookmarksBarWidget final
3434

3535
Function<void(DeprecatedString const& url, Open)> on_bookmark_click;
3636
Function<void(DeprecatedString const&, DeprecatedString const&)> on_bookmark_hover;
37-
Function<void(DeprecatedString const& url)> on_bookmark_add;
37+
Function<void()> on_bookmark_change;
3838

3939
bool contains_bookmark(DeprecatedString const& url);
4040
bool remove_bookmark(DeprecatedString const& url);

Userland/Applications/Browser/Tab.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,6 @@ void Tab::bookmark_current_url()
606606
} else {
607607
BookmarksBarWidget::the().add_bookmark(url, m_title);
608608
}
609-
update_bookmark_button(url);
610609
}
611610

612611
void Tab::update_bookmark_button(DeprecatedString const& url)
@@ -635,10 +634,8 @@ void Tab::did_become_active()
635634
m_statusbar->set_text(url);
636635
};
637636

638-
BookmarksBarWidget::the().on_bookmark_add = [this](auto& url) {
639-
auto current_url = this->url().to_deprecated_string();
640-
if (current_url == url)
641-
update_bookmark_button(current_url);
637+
BookmarksBarWidget::the().on_bookmark_change = [this]() {
638+
update_bookmark_button(url().to_deprecated_string());
642639
};
643640

644641
BookmarksBarWidget::the().remove_from_parent();

0 commit comments

Comments
 (0)