Skip to content

Commit

Permalink
Ensure View subclasses call View::NotifyAccessibilityEvent
Browse files Browse the repository at this point in the history
Currently, ViewAccessibility::NotifyAccessibilityEvent gets called by
some view components. See earlier patchsets.

This method was put into place with the assumption that
ViewAccessibility has a subclass that would handle the real
dispatch. This is true for some platforms e.g. with

AXPlatformNodeDelegate(s), but it isn't true on Chrome OS.

It also misses the call to View::OnAccessibilityEvent within
View::NotifyAccessibilityEvent which a views View can override.

Fix by making this method private/renaming to *Internal
 and only allowing the call from
View::NotifyAccessibilityEvent.

Change-Id: I50493f5d77fe8ed3ba422681f263a0ca703fae7c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3198376
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/main@{#927107}
  • Loading branch information
dtsengchromium authored and Chromium LUCI CQ committed Oct 1, 2021
1 parent d7384e2 commit c952d4e
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 6 deletions.
4 changes: 2 additions & 2 deletions ash/app_list/views/contents_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,8 @@ void ContentsView::UpdateExpandArrowBehavior(AppListViewState target_state) {
// Allow ChromeVox to focus the expand arrow only when peeking launcher.
expand_arrow_view_->GetViewAccessibility().OverrideIsIgnored(
!expand_arrow_enabled);
expand_arrow_view_->GetViewAccessibility().NotifyAccessibilityEvent(
ax::mojom::Event::kTreeChanged);
expand_arrow_view_->NotifyAccessibilityEvent(ax::mojom::Event::kTreeChanged,
true);

expand_arrow_view_->MaybeEnableHintingAnimation(expand_arrow_enabled);
}
Expand Down
2 changes: 1 addition & 1 deletion ash/search_box/search_box_view_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ class SearchBoxTextfield : public views::Textfield {
auto& accessibility = GetViewAccessibility();
if (accessibility.IsIgnored()) {
accessibility.OverrideIsIgnored(false);
accessibility.NotifyAccessibilityEvent(ax::mojom::Event::kTreeChanged);
NotifyAccessibilityEvent(ax::mojom::Event::kTreeChanged, true);
}
}

Expand Down
2 changes: 1 addition & 1 deletion ui/views/accessibility/view_accessibility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ void ViewAccessibility::EndPopupFocusOverride() {
}

void ViewAccessibility::FireFocusAfterMenuClose() {
NotifyAccessibilityEvent(ax::mojom::Event::kFocusAfterMenuClose);
view_->NotifyAccessibilityEvent(ax::mojom::Event::kFocusAfterMenuClose, true);
}

void ViewAccessibility::OverrideRole(const ax::mojom::Role role) {
Expand Down
7 changes: 5 additions & 2 deletions ui/views/accessibility/view_accessibility.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,6 @@ class VIEWS_EXPORT ViewAccessibility {
// it may be a native accessible object implemented by another class.
virtual gfx::NativeViewAccessible GetNativeObject() const;

virtual void NotifyAccessibilityEvent(ax::mojom::Event event_type);

// Causes the screen reader to announce |text|. If the current user is not
// using a screen reader, has no effect.
virtual void AnnounceText(const std::u16string& text);
Expand Down Expand Up @@ -230,10 +228,15 @@ class VIEWS_EXPORT ViewAccessibility {
protected:
explicit ViewAccessibility(View* view);

// Used internally and by View.
virtual void NotifyAccessibilityEvent(ax::mojom::Event event_type);

// Used for testing. Called every time an accessibility event is fired.
AccessibilityEventsCallback accessibility_events_callback_;

private:
friend class View;

// Weak. Owns this.
View* const view_;

Expand Down

0 comments on commit c952d4e

Please sign in to comment.