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

ui/accessibility/platform/ax_platform_node_mac.mm

 // Copyright 2014 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.
 
 #import "ui/accessibility/platform/ax_platform_node_mac.h"
 
 #import <Cocoa/Cocoa.h>
 #include <stddef.h>
 
 #include "base/macros.h"
@@ skipping 212 matching lines @@
 void NotifyMacEvent(AXPlatformNodeCocoa* target, ui::AXEvent event_type) {
   NSAccessibilityPostNotification(
       target, [AXPlatformNodeCocoa nativeNotificationFromAXEvent:event_type]);
 }
 
 }  // namespace
 
 @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 {
   ui::AXPlatformNodeBase* node_;  // Weak. Retains us.
 }
 
 @synthesize node = node_;
 
 // A mapping of AX roles to native roles.
 + (NSString*)nativeRoleFromAXRole:(ui::AXRole)role {
@@ skipping 37 matching lines @@
   return gfx::ScreenRectToNSRect(node_->GetBoundsInScreen());
 }
 
 - (NSString*)getStringAttribute:(ui::AXStringAttribute)attribute {
   std::string attributeValue;
   if (node_->GetStringAttribute(attribute, &attributeValue))
     return base::SysUTF8ToNSString(attributeValue);
   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 {
   return [[self AXRole] isEqualToString:NSAccessibilityUnknownRole] ||
          node_->GetData().HasState(ui::AX_STATE_INVISIBLE);
 }
 
 - (id)accessibilityHitTest:(NSPoint)point {
   for (AXPlatformNodeCocoa* child in [self AXChildren]) {
     if (![child accessibilityIsIgnored] &&
@@ skipping 56 matching lines @@
     NSAccessibilityEnabledAttribute,
     NSAccessibilityFocusedAttribute,
     NSAccessibilityHelpAttribute,
     NSAccessibilityTopLevelUIElementAttribute,
     NSAccessibilityWindowAttribute,
   ];
 
   // 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,
     NSAccessibilitySelectedTextRangeAttribute,
     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:
     case ui::AX_ROLE_MENU_ITEM_CHECK_BOX:
     case ui::AX_ROLE_MENU_ITEM_RADIO:
     case ui::AX_ROLE_RADIO_BUTTON:
     case ui::AX_ROLE_SEARCH_BOX:
     case ui::AX_ROLE_SLIDER:
     case ui::AX_ROLE_SLIDER_THUMB:
     case ui::AX_ROLE_TOGGLE_BUTTON:
@@ skipping 126 matching lines @@
       break;
   }
   return [AXPlatformNodeCocoa nativeSubroleFromAXRole:role];
 }
 
 - (NSString*)AXHelp {
   return [self getStringAttribute:ui::AX_ATTR_DESCRIPTION];
 }
 
 - (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];
 }
 
 - (NSNumber*)AXEnabled {
   return @(!node_->GetData().HasState(ui::AX_STATE_DISABLED));
 }
 
 - (NSNumber*)AXFocused {
   if (node_->GetData().HasState(ui::AX_STATE_FOCUSABLE))
     return
@@ skipping 43 matching lines @@
   return @(node_->GetData().HasState(ui::AX_STATE_SELECTED));
 }
 
 - (NSString*)AXPlaceholderValue {
   return [self getStringAttribute:ui::AX_ATTR_PLACEHOLDER];
 }
 
 // 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 {
   if (node_->GetData().HasState(ui::AX_STATE_PROTECTED))
     return nil;
   // Multiline is not supported on views.
   return @0;
 }
 
 @end
@@ skipping 50 matching lines @@
   }
   NotifyMacEvent(native_node_, event_type);
 }
 
 int AXPlatformNodeMac::GetIndexInParent() {
   // TODO(dmazzoni): implement this.  http://crbug.com/396137
   return -1;
 }
 
 }  // namespace ui