0

[views-ax] Fix cached role/name/description out of sync in the cache (//ash)

We just discovered that CL:5537333 introduced an unexpected change in
behavior for a subset of views around the accessible role, name, and
description. The change is around what role, name, or description to
apply: the one in the ViewAccessibility cache, or the one computed in
the View::GetAccessibleNodeData override. Prior to CL:5537333, the
latter was exposed, but since then, it's the one in the cache that has
the final say.

Ultimately, when the ViewsAX refactor is complete, we'll only ever
expose what is in the ViewAccessibility cache and we'll get rid of
View::GetAccessibleNodeData. However, today, we still care about
maintaining the right ordering between both systems to expose the
right value.

Before we landed CL:5537333, we carefully fixed all views whose tests
indicated a regression. However, several more views did not have the
right accessible tests in place (or did not have any) to prevent such
a regression.

While no user facing bug has been reported thus far in Canary or Beta,
we decided it is safer to fix all potential regressions we introduced
in that change. It's possible that some views are currently exposed
with a button role instead of a toggle button role, for example, and
that the accessible name/description of a bookmark doesn't change
after it gets renamed. Again, we are not aware of any user facing bug
at the moment, but it doesn't mean that there aren't any -- we have a
small population of AT users in Canary and Beta.

The issue happens when a base class sets its role, name, or
description in the cache through a setter, while a subclass sets it
in a View::GetAccessibleNodeData override. The value set in the
override should be the one exposed, but the one set on the base class
ends up being the one that does.

Here's an example:
* InMenuButton should be exposed as a menu item, so it sets its role
  to kMenuItem in GetAccessibleNodeData.
* InMenuButton extends LabelButton, which extends Button.
* Button sets its role to kButton directly in the ViewAccessibility
  cache.
* The role exposed for InMenuButton is kButton instead of kMenuItem.

This CL fixes all instances of this issue in //ash/.

In some of these, the attribute already existed in the cache,
so I just removed it from the View's GetAccessibleNodeData.

The changes to the test files had to be done because those were
using the wrong function to get the accessible data. They were
using View::GetAccessibleNodeData which is wrong because ATs
will always route through ViewAccessibility::GetAccessibleNodeData
(which calls the View's). So these were updated, otherwise the tests
would never test the cached accessible data that lives in
ViewAccessibility.

Given that CL:5537333 is in 127 and 127's Stable cut is on 7/16, we
did those fixes quickly without adding new tests for now. However,
we plan to add tests in a follow up CL.

Bug: 352528963, 325137417
Change-Id: I1a4c1f05ad1ea8f137a06f73fa58dea913c7d109
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5696646
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@google.com>
Commit-Queue: Javier Contreras <javiercon@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1327135}
This commit is contained in:
javiercon@microsoft.com
2024-07-13 06:52:15 +00:00
committed by Chromium LUCI CQ
parent 3f454231a0
commit a6d1c1e900
40 changed files with 99 additions and 198 deletions

@ -27,7 +27,12 @@ SearchResultBaseView::SearchResultBaseView() {
// all relevant key events (e.g. ENTER key for result activation) to search
// result views as needed.
SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
// Mark the result is a list item in the list of search results.
// Also avoids an issue with the nested button case(append and remove
// button are child button of SearchResultView), which is not supported by
// ChromeVox. see details in crbug.com/924776.
GetViewAccessibility().SetRole(ax::mojom::Role::kListBoxOption);
UpdateAccessibleName();
}
SearchResultBaseView::~SearchResultBaseView() {
@ -87,6 +92,8 @@ void SearchResultBaseView::SetResult(SearchResult* result) {
result_->AddObserver(this);
}
OnResultChanged();
UpdateAccessibleName();
}
void SearchResultBaseView::OnResultDestroying() {
@ -138,27 +145,21 @@ void SearchResultBaseView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
return;
}
// Mark the result is a list item in the list of search results.
// Also avoids an issue with the nested button case(append and remove
// button are child button of SearchResultView), which is not supported by
// ChromeVox. see details in crbug.com/924776.
node_data->role = ax::mojom::Role::kListBoxOption;
node_data->SetDefaultActionVerb(ax::mojom::DefaultActionVerb::kClick);
}
void SearchResultBaseView::UpdateAccessibleName() {
// It is possible for the view to be visible but lack a result. When this
// happens, `ComputeAccessibleName()` will return an empty string. Because
// the focusable state is set in the constructor and not updated when the
// result is removed, the accessibility paint checks will fail.
if (!result()) {
node_data->SetNameExplicitlyEmpty();
return;
const std::u16string name = ComputeAccessibleName();
if (name.empty()) {
GetViewAccessibility().SetName(
name, ax::mojom::NameFrom::kAttributeExplicitlyEmpty);
} else {
GetViewAccessibility().SetName(name);
}
node_data->SetName(ComputeAccessibleName());
}
void SearchResultBaseView::UpdateAccessibleName() {
GetViewAccessibility().SetName(ComputeAccessibleName());
}
void SearchResultBaseView::ClearResult() {

@ -224,6 +224,7 @@ class CaptureModeOption
SetInkDropForButton(this);
GetViewAccessibility().SetIsLeaf(true);
GetViewAccessibility().SetName(GetOptionLabel());
GetViewAccessibility().SetRole(ax::mojom::Role::kRadioButton);
SetEnabled(enabled);
}
@ -289,8 +290,6 @@ class CaptureModeOption
void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
Button::GetAccessibleNodeData(node_data);
node_data->role = ax::mojom::Role::kRadioButton;
node_data->SetName(GetOptionLabel());
node_data->SetCheckedState(IsOptionChecked()
? ax::mojom::CheckedState::kTrue
: ax::mojom::CheckedState::kFalse);

@ -141,6 +141,9 @@ ResizeToggleMenu::MenuButtonView::MenuButtonView(PressedCallback callback,
views::FocusRing::Get(this)->SetColorId(
static_cast<ui::ColorId>(cros_tokens::kCrosSysFocusRing));
}
GetViewAccessibility().SetRole(ax::mojom::Role::kMenuItem);
GetViewAccessibility().SetName(l10n_util::GetStringUTF16(title_string_id_));
}
ResizeToggleMenu::MenuButtonView::~MenuButtonView() = default;
@ -446,11 +449,4 @@ void ResizeToggleMenu::CloseBubble() {
bubble_widget_->CloseWithReason(views::Widget::ClosedReason::kUnspecified);
}
void ResizeToggleMenu::MenuButtonView::GetAccessibleNodeData(
ui::AXNodeData* node_data) {
views::Button::GetAccessibleNodeData(node_data);
node_data->role = ax::mojom::Role::kMenuItem;
node_data->SetName(l10n_util::GetStringUTF16(title_string_id_));
}
} // namespace arc

@ -51,9 +51,6 @@ class ResizeToggleMenu : public views::WidgetObserver,
void SetSelected(bool is_selected);
// views::View:
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
private:
// views::View:
void OnThemeChanged() override;

@ -221,14 +221,14 @@ class GlanceablesTaskView::CheckButton : public views::ImageButton {
UpdateImage();
SetFlipCanvasOnPaintForRTLUI(/*enable=*/false);
views::FocusRing::Get(this)->SetColorId(cros_tokens::kCrosSysFocusRing);
GetViewAccessibility().SetName(l10n_util::GetStringUTF16(
IDS_GLANCEABLES_TASKS_TASK_ITEM_MARK_COMPLETED_ACCESSIBLE_NAME));
}
void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
views::ImageButton::GetAccessibleNodeData(node_data);
node_data->SetName(l10n_util::GetStringUTF16(
IDS_GLANCEABLES_TASKS_TASK_ITEM_MARK_COMPLETED_ACCESSIBLE_NAME));
const ax::mojom::CheckedState checked_state =
checked_ ? ax::mojom::CheckedState::kTrue
: ax::mojom::CheckedState::kFalse;

@ -344,6 +344,8 @@ class AuthDialogContentsView::TitleLabel : public views::Label {
SetPreferredSize(gfx::Size(kContainerPreferredWidth,
GetHeightForWidth(kContainerPreferredWidth)));
SetHorizontalAlignment(gfx::ALIGN_CENTER);
GetViewAccessibility().SetRole(ax::mojom::Role::kStaticText);
}
bool IsShowingError() const { return is_showing_error_; }
@ -366,12 +368,6 @@ class AuthDialogContentsView::TitleLabel : public views::Label {
true /*send_native_event*/);
}
// views::View
void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
node_data->role = ax::mojom::Role::kStaticText;
node_data->SetNameChecked(GetViewAccessibility().GetCachedName());
}
private:
bool is_showing_error_ = false;
};

@ -7,6 +7,7 @@
#include "ui/accessibility/ax_node_data.h"
#include "ui/base/metadata/metadata_impl_macros.h"
#include "ui/base/models/image_model.h"
#include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/controls/image_view.h"
#include "ui/views/controls/label.h"
@ -22,6 +23,8 @@ BottomStatusIndicator::BottomStatusIndicator(TappedCallback on_tapped_callback)
SetFocusBehavior(FocusBehavior::ALWAYS);
SetVisible(false);
GetViewAccessibility().SetRole(ax::mojom::Role::kStaticText);
}
BottomStatusIndicator::~BottomStatusIndicator() = default;
@ -34,11 +37,6 @@ void BottomStatusIndicator::SetIcon(const gfx::VectorIcon& vector_icon,
ui::ImageModel::FromVectorIcon(vector_icon, color_id, icon_size));
}
void BottomStatusIndicator::GetAccessibleNodeData(ui::AXNodeData* node_data) {
node_data->role = role_;
node_data->SetName(label()->GetText());
}
BEGIN_METADATA(BottomStatusIndicator)
END_METADATA

@ -33,17 +33,11 @@ class BottomStatusIndicator final : public views::LabelButton {
ui::ColorId color_id,
int icon_size = 0);
void set_role_for_accessibility(ax::mojom::Role role) { role_ = role; }
// views::View:
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
base::WeakPtr<BottomStatusIndicator> AsWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
}
private:
ax::mojom::Role role_ = ax::mojom::Role::kStaticText;
base::WeakPtrFactory<BottomStatusIndicator> weak_ptr_factory_{this};
};

@ -503,7 +503,7 @@ void LockContentsView::ShowEnterpriseDomainManager(
bottom_status_indicator_->SetText(l10n_util::GetStringFUTF16(
IDS_ASH_LOGIN_MANAGED_DEVICE_INDICATOR, ui::GetChromeOSDeviceName(),
base::UTF8ToUTF16(entreprise_domain_manager)));
bottom_status_indicator_->set_role_for_accessibility(
bottom_status_indicator_->GetViewAccessibility().SetRole(
ax::mojom::Role::kButton);
bottom_status_indicator_state_ = BottomIndicatorState::kManagedDevice;
UpdateBottomStatusIndicatorColors();
@ -513,7 +513,7 @@ void LockContentsView::ShowEnterpriseDomainManager(
void LockContentsView::ShowAdbEnabled() {
bottom_status_indicator_->SetText(
l10n_util::GetStringUTF16(IDS_ASH_LOGIN_SCREEN_UNVERIFIED_CODE_WARNING));
bottom_status_indicator_->set_role_for_accessibility(
bottom_status_indicator_->GetViewAccessibility().SetRole(
ax::mojom::Role::kStaticText);
bottom_status_indicator_state_ = BottomIndicatorState::kAdbSideLoadingEnabled;
UpdateBottomStatusIndicatorColors();

@ -353,7 +353,11 @@ class LoginUserView::TapButton : public views::Button {
public:
TapButton(PressedCallback callback, LoginUserView* parent)
: views::Button(std::move(callback)), parent_(parent) {}
: views::Button(std::move(callback)), parent_(parent) {
// TODO(https://crbug.com/1065516): Define the button name.
GetViewAccessibility().SetName(
"", ax::mojom::NameFrom::kAttributeExplicitlyEmpty);
}
TapButton(const TapButton&) = delete;
TapButton& operator=(const TapButton&) = delete;
@ -369,11 +373,6 @@ class LoginUserView::TapButton : public views::Button {
views::Button::OnBlur();
parent_->UpdateOpacity();
}
void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
// TODO(https://crbug.com/1065516): Define the button name.
node_data->SetNameExplicitlyEmpty();
Button::GetAccessibleNodeData(node_data);
}
private:
const raw_ptr<LoginUserView> parent_;

@ -28,7 +28,7 @@ constexpr gfx::Insets kButtonPadding{0};
ProjectorButton::ProjectorButton(views::Button::PressedCallback callback,
const std::u16string& name)
: ToggleImageButton(std::move(callback)), name_(name) {
: ToggleImageButton(std::move(callback)) {
SetPreferredSize(gfx::Size(kProjectorButtonSize, kProjectorButtonSize));
SetBorder(views::CreateEmptyBorder(kButtonPadding));
@ -41,6 +41,9 @@ ProjectorButton::ProjectorButton(views::Button::PressedCallback callback,
/*highlight_on_focus=*/true);
SetTooltipText(name);
GetViewAccessibility().SetRole(ax::mojom::Role::kButton);
GetViewAccessibility().SetName(name);
}
void ProjectorButton::OnPaintBackground(gfx::Canvas* canvas) {
@ -78,11 +81,6 @@ void ProjectorButton::OnThemeChanged() {
this, StyleUtil::kBaseColor | StyleUtil::kHighlightOpacity);
}
void ProjectorButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
node_data->role = ax::mojom::Role::kButton;
node_data->SetName(name_);
}
BEGIN_METADATA(ProjectorButton)
END_METADATA

@ -31,10 +31,6 @@ class ASH_EXPORT ProjectorButton : public views::ToggleImageButton {
// views::ToggleImageButton:
void OnPaintBackground(gfx::Canvas* canvas) override;
void OnThemeChanged() override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
private:
std::u16string name_;
};
} // namespace ash

@ -51,6 +51,8 @@ void ShelfButton::OnThemeChanged() {
StyleUtil::ConfigureInkDropAttributes(
this, StyleUtil::kBaseColor | StyleUtil::kInkDropOpacity);
}
GetViewAccessibility().SetRole(ax::mojom::Role::kButton);
}
gfx::Rect ShelfButton::GetAnchorBoundsInScreen() const {
@ -67,12 +69,6 @@ void ShelfButton::AboutToRequestFocusFromTabTraversal(bool reverse) {
this, reverse);
}
// Do not remove this function to avoid unnecessary ChromeVox announcement
// triggered by Button::GetAccessibleNodeData. (See https://crbug.com/932200)
void ShelfButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
node_data->role = ax::mojom::Role::kButton;
}
////////////////////////////////////////////////////////////////////////////////
// views::Button

@ -30,7 +30,6 @@ class ASH_EXPORT ShelfButton : public views::Button {
void OnThemeChanged() override;
gfx::Rect GetAnchorBoundsInScreen() const override;
void AboutToRequestFocusFromTabTraversal(bool reverse) override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
void NotifyClick(const ui::Event& event) override;
Shelf* shelf() { return shelf_; }

@ -93,11 +93,6 @@ gfx::Size ShelfControlButton::CalculatePreferredSize(
ShelfConfig::Get()->control_size());
}
void ShelfControlButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
ShelfButton::GetAccessibleNodeData(node_data);
node_data->SetNameChecked(GetViewAccessibility().GetCachedName());
}
BEGIN_METADATA(ShelfControlButton)
END_METADATA

@ -37,7 +37,6 @@ class ASH_EXPORT ShelfControlButton : public ShelfButton {
// ShelfButton:
gfx::Size CalculatePreferredSize(
const views::SizeBounds& available_size) const override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
private:
gfx::Rect ideal_bounds_;

@ -29,7 +29,9 @@ Checkbox::Checkbox(int button_width,
std::move(callback),
label,
insets,
image_label_spacing) {}
image_label_spacing) {
GetViewAccessibility().SetRole(ax::mojom::Role::kCheckBox);
}
Checkbox::~Checkbox() = default;
@ -45,17 +47,6 @@ bool Checkbox::IsIconOnTheLeftSide() {
return true;
}
void Checkbox::GetAccessibleNodeData(ui::AXNodeData* node_data) {
OptionButtonBase::GetAccessibleNodeData(node_data);
node_data->role = ax::mojom::Role::kCheckBox;
const std::u16string cached_name = GetViewAccessibility().GetCachedName();
// Explicitly set the name so that this is compatible with
// `MenuItemView::GetAccessibleNodeData()`.
if (!cached_name.empty()) {
node_data->SetName(cached_name);
}
}
BEGIN_METADATA(Checkbox)
END_METADATA

@ -35,7 +35,6 @@ class ASH_EXPORT Checkbox : public OptionButtonBase {
gfx::ImageSkia GetImage(ButtonState for_state) const override;
const gfx::VectorIcon& GetVectorIcon() const override;
bool IsIconOnTheLeftSide() override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
};
} // namespace ash

@ -130,6 +130,8 @@ class ComboboxMenuOptionGroup : public RadioButtonGroup {
kMenuItemInnerPadding,
kCheckmarkLabelSpacing) {
GetViewAccessibility().SetProperties(ax::mojom::Role::kListBox);
GetViewAccessibility().SetName(
"", ax::mojom::NameFrom::kAttributeExplicitlyEmpty);
}
// RadioButtonGroup:
@ -142,11 +144,6 @@ class ComboboxMenuOptionGroup : public RadioButtonGroup {
buttons_.push_back(button);
return button;
}
void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
RadioButtonGroup::GetAccessibleNodeData(node_data);
node_data->SetNameExplicitlyEmpty();
}
};
BEGIN_METADATA(ComboboxMenuOptionGroup)

@ -577,17 +577,6 @@ void IconButton::PaintButtonContents(gfx::Canvas* canvas) {
views::ImageButton::PaintButtonContents(canvas);
}
void IconButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
views::ImageButton::GetAccessibleNodeData(node_data);
if (is_togglable_) {
node_data->role = ax::mojom::Role::kToggleButton;
node_data->SetCheckedState(toggled_ ? ax::mojom::CheckedState::kTrue
: ax::mojom::CheckedState::kFalse);
} else {
node_data->role = ax::mojom::Role::kButton;
}
}
void IconButton::NotifyClick(const ui::Event& event) {
if (is_togglable_) {
chromeos::haptics_util::PlayHapticToggleEffect(

@ -196,7 +196,6 @@ class ASH_EXPORT IconButton : public views::ImageButton {
void OnFocus() override;
void OnBlur() override;
void PaintButtonContents(gfx::Canvas* canvas) override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
void NotifyClick(const ui::Event& event) override;
protected:

@ -15,6 +15,7 @@
#include "ui/compositor/layer.h"
#include "ui/gfx/canvas.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/border.h"
#include "ui/views/controls/highlight_path_generator.h"
#include "ui/views/controls/image_view.h"
@ -75,6 +76,7 @@ TabSliderButton::TabSliderButton(PressedCallback callback,
views::InstallPillHighlightPathGenerator(this);
SetTooltipText(tooltip_text);
GetViewAccessibility().SetRole(ax::mojom::Role::kToggleButton);
}
TabSliderButton::~TabSliderButton() = default;
@ -106,9 +108,6 @@ SkColor TabSliderButton::GetColorIdOnButtonState() {
void TabSliderButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
Button::GetAccessibleNodeData(node_data);
const std::u16string tooltip = GetTooltipText(gfx::Point());
node_data->role = ax::mojom::Role::kToggleButton;
node_data->SetName(tooltip);
node_data->SetCheckedState(selected_ ? ax::mojom::CheckedState::kTrue
: ax::mojom::CheckedState::kFalse);
}

@ -27,6 +27,7 @@
#include "ui/accessibility/accessibility_features.h"
#include "ui/accessibility/ax_enums.mojom-shared.h"
#include "ui/accessibility/ax_node_data.h"
#include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/controls/label.h"
#include "ui/views/view.h"
#include "ui/views/view_utils.h"
@ -139,7 +140,7 @@ speech::LanguageCode fr_fr() {
// Returns true if `view` is marked checked for accessibility.
bool IsCheckedForAccessibility(views::View* view) {
ui::AXNodeData node_data;
view->GetAccessibleNodeData(&node_data);
view->GetViewAccessibility().GetAccessibleNodeData(&node_data);
return node_data.GetCheckedState() == ax::mojom::CheckedState::kTrue;
}

@ -151,12 +151,6 @@ void FloatingMenuButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
return;
}
views::ImageButton::GetAccessibleNodeData(node_data);
if (!is_a11y_togglable_) {
return;
}
node_data->role = ax::mojom::Role::kToggleButton;
node_data->SetCheckedState(toggled_ ? ax::mojom::CheckedState::kTrue
: ax::mojom::CheckedState::kFalse);
}
void FloatingMenuButton::UpdateImage() {

@ -142,7 +142,6 @@ class ImeListItemView : public views::Button {
void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
views::Button::GetAccessibleNodeData(node_data);
node_data->role = ax::mojom::Role::kCheckBox;
node_data->SetCheckedState(selected_ ? ax::mojom::CheckedState::kTrue
: ax::mojom::CheckedState::kFalse);
}

@ -90,6 +90,7 @@ class LocaleItemView : public views::Button {
tri_view->AddView(TriView::Container::END, checked_image);
}
GetViewAccessibility().SetName(display_name_view->GetText());
GetViewAccessibility().SetRole(ax::mojom::Role::kCheckBox);
}
LocaleItemView(const LocaleItemView&) = delete;
LocaleItemView& operator=(const LocaleItemView&) = delete;
@ -107,7 +108,6 @@ class LocaleItemView : public views::Button {
void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
views::Button::GetAccessibleNodeData(node_data);
node_data->role = ax::mojom::Role::kCheckBox;
node_data->SetCheckedState(checked_ ? ax::mojom::CheckedState::kTrue
: ax::mojom::CheckedState::kFalse);
}

@ -279,32 +279,11 @@ void HoverHighlightView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
if (right_view_ && right_view_->GetVisible() &&
std::string(right_view_->GetClassName()).find("Button") !=
std::string::npos) {
// Allow selection of sub-components.
node_data->role = ax::mojom::Role::kGenericContainer;
// Include "press search plus space to activate" when announcing.
node_data->SetDefaultActionVerb(ax::mojom::DefaultActionVerb::kClick);
node_data->SetName(GetViewAccessibility().GetCachedName());
node_data->SetDescription(
l10n_util::GetStringUTF16(IDS_ASH_A11Y_ROLE_BUTTON));
} else {
views::Button::GetAccessibleNodeData(node_data);
}
ax::mojom::CheckedState checked_state;
if (accessibility_state_ == AccessibilityState::CHECKED_CHECKBOX) {
checked_state = ax::mojom::CheckedState::kTrue;
} else if (accessibility_state_ == AccessibilityState::UNCHECKED_CHECKBOX) {
checked_state = ax::mojom::CheckedState::kFalse;
} else {
return; // Not a checkbox
}
// Checkbox
node_data->role = ax::mojom::Role::kCheckBox;
node_data->SetCheckedState(checked_state);
}
gfx::Size HoverHighlightView::CalculatePreferredSize(

@ -603,7 +603,10 @@ void TrayBackgroundView::AboutToRequestFocusFromTabTraversal(bool reverse) {
void TrayBackgroundView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
views::Button::GetAccessibleNodeData(node_data);
node_data->SetName(GetAccessibleNameForTray());
// Override the name set in `LabelButton::SetText`.
// TODO(crbug.com/325137417): Remove this once the accessible name is set in
// the cache as soon as the name is updated.
GetViewAccessibility().SetName(GetAccessibleNameForTray());
if (LockScreen::HasInstance()) {
GetViewAccessibility().SetNextFocus(LockScreen::Get()->widget());

@ -656,20 +656,6 @@ void FeatureTile::SetDownloadState(DownloadState state, int progress) {
NotifyDownloadStateChanged();
}
void FeatureTile::GetAccessibleNodeData(ui::AXNodeData* node_data) {
views::Button::GetAccessibleNodeData(node_data);
// If the icon is clickable then the main feature tile usually takes the user
// to a detailed page (like Network or Bluetooth). Those tiles act more like a
// regular button than a toggle button.
if (is_togglable_ && !is_icon_clickable_) {
node_data->role = ax::mojom::Role::kToggleButton;
node_data->SetCheckedState(toggled_ ? ax::mojom::CheckedState::kTrue
: ax::mojom::CheckedState::kFalse);
} else {
node_data->role = ax::mojom::Role::kButton;
}
}
void FeatureTile::AddLayerToRegion(ui::Layer* layer,
views::LayerRegion region) {
// This routes background layers to `ink_drop_container_` instead of `this` to

@ -204,7 +204,6 @@ class ASH_EXPORT FeatureTile : public views::Button {
void SetDownloadState(DownloadState state, int progress);
// views::View:
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
void AddLayerToRegion(ui::Layer* layer, views::LayerRegion region) override;
void RemoveLayerFromRegions(ui::Layer* layer) override;

@ -23,6 +23,7 @@
#include "ui/base/l10n/l10n_util.h"
#include "ui/chromeos/styles/cros_tokens_color_mappings.h"
#include "ui/gfx/geometry/rrect_f.h"
#include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/animation/ink_drop.h"
#include "ui/views/animation/ink_drop_state.h"
#include "ui/views/controls/highlight_path_generator.h"
@ -483,13 +484,13 @@ TEST_P(FeatureTileTest, AccessibilityRoles) {
/*is_togglable=*/true);
togglable_tile.SetToggled(true);
ui::AXNodeData node_data;
togglable_tile.GetAccessibleNodeData(&node_data);
togglable_tile.GetViewAccessibility().GetAccessibleNodeData(&node_data);
EXPECT_EQ(node_data.role, ax::mojom::Role::kToggleButton);
EXPECT_EQ(node_data.GetCheckedState(), ax::mojom::CheckedState::kTrue);
togglable_tile.SetToggled(false);
ui::AXNodeData node_data2;
togglable_tile.GetAccessibleNodeData(&node_data2);
togglable_tile.GetViewAccessibility().GetAccessibleNodeData(&node_data2);
EXPECT_EQ(node_data2.role, ax::mojom::Role::kToggleButton);
EXPECT_EQ(node_data2.GetCheckedState(), ax::mojom::CheckedState::kFalse);
@ -498,14 +499,14 @@ TEST_P(FeatureTileTest, AccessibilityRoles) {
// tile takes the user to a detail page.
togglable_tile.SetIconClickable(true);
ui::AXNodeData node_data3;
togglable_tile.GetAccessibleNodeData(&node_data3);
togglable_tile.GetViewAccessibility().GetAccessibleNodeData(&node_data3);
EXPECT_EQ(node_data3.role, ax::mojom::Role::kButton);
// Non-togglable feature tiles are just buttons.
FeatureTile non_togglable_tile(views::Button::PressedCallback(),
/*is_togglable=*/false);
ui::AXNodeData node_data4;
non_togglable_tile.GetAccessibleNodeData(&node_data4);
non_togglable_tile.GetViewAccessibility().GetAccessibleNodeData(&node_data4);
EXPECT_EQ(node_data4.role, ax::mojom::Role::kButton);
}

@ -142,18 +142,16 @@ QsBatteryInfoViewBase::QsBatteryInfoViewBase(
SetImageLabelSpacing(kImageLabelSpacing);
TypographyProvider::Get()->StyleLabel(TypographyToken::kCrosButton2,
*label());
GetViewAccessibility().SetName(
PowerStatus::Get()->GetAccessibleNameString(/*full_description=*/true));
GetViewAccessibility().SetRole(ax::mojom::Role::kButton);
}
QsBatteryInfoViewBase::~QsBatteryInfoViewBase() {
PowerStatus::Get()->RemoveObserver(this);
}
void QsBatteryInfoViewBase::GetAccessibleNodeData(ui::AXNodeData* node_data) {
node_data->role = ax::mojom::Role::kButton;
node_data->SetName(
PowerStatus::Get()->GetAccessibleNameString(/*full_description=*/true));
}
void QsBatteryInfoViewBase::ChildPreferredSizeChanged(views::View* child) {
PreferredSizeChanged();
}
@ -185,6 +183,8 @@ void QsBatteryInfoViewBase::UpdateIconAndText(bool bsm_active) {
const std::u16string percentage_text =
PowerStatus::Get()->GetStatusStrings().first;
SetText(percentage_text);
GetViewAccessibility().SetName(
PowerStatus::Get()->GetAccessibleNameString(/*full_description=*/true));
SetVisible(!percentage_text.empty());
if (GetColorProvider()) {

@ -48,8 +48,6 @@ class ASH_EXPORT QsBatteryInfoViewBase : public PillButton,
void ConfigureIcon(bool bsm_active = false);
private:
// views::View:
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
void ChildPreferredSizeChanged(views::View* child) override;
void ChildVisibilityChanged(views::View* child) override;

@ -286,6 +286,11 @@ UserItemButton::UserItemButton(PressedCallback callback,
SetTooltipText(GetUserItemAccessibleString(user_index));
SetFocusPainter(TrayPopupUtils::CreateFocusPainter());
// The button for the currently active user is not clickable.
GetViewAccessibility().SetRole(user_index_ == 0 ? ax::mojom::Role::kLabelText
: ax::mojom::Role::kButton);
GetViewAccessibility().SetName(GetUserItemAccessibleString(user_index_));
}
void UserItemButton::SetCaptureState(MediaCaptureState capture_state) {
@ -322,13 +327,6 @@ std::u16string UserItemButton::GetTooltipText(const gfx::Point& p) const {
return views::Button::GetTooltipText(p);
}
void UserItemButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
// The button for the currently active user is not clickable.
node_data->role =
user_index_ == 0 ? ax::mojom::Role::kLabelText : ax::mojom::Role::kButton;
node_data->SetName(GetUserItemAccessibleString(user_index_));
}
BEGIN_METADATA(UserItemButton)
END_METADATA

@ -47,7 +47,6 @@ class UserItemButton : public views::Button {
// views::Button:
std::u16string GetTooltipText(const gfx::Point& p) const override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
private:
const int user_index_;

@ -58,7 +58,13 @@ namespace ash {
// DeskButton:
DeskButton::DeskButton()
: views::Button(base::BindRepeating(&DeskButton::OnButtonPressed,
base::Unretained(this))) {}
base::Unretained(this))) {
// Avoid failing accessibility checks if we don't have a name.
if (GetViewAccessibility().GetCachedName().empty()) {
GetViewAccessibility().SetName(
"", ax::mojom::NameFrom::kAttributeExplicitlyEmpty);
}
}
DeskButton::~DeskButton() {}
@ -153,11 +159,7 @@ void DeskButton::Layout(PassKey) {
}
void DeskButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
// Avoid failing accessibility checks if we don't have a name.
Button::GetAccessibleNodeData(node_data);
if (GetViewAccessibility().GetCachedName().empty()) {
node_data->SetNameExplicitlyEmpty();
}
ShelfWidget* shelf_widget =
Shelf::ForWindow(GetWidget()->GetNativeWindow())->shelf_widget();

@ -20,6 +20,7 @@
#include "ui/base/metadata/metadata_impl_macros.h"
#include "ui/chromeos/styles/cros_tokens_color_mappings.h"
#include "ui/compositor/layer.h"
#include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/background.h"
#include "ui/views/controls/highlight_path_generator.h"
#include "ui/views/view_class_properties.h"
@ -29,7 +30,13 @@ namespace ash {
DeskSwitchButton::DeskSwitchButton()
: ImageButton(
base::BindRepeating(&DeskSwitchButton::DeskSwitchButtonPressed,
base::Unretained(this))) {}
base::Unretained(this))) {
// Avoid failing accessibility checks if we don't have a name.
if (GetViewAccessibility().GetCachedName().empty()) {
GetViewAccessibility().SetName(
"", ax::mojom::NameFrom::kAttributeExplicitlyEmpty);
}
}
DeskSwitchButton::~DeskSwitchButton() = default;
@ -41,14 +48,6 @@ gfx::Size DeskSwitchButton::CalculatePreferredSize(
: kDeskButtonSwitchButtonHeightHorizontal);
}
void DeskSwitchButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
// Avoid failing accessibility checks if we don't have a name.
views::ImageButton::GetAccessibleNodeData(node_data);
if (GetViewAccessibility().GetCachedName().empty()) {
node_data->SetNameExplicitlyEmpty();
}
}
void DeskSwitchButton::OnMouseEvent(ui::MouseEvent* event) {
if (GetEnabled() && event->type() == ui::ET_MOUSE_PRESSED &&
event->IsOnlyRightMouseButton()) {

@ -35,7 +35,6 @@ class ASH_EXPORT DeskSwitchButton : public views::ImageButton {
// views::ImageButton:
gfx::Size CalculatePreferredSize(
const views::SizeBounds& available_size) const override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
void OnMouseEvent(ui::MouseEvent* event) override;
void OnGestureEvent(ui::GestureEvent* event) override;
void StateChanged(ButtonState old_state) override;

@ -573,16 +573,17 @@ void DeskPreviewView::UpdateAccessibleName() {
: IDS_ASH_DESKS_DESK_PREVIEW_INACTIVE,
desk->name()));
}
// Avoid failing accessibility checks if we don't have a name.
if (GetViewAccessibility().GetCachedName().empty()) {
GetViewAccessibility().SetName(
"", ax::mojom::NameFrom::kAttributeExplicitlyEmpty);
}
}
void DeskPreviewView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
views::Button::GetAccessibleNodeData(node_data);
// Avoid failing accessibility checks if we don't have a name.
if (GetViewAccessibility().GetCachedName().empty()) {
node_data->SetNameExplicitlyEmpty();
}
node_data->AddStringAttribute(
ax::mojom::StringAttribute::kRoleDescription,
l10n_util::GetStringUTF8(IDS_ASH_DESKS_DESK_PREVIEW_ROLE_DESCRIPTION));

@ -306,6 +306,8 @@ SavedDeskItemView::SavedDeskItemView(std::unique_ptr<DeskTemplate> saved_desk)
icon_container_view_->layer()->SetOpacity(1.0f);
AddAccelerator(ui::Accelerator(ui::VKEY_W, ui::EF_CONTROL_DOWN));
GetViewAccessibility().SetRole(ax::mojom::Role::kButton);
}
SavedDeskItemView::~SavedDeskItemView() {
@ -403,8 +405,12 @@ void SavedDeskItemView::UpdateSavedDesk(
}
void SavedDeskItemView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
node_data->role = ax::mojom::Role::kButton;
node_data->SetName(ComputeAccessibleName());
// We must set the updated accessible name directly in the cache to override
// the one set in `LabelButton::SetText`. This is temporary.
//
// TODO(crbug.com/325137417): Remove this once the accessible name is set in
// the cache as soon as the name is updated.
GetViewAccessibility().SetName(ComputeAccessibleName());
node_data->AddStringAttribute(
ax::mojom::StringAttribute::kDescription,