Commit c5c2f0c0 authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

views: fix IconLabelBubbleView accessibility properties

IconLabelBubbleView is a Button and should use Button::GetAccessibleNodeData,
not Label::GetAccessibleNodeData. Setting its button text causes the right
behavior here.

Before: "This site is using your location."
After: "This site is using your location. Button."

Maybe ideally: "This site is using your location. Manage location access.
               Button."

... but that requires new strings.

Bug: 879014
Change-Id: I7b4840e17ea70eede41ee0b3095a8a2de288c6ea
Reviewed-on: https://chromium-review.googlesource.com/c/1331668
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarAaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607608}
parent 784739b0
...@@ -47,6 +47,7 @@ void ContentSettingImageView::Update() { ...@@ -47,6 +47,7 @@ void ContentSettingImageView::Update() {
// Note: We explicitly want to call this even if |web_contents| is NULL, so we // Note: We explicitly want to call this even if |web_contents| is NULL, so we
// get hidden properly while the user is editing the omnibox. // get hidden properly while the user is editing the omnibox.
content_setting_image_model_->Update(web_contents); content_setting_image_model_->Update(web_contents);
SetTooltipText(content_setting_image_model_->get_tooltip());
if (!content_setting_image_model_->is_visible()) { if (!content_setting_image_model_->is_visible()) {
SetVisible(false); SetVisible(false);
...@@ -90,12 +91,6 @@ void ContentSettingImageView::OnBoundsChanged( ...@@ -90,12 +91,6 @@ void ContentSettingImageView::OnBoundsChanged(
IconLabelBubbleView::OnBoundsChanged(previous_bounds); IconLabelBubbleView::OnBoundsChanged(previous_bounds);
} }
bool ContentSettingImageView::GetTooltipText(const gfx::Point& p,
base::string16* tooltip) const {
*tooltip = content_setting_image_model_->get_tooltip();
return !tooltip->empty();
}
bool ContentSettingImageView::OnMousePressed(const ui::MouseEvent& event) { bool ContentSettingImageView::OnMousePressed(const ui::MouseEvent& event) {
// Pause animation so that the icon does not shrink and deselect while the // Pause animation so that the icon does not shrink and deselect while the
// user is attempting to press it. // user is attempting to press it.
......
...@@ -70,8 +70,6 @@ class ContentSettingImageView : public IconLabelBubbleView, ...@@ -70,8 +70,6 @@ class ContentSettingImageView : public IconLabelBubbleView,
// IconLabelBubbleView: // IconLabelBubbleView:
const char* GetClassName() const override; const char* GetClassName() const override;
void OnBoundsChanged(const gfx::Rect& previous_bounds) override; void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
bool GetTooltipText(const gfx::Point& p,
base::string16* tooltip) const override;
bool OnMousePressed(const ui::MouseEvent& event) override; bool OnMousePressed(const ui::MouseEvent& event) override;
bool OnKeyPressed(const ui::KeyEvent& event) override; bool OnKeyPressed(const ui::KeyEvent& event) override;
void OnNativeThemeChanged(const ui::NativeTheme* native_theme) override; void OnNativeThemeChanged(const ui::NativeTheme* native_theme) override;
......
...@@ -167,6 +167,7 @@ bool IconLabelBubbleView::ShouldShowLabel() const { ...@@ -167,6 +167,7 @@ bool IconLabelBubbleView::ShouldShowLabel() const {
} }
void IconLabelBubbleView::SetLabel(const base::string16& label) { void IconLabelBubbleView::SetLabel(const base::string16& label) {
SetAccessibleName(label);
label_->SetText(label); label_->SetText(label);
separator_view_->SetVisible(ShouldShowSeparator()); separator_view_->SetVisible(ShouldShowSeparator());
} }
...@@ -289,17 +290,6 @@ bool IconLabelBubbleView::OnMousePressed(const ui::MouseEvent& event) { ...@@ -289,17 +290,6 @@ bool IconLabelBubbleView::OnMousePressed(const ui::MouseEvent& event) {
return Button::OnMousePressed(event); return Button::OnMousePressed(event);
} }
void IconLabelBubbleView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
label_->GetAccessibleNodeData(node_data);
if (node_data->GetStringAttribute(ax::mojom::StringAttribute::kName)
.empty()) {
// Fallback name when there is no accessible name from the label.
base::string16 tooltip_text;
GetTooltipText(gfx::Point(), &tooltip_text);
node_data->SetName(tooltip_text);
}
}
void IconLabelBubbleView::OnNativeThemeChanged( void IconLabelBubbleView::OnNativeThemeChanged(
const ui::NativeTheme* native_theme) { const ui::NativeTheme* native_theme) {
label_->SetEnabledColor(GetTextColor()); label_->SetEnabledColor(GetTextColor());
......
...@@ -135,7 +135,6 @@ class IconLabelBubbleView : public views::InkDropObserver, ...@@ -135,7 +135,6 @@ class IconLabelBubbleView : public views::InkDropObserver,
void OnBoundsChanged(const gfx::Rect& previous_bounds) override; void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
void Layout() override; void Layout() override;
bool OnMousePressed(const ui::MouseEvent& event) override; bool OnMousePressed(const ui::MouseEvent& event) override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
void OnNativeThemeChanged(const ui::NativeTheme* native_theme) override; void OnNativeThemeChanged(const ui::NativeTheme* native_theme) override;
void AddInkDropLayer(ui::Layer* ink_drop_layer) override; void AddInkDropLayer(ui::Layer* ink_drop_layer) override;
void RemoveInkDropLayer(ui::Layer* ink_drop_layer) override; void RemoveInkDropLayer(ui::Layer* ink_drop_layer) override;
......
...@@ -52,13 +52,6 @@ bool LocationIconView::OnMouseDragged(const ui::MouseEvent& event) { ...@@ -52,13 +52,6 @@ bool LocationIconView::OnMouseDragged(const ui::MouseEvent& event) {
return IconLabelBubbleView::OnMouseDragged(event); return IconLabelBubbleView::OnMouseDragged(event);
} }
bool LocationIconView::GetTooltipText(const gfx::Point& p,
base::string16* tooltip) const {
if (show_tooltip_)
*tooltip = l10n_util::GetStringUTF16(IDS_TOOLTIP_LOCATION_ICON);
return show_tooltip_;
}
SkColor LocationIconView::GetTextColor() const { SkColor LocationIconView::GetTextColor() const {
return delegate_->GetSecurityChipColor( return delegate_->GetSecurityChipColor(
delegate_->GetLocationBarModel()->GetSecurityLevel(false)); delegate_->GetLocationBarModel()->GetSecurityLevel(false));
...@@ -217,7 +210,9 @@ void LocationIconView::Update() { ...@@ -217,7 +210,9 @@ void LocationIconView::Update() {
bool is_editing_or_empty = delegate_->IsEditingOrEmpty(); bool is_editing_or_empty = delegate_->IsEditingOrEmpty();
// The tooltip should be shown if we are not editing or empty. // The tooltip should be shown if we are not editing or empty.
show_tooltip_ = !is_editing_or_empty; SetTooltipText(is_editing_or_empty
? base::string16()
: l10n_util::GetStringUTF16(IDS_TOOLTIP_LOCATION_ICON));
// If the omnibox is empty or editing, the user should not be able to left // If the omnibox is empty or editing, the user should not be able to left
// click on the icon. As such, the icon should not show a highlight or be // click on the icon. As such, the icon should not show a highlight or be
......
...@@ -71,8 +71,6 @@ class LocationIconView : public IconLabelBubbleView { ...@@ -71,8 +71,6 @@ class LocationIconView : public IconLabelBubbleView {
gfx::Size GetMinimumSize() const override; gfx::Size GetMinimumSize() const override;
bool OnMousePressed(const ui::MouseEvent& event) override; bool OnMousePressed(const ui::MouseEvent& event) override;
bool OnMouseDragged(const ui::MouseEvent& event) override; bool OnMouseDragged(const ui::MouseEvent& event) override;
bool GetTooltipText(const gfx::Point& p,
base::string16* tooltip) const override;
SkColor GetTextColor() const override; SkColor GetTextColor() const override;
bool ShouldShowSeparator() const override; bool ShouldShowSeparator() const override;
bool ShouldShowExtraEndSpace() const override; bool ShouldShowExtraEndSpace() const override;
...@@ -117,9 +115,6 @@ class LocationIconView : public IconLabelBubbleView { ...@@ -117,9 +115,6 @@ class LocationIconView : public IconLabelBubbleView {
int GetSlideDurationTime() const override; int GetSlideDurationTime() const override;
// True if hovering this view should display a tooltip.
bool show_tooltip_;
Delegate* delegate_; Delegate* delegate_;
// Used to scope the lifetime of asynchronous icon fetch callbacks to the // Used to scope the lifetime of asynchronous icon fetch callbacks to the
......
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