MacViews a11y: Allow VoiceOver to read out views::Label word-by-word.

ui/accessibility/platform/ax_platform_node_mac.mm

Index: ui/accessibility/platform/ax_platform_node_mac.mm
diff --git a/ui/accessibility/platform/ax_platform_node_mac.mm b/ui/accessibility/platform/ax_platform_node_mac.mm
index 8430b3833f825fde8e9abde1c8d712fbb88e3204..98c3f8e97712f1aac0ab73d6acc7b3e44e86c5f5 100644
--- a/ui/accessibility/platform/ax_platform_node_mac.mm
+++ b/ui/accessibility/platform/ax_platform_node_mac.mm
@@ -230,6 +230,8 @@ void NotifyMacEvent(AXPlatformNodeCocoa* target, ui::AXEvent event_type) {
 @interface AXPlatformNodeCocoa ()
 // Helper function for string attributes that don't require extra processing.
 - (NSString*)getStringAttribute:(ui::AXStringAttribute)attribute;
+// The value that should be read out by screen readers.
+- (NSString*)getStringValue;
 @end
 
 @implementation AXPlatformNodeCocoa {
@@ -287,6 +289,13 @@ void NotifyMacEvent(AXPlatformNodeCocoa* target, ui::AXEvent event_type) {
   return nil;
 }
 
+- (NSString*)getStringValue {
+  if (node_->GetData().HasState(ui::AX_STATE_PROTECTED))

[dmazzoni, 2017/06/13 20:35:54]

I thought we were supposed to expose something for
protected fields. I'll have to look more closely at
what Safari does today.

On other platforms we just expose the bullets directly, i.e. if
the user types "hunter2" we expose "*******" as the value
but we support all cursor, selection, and value apis on that
field as if it was unprotected and really contained stars.

I'm wondering if this is still correct or it's historical or wrong.

[tapted, 2017/06/14 07:21:59]

My best guess is that macOS had a regression that got fixed, but was still
present when the code first went in, and the goal was to mimic what AppKit does.
I'm pretty sure I've looked at what AppKit was doing in the past, and it's quite
different to the current behaviour. There are some openradar notes about broken
secure text fields on Mac too - e.g. http://openradar.appspot.com/12206825 and
http://openradar.appspot.com/24607522

Anyway, I got all the secure text input stuff up to speed with current behaviour
and added test coverage.

+    return nil;
+  id value = [self AXValue];
+  return [value isKindOfClass:[NSString class]] ? value : [self AXTitle];

[dmazzoni, 2017/06/13 20:35:54]

This seems like a backwards way of doing it to me.

I'd have getStringValue return AX_ATTR_VALUE, and then
reimplement AXValue in terms of getStringValue rather
than the other way around.

Internally, "value" means the string value of a form control,
and it never means anything else. On Mac, AXValue is overloaded
to expose a lot of other things, but we should keep that logic
inside AXValue and not in helper functions that access internals,
if that makes sense.

[tapted, 2017/06/14 07:21:59]

Part of the concern was that some of the methods were assuming that AXValue
would return a string. E.g. AXNumberOfCharacters returning [[self AXValue]
length] feels dangerous to me -- it's fine if the AXValue is nil, but if it's an
NSValue, asking for length will throw an exception.

I've renamed this and moved things around -- see what you think.

+}
+
 // NSAccessibility informal protocol implementation.
 
 - (BOOL)accessibilityIsIgnored {
@@ -363,8 +372,8 @@ void NotifyMacEvent(AXPlatformNodeCocoa* target, ui::AXEvent event_type) {
   // Attributes required for user-editable controls.
   NSArray* const kValueAttributes = @[ NSAccessibilityValueAttribute ];
 
-  // Attributes required for unprotected textfields.
-  NSArray* const kUnprotectedTextfieldAttributes = @[
+  // Attributes required for unprotected textfields and labels.
+  NSArray* const kUnprotectedTextAttributes = @[
     NSAccessibilityInsertionPointLineNumberAttribute,
     NSAccessibilityNumberOfCharactersAttribute,
     NSAccessibilitySelectedTextAttribute,
@@ -372,19 +381,20 @@ void NotifyMacEvent(AXPlatformNodeCocoa* target, ui::AXEvent event_type) {
     NSAccessibilityVisibleCharacterRangeAttribute,
   ];
 
-  // Required for all textfields, including protected ones.
-  NSString* const kTextfieldAttributes =
-      NSAccessibilityPlaceholderValueAttribute;
+  // Required for all text, including protected textfields.
+  NSString* const kTextAttributes = NSAccessibilityPlaceholderValueAttribute;
 
   base::scoped_nsobject<NSMutableArray> axAttributes(
       [[NSMutableArray alloc] init]);
 
   [axAttributes addObjectsFromArray:kAllRoleAttributes];
+
   switch (node_->GetData().role) {
     case ui::AX_ROLE_TEXT_FIELD:
-      [axAttributes addObject:kTextfieldAttributes];
+    case ui::AX_ROLE_STATIC_TEXT:
+      [axAttributes addObject:kTextAttributes];
       if (!node_->GetData().HasState(ui::AX_STATE_PROTECTED))
-        [axAttributes addObjectsFromArray:kUnprotectedTextfieldAttributes];
+        [axAttributes addObjectsFromArray:kUnprotectedTextAttributes];
     // Fallthrough.
     case ui::AX_ROLE_CHECK_BOX:
     case ui::AX_ROLE_COMBO_BOX:
@@ -531,8 +541,14 @@ void NotifyMacEvent(AXPlatformNodeCocoa* target, ui::AXEvent event_type) {
 }
 
 - (id)AXValue {
-  if (node_->GetData().role == ui::AX_ROLE_TAB)
-    return [self AXSelected];
+  switch (node_->GetData().role) {
+    case ui::AX_ROLE_TAB:
+      return [self AXSelected];
+    case ui::AX_ROLE_STATIC_TEXT:
+      return [self AXTitle];
+    default:
+      break;
+  }
   return [self getStringAttribute:ui::AX_ATTR_VALUE];
 }
 
@@ -596,41 +612,36 @@ void NotifyMacEvent(AXPlatformNodeCocoa* target, ui::AXEvent event_type) {
 // Text-specific attributes.
 
 - (NSString*)AXSelectedText {
-  if (node_->GetData().HasState(ui::AX_STATE_PROTECTED))
+  NSString* value = [self getStringValue];
+  if (!value)
     return nil;
 
   NSRange selectedTextRange;
   [[self AXSelectedTextRange] getValue:&selectedTextRange];
-  return [[self AXValue] substringWithRange:selectedTextRange];
+  return [value substringWithRange:selectedTextRange];
 }
 
 - (NSValue*)AXSelectedTextRange {
-  if (node_->GetData().HasState(ui::AX_STATE_PROTECTED))
+  if (![self getStringValue])
     return nil;
 
-  int textDir, start, end;
-  node_->GetIntAttribute(ui::AX_ATTR_TEXT_DIRECTION, &textDir);
+  // Selection might not be supported. Return (NSRange){0,0} in that case.
+  int start = 0, end = 0;
   node_->GetIntAttribute(ui::AX_ATTR_TEXT_SEL_START, &start);
   node_->GetIntAttribute(ui::AX_ATTR_TEXT_SEL_END, &end);
-  // NSRange cannot represent the direction the text was selected in, so make
-  // sure the correct selection index is used when creating a new range, taking
-  // into account the textfield text direction as well.
-  bool isReversed = (textDir == ui::AX_TEXT_DIRECTION_RTL) ||
-                    (textDir == ui::AX_TEXT_DIRECTION_BTT);
-  int beginSelectionIndex = (end > start && !isReversed) ? start : end;
-  return [NSValue valueWithRange:{beginSelectionIndex, abs(end - start)}];
+
+  // NSRange cannot represent the direction the text was selected in.
+  return [NSValue valueWithRange:{std::min(start, end), abs(end - start)}];
 }
 
 - (NSNumber*)AXNumberOfCharacters {
-  if (node_->GetData().HasState(ui::AX_STATE_PROTECTED))
-    return nil;
-  return @([[self AXValue] length]);
+  NSString* value = [self getStringValue];
+  return value ? @([value length]) : nil;
 }
 
 - (NSValue*)AXVisibleCharacterRange {
-  if (node_->GetData().HasState(ui::AX_STATE_PROTECTED))
-    return nil;
-  return [NSValue valueWithRange:{0, [[self AXNumberOfCharacters] intValue]}];
+  NSString* value = [self getStringValue];
+  return value ? [NSValue valueWithRange:{0, [value length]}] : nil;
 }
 
 - (NSNumber*)AXInsertionPointLineNumber {