From 6a5129bc34e663fc692800b842afe2bc54d22b38 Mon Sep 17 00:00:00 2001 From: Ashish Kumar <ashishkum@microsoft.com> Date: Sun, 28 Jul 2024 13:35:50 +0000 Subject: [PATCH] [views-ax] Migrating kName attribute to new system in ui/message_center This CL migrates the kName value for all files in ui/messages_center/views/* whenever its value should change, rather than querying the value and computing it only when needed. This CL eliminates the direct assignment of the kName attribute within the View::GetAccessibleNodeData method. Instead, it introduces a mechanism to update the kName attribute in the accessibility cache when any attribute that contributes to the accessible name undergoes a change. This approach ensures that the accessible name remains up-to-date in the accessibility cache. This CL is part of the ViewsAX project: https://docs.google.com/document/d/1Ku7HOyDsiZem1yaV6ccZ-tz3lO2XR2NEcm8HjR6d-VY/edit#heading=h.ke1u3utej413 Bug: 325137417 Change-Id: I1bfb2c10377207197d12b61bae7846cd2b0d8265 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5665386 Reviewed-by: Jacques Newman <janewman@microsoft.com> Reviewed-by: Javier Contreras <javiercon@microsoft.com> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com> Commit-Queue: Ashish Kumar <ashishkum@microsoft.com> Cr-Commit-Position: refs/heads/main@{#1334007} --- ui/message_center/views/message_view.cc | 2 -- .../views/notification_header_view.cc | 16 ++++++++++++++-- .../views/notification_header_view_unittest.cc | 15 +++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/ui/message_center/views/message_view.cc b/ui/message_center/views/message_view.cc index 50302446b4149..5bbb9a54616e9 100644 --- a/ui/message_center/views/message_view.cc +++ b/ui/message_center/views/message_view.cc @@ -225,8 +225,6 @@ void MessageView::GetAccessibleNodeData(ui::AXNodeData* node_data) { if (GetViewAccessibility().GetCachedName().empty()) { node_data->SetNameFrom(ax::mojom::NameFrom::kAttributeExplicitlyEmpty); } - - node_data->SetNameChecked(GetViewAccessibility().GetCachedName()); } bool MessageView::OnMousePressed(const ui::MouseEvent& event) { diff --git a/ui/message_center/views/notification_header_view.cc b/ui/message_center/views/notification_header_view.cc index 21b15f2b07167..02ebaac0a099b 100644 --- a/ui/message_center/views/notification_header_view.cc +++ b/ui/message_center/views/notification_header_view.cc @@ -133,7 +133,6 @@ void ExpandButton::OnThemeChanged() { void ExpandButton::GetAccessibleNodeData(ui::AXNodeData* node_data) { node_data->role = ax::mojom::Role::kButton; - node_data->SetName(GetTooltipText(gfx::Point())); if (GetTooltipText().empty()) node_data->SetNameFrom(ax::mojom::NameFrom::kAttributeExplicitlyEmpty); @@ -236,6 +235,14 @@ NotificationHeaderView::NotificationHeaderView(PressedCallback callback) // Not focusable by default, only for accessibility. SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY); UpdateExpandedCollapsedAccessibleState(); + + if (app_name_view_->GetText().empty()) { + GetViewAccessibility().SetName( + app_name_view_->GetText(), + ax::mojom::NameFrom::kAttributeExplicitlyEmpty); + } else { + GetViewAccessibility().SetName(app_name_view_->GetText()); + } } NotificationHeaderView::~NotificationHeaderView() = default; @@ -268,6 +275,12 @@ void NotificationHeaderView::ClearAppIcon() { void NotificationHeaderView::SetAppName(const std::u16string& name) { app_name_view_->SetText(name); + if (name.empty()) { + GetViewAccessibility().SetName( + name, ax::mojom::NameFrom::kAttributeExplicitlyEmpty); + } else { + GetViewAccessibility().SetName(name); + } } void NotificationHeaderView::SetAppNameElideBehavior( @@ -299,7 +312,6 @@ void NotificationHeaderView::GetAccessibleNodeData(ui::AXNodeData* node_data) { Button::GetAccessibleNodeData(node_data); node_data->role = ax::mojom::Role::kGenericContainer; - node_data->SetName(app_name_view_->GetText()); node_data->SetDescription(summary_text_view_->GetText() + u" " + timestamp_view_->GetText()); } diff --git a/ui/message_center/views/notification_header_view_unittest.cc b/ui/message_center/views/notification_header_view_unittest.cc index 1550745043430..7e125547c0fe7 100644 --- a/ui/message_center/views/notification_header_view_unittest.cc +++ b/ui/message_center/views/notification_header_view_unittest.cc @@ -392,6 +392,21 @@ TEST_F(NotificationHeaderViewTest, AccessibleExpandAndCollapse) { 1); } +TEST_F(NotificationHeaderViewTest, AccessibleNameTest) { + ui::AXNodeData data; + notification_header_view_->GetViewAccessibility().GetAccessibleNodeData( + &data); + EXPECT_EQ(data.GetString16Attribute(ax::mojom::StringAttribute::kName), u""); + EXPECT_EQ(data.GetNameFrom(), ax::mojom::NameFrom::kAttributeExplicitlyEmpty); + + data = ui::AXNodeData(); + notification_header_view_->SetAppName(u"Some app name"); + notification_header_view_->GetViewAccessibility().GetAccessibleNodeData( + &data); + EXPECT_EQ(data.GetString16Attribute(ax::mojom::StringAttribute::kName), + u"Some app name"); +} + TEST_F(NotificationHeaderViewTest, MetadataTest) { views::test::TestViewMetadata(notification_header_view_); }