Commit 4a2014ae authored by Alan Cutter's avatar Alan Cutter Committed by Commit Bot

Fix PageActionIconView text colors

PwaIconView was the first PageActionIconView to make use of the label
functionality. This revealed two bugs in PageActionIconView:
 - The text color was hard coded as kColorId_LabelDisabledColor.
 - OnNativeThemeChanged() didn't call the superclass resulting in
   uninitialised values for LabelButton::button_state_colors_.

This CL fixes both issues.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=386810&signed_aid=781DgZbUS5lVNN4F1taohA==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=386811&signed_aid=n-KvGoW3qBU6UsD6x72gnA==&inline=1

Change-Id: I834b7720932bab5eff93892ea7026b8d6140a720
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1545431
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#646643}
parent e508bc24
...@@ -65,10 +65,13 @@ bool PageActionIconView::Update() { ...@@ -65,10 +65,13 @@ bool PageActionIconView::Update() {
return false; return false;
} }
SkColor PageActionIconView::GetLabelColorForTesting() const {
return label()->enabled_color();
}
SkColor PageActionIconView::GetTextColor() const { SkColor PageActionIconView::GetTextColor() const {
// Returns the color of the label shown during animation.
return GetNativeTheme()->GetSystemColor( return GetNativeTheme()->GetSystemColor(
ui::NativeTheme::kColorId_LabelDisabledColor); ui::NativeTheme::kColorId_TextfieldDefaultColor);
} }
void PageActionIconView::GetAccessibleNodeData(ui::AXNodeData* node_data) { void PageActionIconView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
...@@ -144,6 +147,7 @@ void PageActionIconView::ViewHierarchyChanged( ...@@ -144,6 +147,7 @@ void PageActionIconView::ViewHierarchyChanged(
} }
void PageActionIconView::OnNativeThemeChanged(const ui::NativeTheme* theme) { void PageActionIconView::OnNativeThemeChanged(const ui::NativeTheme* theme) {
IconLabelBubbleView::OnNativeThemeChanged(theme);
UpdateIconImage(); UpdateIconImage();
} }
......
...@@ -59,6 +59,8 @@ class PageActionIconView : public IconLabelBubbleView { ...@@ -59,6 +59,8 @@ class PageActionIconView : public IconLabelBubbleView {
// Retrieve the text to be used for a tooltip or accessible name. // Retrieve the text to be used for a tooltip or accessible name.
virtual base::string16 GetTextForTooltipAndAccessibleName() const = 0; virtual base::string16 GetTextForTooltipAndAccessibleName() const = 0;
SkColor GetLabelColorForTesting() const;
protected: protected:
enum ExecuteSource { enum ExecuteSource {
EXECUTE_SOURCE_MOUSE, EXECUTE_SOURCE_MOUSE,
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "chrome/test/base/ui_test_utils.h" #include "chrome/test/base/ui_test_utils.h"
#include "content/public/common/referrer.h" #include "content/public/common/referrer.h"
#include "services/network/public/cpp/network_switches.h" #include "services/network/public/cpp/network_switches.h"
#include "ui/gfx/color_utils.h"
class PwaInstallViewBrowserTest : public InProcessBrowserTest { class PwaInstallViewBrowserTest : public InProcessBrowserTest {
public: public:
...@@ -219,3 +220,22 @@ IN_PROC_BROWSER_TEST_F(PwaInstallViewBrowserTest, ...@@ -219,3 +220,22 @@ IN_PROC_BROWSER_TEST_F(PwaInstallViewBrowserTest,
EXPECT_TRUE(pwa_install_view_->visible()); EXPECT_TRUE(pwa_install_view_->visible());
EXPECT_TRUE(pwa_install_view_->is_animating_label()); EXPECT_TRUE(pwa_install_view_->is_animating_label());
} }
// Tests that the icon label is visible against the omnibox background after the
// native widget becomes active.
IN_PROC_BROWSER_TEST_F(PwaInstallViewBrowserTest, TextContrast) {
NavigateToURL(GetInstallableAppURL());
ASSERT_TRUE(app_banner_manager_->WaitForInstallableCheck());
EXPECT_TRUE(pwa_install_view_->visible());
EXPECT_TRUE(pwa_install_view_->is_animating_label());
pwa_install_view_->GetWidget()->OnNativeWidgetActivationChanged(true);
BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser());
SkColor omnibox_background = browser_view->GetLocationBarView()->GetColor(
OmniboxPart::LOCATION_BAR_BACKGROUND);
SkColor label_color = pwa_install_view_->GetLabelColorForTesting();
EXPECT_EQ(SkColorGetA(label_color), SK_AlphaOPAQUE);
EXPECT_GT(color_utils::GetContrastRatio(omnibox_background, label_color),
color_utils::kMinimumReadableContrastRatio);
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment