0

Call super class AddedToWidget/RemovedFromWidget from BookmarkButton

This addresses an issue with the bookmark bar icons becoming dim
upon certain actions occurring like opening a new window.

Bug: 385805737
Change-Id: Id05f166305d6122190ee7219beee631fcb039a6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6131420
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Andrew Williams <awillia@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1401117}
This commit is contained in:
Andrew Williams
2024-12-31 05:53:08 -08:00
committed by Chromium LUCI CQ
parent a627aa42e5
commit d67e2809d9
3 changed files with 33 additions and 0 deletions
chrome/browser/ui/views/bookmarks
ui/views/controls/button

@ -50,6 +50,7 @@
#include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/controls/button/label_button.h"
#include "ui/views/controls/button/menu_button.h"
#include "ui/views/style/platform_style.h"
#include "ui/views/test/views_test_utils.h"
#include "ui/views/view_utils.h"
#include "url/gurl.h"
@ -660,6 +661,30 @@ TEST_F(BookmarkBarViewInWidgetTest, UpdateTooltipText) {
EXPECT_EQ(u"new title\na.com", button->GetTooltipText(p));
}
// Regression test for https://crbug.com/385805737. When BookmarkButton receives
// an AddedToWidget call, it should also call the corresponding superclass
// method (specifically, `LabelButton::AddedToWidget()` must be called).
TEST_F(BookmarkBarViewInWidgetTest,
BookmarkButtonAddedToWidgetCallsSuperclass) {
widget()->ShowInactive();
widget()->Hide();
bookmarks::test::AddNodesFromModelString(model(),
model()->bookmark_bar_node(), "a b");
SizeUntilButtonsVisible(1);
// `BookmarkButton::AddedToWidget()` will have been called, so ensure that
// `LabelButton::AddedToWidget()` has been called as well.
ASSERT_EQ(1u, test_helper_->GetBookmarkButtonCount());
views::LabelButton* button = test_helper_->GetBookmarkButton(0);
ASSERT_TRUE(button);
// The `LabelButton::AddedToWidget()` call only has an effect for bookmark
// buttons on certain platforms, so gate the check.
if (views::PlatformStyle::kInactiveWidgetControlsAppearDisabled) {
EXPECT_TRUE(button->has_paint_as_active_subscription_for_testing());
}
}
// TODO(crbug.com/375364962): Flaky on Windows & Linux.
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX)
#define MAYBE_AccessibleRoleDescription DISABLED_AccessibleRoleDescription

@ -118,12 +118,16 @@ BookmarkButton::BookmarkButton(PressedCallback callback,
BookmarkButton::~BookmarkButton() = default;
void BookmarkButton::AddedToWidget() {
BookmarkButtonBase::AddedToWidget();
widget_observation_.Observe(GetWidget());
UpdateMaxTooltipWidth();
}
void BookmarkButton::RemovedFromWidget() {
BookmarkButtonBase::RemovedFromWidget();
widget_observation_.Reset();
}

@ -188,6 +188,10 @@ class VIEWS_EXPORT LabelButton : public Button,
// widget, and the parent of the containing widget.
ButtonState GetVisualState() const;
bool has_paint_as_active_subscription_for_testing() {
return !!paint_as_active_subscription_;
}
protected:
LabelButtonImageContainer* image_container() {
return image_container_.get();