Selectively disable accessibility paint checks
This disables accessibility checking for some views that are currently
reported as focusable but without an accessible name. This lets us
enable the DCHECK without fixing all offenders, which we can shard out
separately.
This change does not enable the checks yet, that will be done in a
separate change which will be easier to revert without reverting the
system.
Bug: 1218186
Change-Id: I0610a0e8a7c58962fb4883bc0b83b9bd6159f6f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3138466
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#917895}
diff --git a/ash/ambient/ui/ambient_container_view.cc b/ash/ambient/ui/ambient_container_view.cc
index f952e02..70c97d40 100644
--- a/ash/ambient/ui/ambient_container_view.cc
+++ b/ash/ambient/ui/ambient_container_view.cc
@@ -14,6 +14,7 @@
#include "ash/public/cpp/shell_window_ids.h"
#include "ui/aura/window.h"
#include "ui/base/metadata/metadata_impl_macros.h"
+#include "ui/views/accessibility/accessibility_paint_checks.h"
#include "ui/views/background.h"
#include "ui/views/layout/fill_layout.h"
#include "ui/views/view.h"
@@ -23,6 +24,10 @@
AmbientContainerView::AmbientContainerView(AmbientViewDelegate* delegate)
: delegate_(delegate) {
+ // TODO(crbug.com/1218186): Remove this, this is in place temporarily to be
+ // able to submit accessibility checks, but this focusable View needs to
+ // add a name so that the screen reader knows what to announce.
+ SetProperty(views::kSkipAccessibilityPaintChecks, true);
SetID(AmbientViewID::kAmbientContainerView);
Init();
}
diff --git a/ash/app_list/views/app_list_item_view.cc b/ash/app_list/views/app_list_item_view.cc
index c39ee60..b02ec6b 100644
--- a/ash/app_list/views/app_list_item_view.cc
+++ b/ash/app_list/views/app_list_item_view.cc
@@ -46,6 +46,7 @@
#include "ui/gfx/skia_paint_util.h"
#include "ui/gfx/transform_util.h"
#include "ui/strings/grit/ui_strings.h"
+#include "ui/views/accessibility/accessibility_paint_checks.h"
#include "ui/views/background.h"
#include "ui/views/controls/image_view.h"
#include "ui/views/controls/label.h"
@@ -269,6 +270,10 @@
view_delegate_(view_delegate),
is_notification_indicator_enabled_(
features::IsNotificationIndicatorEnabled()) {
+ // TODO(crbug.com/1218186): Remove this, this is in place temporarily to be
+ // able to submit accessibility checks. This crashes if fetching a11y node
+ // data during paint because message_view_ is null.
+ SetProperty(views::kSkipAccessibilityPaintChecks, true);
DCHECK(grid_delegate_);
DCHECK(view_delegate_);
SetFocusBehavior(FocusBehavior::ALWAYS);
diff --git a/ash/assistant/ui/assistant_web_container_view.cc b/ash/assistant/ui/assistant_web_container_view.cc
index 7f1a90c..d8041d7b 100644
--- a/ash/assistant/ui/assistant_web_container_view.cc
+++ b/ash/assistant/ui/assistant_web_container_view.cc
@@ -18,6 +18,7 @@
#include "ui/base/window_open_disposition.h"
#include "ui/display/display.h"
#include "ui/display/screen.h"
+#include "ui/views/accessibility/accessibility_paint_checks.h"
#include "ui/views/background.h"
#include "ui/views/border.h"
#include "ui/views/layout/fill_layout.h"
@@ -39,6 +40,10 @@
AssistantWebContainerView::AssistantWebContainerView(
AssistantWebViewDelegate* web_container_view_delegate)
: web_container_view_delegate_(web_container_view_delegate) {
+ // TODO(crbug.com/1218186): Remove this, this is in place temporarily to be
+ // able to submit accessibility checks, but this focusable View needs to
+ // add a name so that the screen reader knows what to announce.
+ SetProperty(views::kSkipAccessibilityPaintChecks, true);
InitLayout();
}
diff --git a/ash/shelf/shelf_app_button.cc b/ash/shelf/shelf_app_button.cc
index a666aec..b3831e3 100644
--- a/ash/shelf/shelf_app_button.cc
+++ b/ash/shelf/shelf_app_button.cc
@@ -37,6 +37,7 @@
#include "ui/gfx/scoped_canvas.h"
#include "ui/gfx/skbitmap_operations.h"
#include "ui/gfx/transform_util.h"
+#include "ui/views/accessibility/accessibility_paint_checks.h"
#include "ui/views/animation/ink_drop.h"
#include "ui/views/animation/ink_drop_impl.h"
#include "ui/views/animation/square_ink_drop_ripple.h"
@@ -287,6 +288,11 @@
};
icon_shadows_.assign(kShadows, kShadows + base::size(kShadows));
+ // TODO(crbug.com/1218186): Remove this, this is in place temporarily to be
+ // able to submit accessibility checks. This crashes if fetching a11y node
+ // data during paint because message_view_ is null.
+ SetProperty(views::kSkipAccessibilityPaintChecks, true);
+
views::InkDrop::Get(this)->SetCreateRippleCallback(base::BindRepeating(
[](ShelfAppButton* host) -> std::unique_ptr<views::InkDropRipple> {
const gfx::Rect small_ripple_area = host->CalculateSmallRippleArea();
diff --git a/ash/shortcut_viewer/views/keyboard_shortcut_view.cc b/ash/shortcut_viewer/views/keyboard_shortcut_view.cc
index b14f4283..b1f355f8 100644
--- a/ash/shortcut_viewer/views/keyboard_shortcut_view.cc
+++ b/ash/shortcut_viewer/views/keyboard_shortcut_view.cc
@@ -45,6 +45,7 @@
#include "ui/events/types/event_type.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/gfx/presentation_feedback.h"
+#include "ui/views/accessibility/accessibility_paint_checks.h"
#include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/background.h"
#include "ui/views/border.h"
@@ -125,6 +126,10 @@
scroller->ClipHeightTo(0, 0);
scroller->SetContents(std::move(content_view));
scroller->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
+ // TODO(crbug.com/1218186): Remove this, this is in place temporarily to be
+ // able to submit accessibility checks. This crashes if fetching a11y node
+ // data during paint because message_view_ is null.
+ scroller->SetProperty(views::kSkipAccessibilityPaintChecks, true);
return scroller;
}
diff --git a/ash/system/media/unified_media_controls_view.cc b/ash/system/media/unified_media_controls_view.cc
index 80faac0..bc7e94f0 100644
--- a/ash/system/media/unified_media_controls_view.cc
+++ b/ash/system/media/unified_media_controls_view.cc
@@ -16,6 +16,7 @@
#include "components/vector_icons/vector_icons.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/paint_vector_icon.h"
+#include "ui/views/accessibility/accessibility_paint_checks.h"
#include "ui/views/animation/flood_fill_ink_drop_ripple.h"
#include "ui/views/animation/ink_drop_highlight.h"
#include "ui/views/animation/ink_drop_impl.h"
@@ -162,6 +163,10 @@
},
this)),
controller_(controller) {
+ // TODO(crbug.com/1218186): Remove this, this is in place temporarily to be
+ // able to submit accessibility checks. This crashes if fetching a11y node
+ // data during paint because message_view_ is null.
+ SetProperty(views::kSkipAccessibilityPaintChecks, true);
SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
SetBackground(views::CreateRoundedRectBackground(GetBackgroundColor(),
kMediaControlsCornerRadius));
diff --git a/ui/message_center/views/message_popup_view.cc b/ui/message_center/views/message_popup_view.cc
index f629334..3950190 100644
--- a/ui/message_center/views/message_popup_view.cc
+++ b/ui/message_center/views/message_popup_view.cc
@@ -15,6 +15,7 @@
#include "ui/message_center/public/cpp/message_center_constants.h"
#include "ui/message_center/views/message_popup_collection.h"
#include "ui/message_center/views/message_view.h"
+#include "ui/views/accessibility/accessibility_paint_checks.h"
#include "ui/views/layout/fill_layout.h"
#include "ui/views/widget/widget.h"
@@ -49,6 +50,10 @@
: message_view_(nullptr),
popup_collection_(popup_collection),
a11y_feedback_on_init_(false) {
+ // TODO(crbug.com/1218186): Remove this, this is in place temporarily to be
+ // able to submit accessibility checks. This crashes if fetching a11y node
+ // data during paint because message_view_ is null.
+ SetProperty(views::kSkipAccessibilityPaintChecks, true);
set_suppress_default_focus_handling();
SetLayoutManager(std::make_unique<views::FillLayout>());
}
diff --git a/ui/views/accessibility/accessibility_paint_checks.cc b/ui/views/accessibility/accessibility_paint_checks.cc
index 1ad68c3..8c2dfe6 100644
--- a/ui/views/accessibility/accessibility_paint_checks.cc
+++ b/ui/views/accessibility/accessibility_paint_checks.cc
@@ -13,6 +13,8 @@
namespace views {
+DEFINE_UI_CLASS_PROPERTY_KEY(bool, kSkipAccessibilityPaintChecks, false)
+
namespace {
std::string GetViewTreeAsString(View* view) {
@@ -24,6 +26,8 @@
} // namespace
void RunAccessibilityPaintChecks(View* view) {
+ if (view->GetProperty(kSkipAccessibilityPaintChecks))
+ return;
ui::AXNodeData node_data;
view->GetViewAccessibility().GetAccessibleNodeData(&node_data);
diff --git a/ui/views/accessibility/accessibility_paint_checks.h b/ui/views/accessibility/accessibility_paint_checks.h
index 70b4c96..ab0b1c4ec 100644
--- a/ui/views/accessibility/accessibility_paint_checks.h
+++ b/ui/views/accessibility/accessibility_paint_checks.h
@@ -5,6 +5,9 @@
#ifndef UI_VIEWS_ACCESSIBILITY_ACCESSIBILITY_PAINT_CHECKS_H_
#define UI_VIEWS_ACCESSIBILITY_ACCESSIBILITY_PAINT_CHECKS_H_
+#include "ui/base/class_property.h"
+#include "ui/views/views_export.h"
+
namespace views {
class View;
@@ -13,6 +16,12 @@
// View is ready to be displayed to the user it should also be accessible.
void RunAccessibilityPaintChecks(View* view);
+// Skip accessibility paint checks on a specific View.
+// TODO(pbos): Remove this key. Do not add new uses to it, instead make sure
+// that the offending View is fixed.
+VIEWS_EXPORT extern const ui::ClassProperty<bool>* const
+ kSkipAccessibilityPaintChecks;
+
} // namespace views
#endif // UI_VIEWS_ACCESSIBILITY_ACCESSIBILITY_PAINT_CHECKS_H_
diff --git a/ui/views/touchui/touch_selection_controller_impl_unittest.cc b/ui/views/touchui/touch_selection_controller_impl_unittest.cc
index 4894142..8fcfe7ee 100644
--- a/ui/views/touchui/touch_selection_controller_impl_unittest.cc
+++ b/ui/views/touchui/touch_selection_controller_impl_unittest.cc
@@ -25,6 +25,7 @@
#include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/render_text.h"
+#include "ui/views/accessibility/accessibility_paint_checks.h"
#include "ui/views/controls/textfield/textfield.h"
#include "ui/views/controls/textfield/textfield_test_api.h"
#include "ui/views/test/views_test_base.h"
@@ -90,6 +91,10 @@
void CreateTextfield() {
textfield_ = new Textfield();
+ // TODO(crbug.com/1218186): Remove this, this is in place temporarily to be
+ // able to submit accessibility checks, but this focusable View needs to
+ // add a name so that the screen reader knows what to announce.
+ textfield_->SetProperty(views::kSkipAccessibilityPaintChecks, true);
textfield_widget_ = new Widget;
Widget::InitParams params =
CreateParams(Widget::InitParams::TYPE_WINDOW_FRAMELESS);
diff --git a/ui/views/widget/widget_interactive_uitest.cc b/ui/views/widget/widget_interactive_uitest.cc
index 7c91221..0b571e2 100644
--- a/ui/views/widget/widget_interactive_uitest.cc
+++ b/ui/views/widget/widget_interactive_uitest.cc
@@ -31,6 +31,7 @@
#include "ui/events/event_utils.h"
#include "ui/events/test/event_generator.h"
#include "ui/gfx/native_widget_types.h"
+#include "ui/views/accessibility/accessibility_paint_checks.h"
#include "ui/views/bubble/bubble_dialog_delegate_view.h"
#include "ui/views/controls/textfield/textfield.h"
#include "ui/views/controls/textfield/textfield_test_api.h"
@@ -281,6 +282,17 @@
base::RepeatingTimer timer_;
};
+// TODO(pbos): Rewrite this to return unique_ptr.
+views::Textfield* CreateTextfield() {
+ auto textfield = std::make_unique<Textfield>();
+ // TODO(crbug.com/1218186): Remove this, this is in place temporarily to be
+ // able to submit accessibility checks, but this focusable View needs to
+ // add a name so that the screen reader knows what to announce. Consider
+ // adding bogus placeholder text here.
+ textfield->SetProperty(views::kSkipAccessibilityPaintChecks, true);
+ return textfield.release();
+}
+
} // namespace
class WidgetTestInteractive : public WidgetTest {
@@ -911,7 +923,7 @@
WidgetAutoclosePtr widget(CreateTopLevelNativeWidget());
widget->SetBounds(gfx::Rect(0, 0, 200, 200));
- Textfield* textfield = new Textfield;
+ Textfield* textfield = CreateTextfield();
textfield->SetBounds(0, 0, 200, 20);
textfield->SetText(u"some text");
widget->GetRootView()->AddChildView(textfield);
@@ -1261,7 +1273,7 @@
// Create a top level desktop native widget.
WidgetAutoclosePtr top_level(CreateTopLevelNativeWidget());
- Textfield* textfield = new Textfield;
+ Textfield* textfield = CreateTextfield();
textfield->SetBounds(0, 0, 200, 20);
top_level->GetRootView()->AddChildView(textfield);
ShowSync(top_level.get());
@@ -1275,7 +1287,7 @@
Widget* modal_dialog_widget = DialogDelegate::CreateDialogWidget(
dialog_delegate.release(), nullptr, top_level->GetNativeView());
modal_dialog_widget->SetBounds(gfx::Rect(0, 0, 100, 10));
- Textfield* dialog_textfield = new Textfield;
+ Textfield* dialog_textfield = CreateTextfield();
dialog_textfield->SetBounds(0, 0, 50, 5);
modal_dialog_widget->GetRootView()->AddChildView(dialog_textfield);
// Dialog widget doesn't need a ShowSync as it gains active status
@@ -1943,7 +1955,7 @@
// Test input method focus changes affected by top window activaction.
TEST_F(WidgetInputMethodInteractiveTest, MAYBE_Activation) {
WidgetAutoclosePtr widget(CreateTopLevelNativeWidget());
- Textfield* textfield = new Textfield;
+ Textfield* textfield = CreateTextfield();
widget->GetRootView()->AddChildView(textfield);
textfield->RequestFocus();
@@ -1961,8 +1973,8 @@
// Test input method focus changes affected by focus changes within 1 window.
TEST_F(WidgetInputMethodInteractiveTest, OneWindow) {
WidgetAutoclosePtr widget(CreateTopLevelNativeWidget());
- Textfield* textfield1 = new Textfield;
- Textfield* textfield2 = new Textfield;
+ Textfield* textfield1 = CreateTextfield();
+ Textfield* textfield2 = CreateTextfield();
textfield2->SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
widget->GetRootView()->AddChildView(textfield1);
widget->GetRootView()->AddChildView(textfield2);
@@ -2008,8 +2020,8 @@
child->SetBounds(gfx::Rect(0, 0, 50, 50));
child->Show();
- Textfield* textfield_parent = new Textfield;
- Textfield* textfield_child = new Textfield;
+ Textfield* textfield_parent = CreateTextfield();
+ Textfield* textfield_child = CreateTextfield();
textfield_parent->SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
parent->GetRootView()->AddChildView(textfield_parent);
child->GetRootView()->AddChildView(textfield_child);
@@ -2051,7 +2063,7 @@
// Test input method focus changes affected by textfield's state changes.
TEST_F(WidgetInputMethodInteractiveTest, TextField) {
WidgetAutoclosePtr widget(CreateTopLevelNativeWidget());
- Textfield* textfield = new Textfield;
+ Textfield* textfield = CreateTextfield();
widget->GetRootView()->AddChildView(textfield);
ShowSync(widget.get());
EXPECT_EQ(ui::TEXT_INPUT_TYPE_NONE,
@@ -2077,7 +2089,7 @@
// Test input method should not work for accelerator.
TEST_F(WidgetInputMethodInteractiveTest, AcceleratorInTextfield) {
WidgetAutoclosePtr widget(CreateTopLevelNativeWidget());
- Textfield* textfield = new Textfield;
+ Textfield* textfield = CreateTextfield();
widget->GetRootView()->AddChildView(textfield);
ShowSync(widget.get());
textfield->SetTextInputType(ui::TEXT_INPUT_TYPE_TEXT);
