a11y/Mac: Add screenreader support for SubtleNotificationView announcements.

chrome/browser/ui/views/subtle_notification_view.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/subtle_notification_view.h"
 
 #include <memory>
 
 #include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/accessibility/ax_view_state.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/font_list.h"
 #include "ui/views/bubble/bubble_border.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/widget/widget.h"
+#include "ui/views/widget/widget_observer.h"
 
 namespace {
 
 // Space between the site info label and the link.
 const int kMiddlePaddingPx = 30;
 
 const int kOuterPaddingHorizPx = 40;
 const int kOuterPaddingVertPx = 8;
 
 // Partially-transparent background color.
 const SkColor kBackgroundColor = SkColorSetARGB(0xcc, 0x28, 0x2c, 0x32);
 
 // Spacing around the key name.
 const int kKeyNameMarginHorizPx = 7;
 const int kKeyNameBorderPx = 1;
 const int kKeyNameCornerRadius = 2;
 const int kKeyNamePaddingPx = 5;
 
+// Delimiter indicating there should be a segment displayed as a keyboard key.
+const base::char16 kKeyNameDelimiter[] = {'|', '\0'};
+
 }  // namespace
 
 // Class containing the instruction text. Contains fancy styling on the keyboard
 // key (not just a simple label).
 class SubtleNotificationView::InstructionView : public views::View {
  public:
   // Creates an InstructionView with specific text. |text| may contain one or
   // more segments delimited by a pair of pipes ('|'); each of these segments
   // will be displayed as a keyboard key. e.g., "Press |Alt|+|Q| to exit" will
   // have "Alt" and "Q" rendered as keys.
   InstructionView(const base::string16& text,
                   const gfx::FontList& font_list,
                   SkColor foreground_color,
                   SkColor background_color);
 
+  base::string16 text() const { return text_; }
   void SetText(const base::string16& text);
 
  private:
   // Adds a label to the end of the notification text. If |format_as_key|,
   // surrounds the label in a rounded-rect border to indicate that it is a
   // keyboard key.
   void AddTextSegment(const base::string16& text, bool format_as_key);
 
   const gfx::FontList& font_list_;
   SkColor foreground_color_;
@@ skipping 22 matching lines @@
 
 void SubtleNotificationView::InstructionView::SetText(
     const base::string16& text) {
   // Avoid replacing the contents with the same text.
   if (text == text_)
     return;
 
   RemoveAllChildViews(true);
 
   // Parse |text|, looking for pipe-delimited segment.
-  std::vector<base::string16> segments =
-      base::SplitString(text, base::ASCIIToUTF16("|"), base::TRIM_WHITESPACE,
-                        base::SPLIT_WANT_ALL);
+  std::vector<base::string16> segments = base::SplitString(
+      text, kKeyNameDelimiter, base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
   // SplitString() returns empty strings for zero-length segments, so given an
   // even number of pipes, there should always be an odd number of segments.
   // The exception is if |text| is entirely empty, in which case the returned
   // list is also empty (rather than containing a single empty string).
   DCHECK(segments.empty() || segments.size() % 2 == 1);
 
   // Add text segment, alternating between non-key (no border) and key (border)
   // formatting.
   bool format_as_key = false;
   for (const auto& segment : segments) {
@@ skipping 67 matching lines @@
 }
 
 SubtleNotificationView::~SubtleNotificationView() {}
 
 void SubtleNotificationView::UpdateContent(
     const base::string16& instruction_text,
     const base::string16& link_text) {
   instruction_view_->SetText(instruction_text);
   instruction_view_->SetVisible(!instruction_text.empty());
   link_->SetText(link_text);
   link_->SetVisible(!link_text.empty());

[Matt Giuca, 2016/07/14 01:16:02]

I think you have to clear accessible_name_ here, or else GetAccessibleState may
return stale data (it only updates the cache if it is empty).

[Patti Lor, 2016/07/19 01:31:51]

:O Good catch thank you! I ended up changing the way |accessible_name_| works
entirely because of this, see my other comment (on patch set 6).

+
+  NotifyAccessibilityEvent(ui::AX_EVENT_TEXT_CHANGED, true);

[dmazzoni, 2016/07/13 23:41:35]

I think AX_EVENT_CHILDREN_CHANGED would be better since |instruction_view_| and
|link_| are the ones changing.

[Patti Lor, 2016/07/19 01:31:51]

Done, thanks. Would you say |link_| and |instruction_view_| need calls to
NotifyAccessibilityEvent() in their SetText() implementations as well, or is
that fine?

[tapted, 2016/07/19 02:46:53]

Yeah - this may depend on how we solve http://crbug.com/610585 (moving the
VoiceOver cursor) and http://crbug.com/610589 (keeping the views::Label inside a
views::Button out of the a11y tree).

In this case, |instruction_view_| has alternating views::Labels and views::Views
(containing a single views:Label). But we wouldn't want the VoiceOver cursor to
iterate over those separately -- they should stop at SubtleNotificationView [or,
possibly, InstructionView], and it should populate AXViewState. a11y hit-tests
on the broken up labels should fail (e.g. by omitting them from as nodes from
the a11y tree)

|link_| isn't used on Mac, but if it was, that can still be a child of
SubtleNotificationView. And I think views::Link::SetText should be responsible
for emitting AX_EVENT_TEXT_CHANGED (and perhaps, e.g., only if has focus).

To emit a AX_EVENT_CHILDREN_CHANGED mapped to a
NSAccessibilityLayoutChangedNotification, we'd need (a) functional a11y children
[InstructionView doesn't properly populate AXViewState right now), and (b) a way
to populate "a userInfo dictionary with the key NSAccessibilityUIElementsKey and
an array containing the UI elements that have been added or changed". That may
still be the right fix, but it should be a TODO

So I think we need a concrete use-case for this - I don't know how OSX assistive
apps will react to NSAccessibilityLayoutChangedNotification without the attached
dictionary.

I'm not even sure we should add the separate AX_EVENT_ALERT below -- is it
needed to make the native VoiceOver work correctly? Should it check that the
widget is visible first (e.g. are we sending 2 identical notifications right
now)? Is UpdateContent ever actually called whilst the widget is already
visible?  If the answers are: no, yes (yes), and no, then the stuff in
OnWidgetVisibilityChanged should be sufficient - doing extra stuff here might
tickle weird bugs.


(But if we can point to an assistive application that doesn't work correctly
without this [either on Mac or elsewhere] then we should definitely explore
this)

+  NotifyAccessibilityEvent(ui::AX_EVENT_ALERT, true);
 }
 
 // static
 views::Widget* SubtleNotificationView::CreatePopupWidget(
     gfx::NativeView parent_view,
     SubtleNotificationView* view,
     bool accept_events) {
   // Initialize the popup.
   views::Widget* popup = new views::Widget;
   views::Widget::InitParams params(views::Widget::InitParams::TYPE_POPUP);
   params.opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
   params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
   params.parent = parent_view;
   params.accept_events = accept_events;
   popup->Init(params);
   popup->SetContentsView(view);
   // We set layout manager to nullptr to prevent the widget from sizing its
   // contents to the same size as itself. This prevents the widget contents from
   // shrinking while we animate the height of the popup to give the impression
   // that it is sliding off the top of the screen.
   // TODO(mgiuca): This probably isn't necessary now that there is no slide
   // animation. Remove it.
   popup->GetRootView()->SetLayoutManager(nullptr);
 
+  // Allow accessibility events to be sent when the view is shown.
+  popup->AddObserver(view);
+
   return popup;
 }
+
+void SubtleNotificationView::SetAccessibleName(
+    const base::string16& accessible_name) {
+  base::RemoveChars(accessible_name, kKeyNameDelimiter, &accessible_name_);
+}
+
+void SubtleNotificationView::GetAccessibleState(ui::AXViewState* state) {
+  state->role = ui::AX_ROLE_LABEL_TEXT;
+
+  // If there is no accessible name set, use the displayed text, but ensure the
+  // '|' delimiters are removed.

[dmazzoni, 2016/07/13 23:41:35]

What code removes the | delimiters?

[Matt Giuca, 2016/07/14 01:16:02]

SetAccessibleName, just above, does this.

I agree this is a bit confusing and the comment probably should be split up
across the two functions.

Here:
// If there is no accessible name set, use the displayed text.

And above in SetAccessibleName:
// Ensure the '|' delimiters are removed.

[Patti Lor, 2016/07/19 01:31:51]

Have removed the side effect here, and left the comment because the code it
describes is now also in the same place. (See my other comment on patchset 6.)

+  if (accessible_name_.empty())
+    SetAccessibleName(instruction_view_->text());
+  state->name = accessible_name_;
+}
+
+void SubtleNotificationView::OnWidgetClosing(views::Widget* widget) {
+  if (widget && widget == GetWidget())
+    widget->RemoveObserver(this);
+}
+
+void SubtleNotificationView::OnWidgetVisibilityChanged(views::Widget* widget,
+                                                       bool visible) {
+  if (visible)
+    NotifyAccessibilityEvent(ui::AX_EVENT_ALERT, true);
+}