MacViews a11y: Allow accessibility clients to set the AXValue on some controls.

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 79fa94904ba8d2fbd2fcb4f57b97db3fa7f8ca29..0df2bcf863259d1ec16f7896dc6ff5e253f2475d 100644
--- a/ui/accessibility/platform/ax_platform_node_mac.mm
+++ b/ui/accessibility/platform/ax_platform_node_mac.mm
@@ -349,8 +349,43 @@ void NotifyMacEvent(NSView* target, ui::AXEvent event_type) {
   return axAttributes.autorelease();
 }
 
-- (BOOL)accessibilityIsAttributeSettable:(NSString*)attribute {
-  return NO;
+- (BOOL)accessibilityIsAttributeSettable:(NSString*)attributeName {
+  // Allow certain attributes to be written via an accessibility client. A
+  // writable attribute will only appear as such if the accessibility element
+  // has a value set for that attribute.
+  base::scoped_nsobject<NSMutableArray> axWritableAttributes(

[tapted, 2016/08/11 04:23:25]

I don't think we need this array unless it's used in a follow-up (in which case
it should be moved to a helper method that returns an NSArray*). But it's a bit
weird to build the array only to check if we put something in it.

I think "switching" on the attributeName  with [attributeName
isEqualToString:..] is the nicer approach.. but I'm a bit out of my depth wrt
how "setting" some of these should actually work :/ - see comments below.

[Patti Lor, 2016/08/12 01:57:42]

Done.

+      [[NSMutableArray alloc] init]);
+
+  NSArray* const kWritableAttributes = @[
+    NSAccessibilitySelectedAttribute, NSAccessibilitySelectedChildrenAttribute,
+    NSAccessibilitySelectedTextAttribute,
+    NSAccessibilitySelectedTextRangeAttribute,

[tapted, 2016/08/11 04:23:25]

I think the TextRange should be settable on ReadOnly text fields as well, same
with the VisibileCharacterRange

[Patti Lor, 2016/08/12 01:57:42]

Done.

+    NSAccessibilityVisibleCharacterRangeAttribute
+  ];
+
+  if (!ui::AXViewState::IsFlagSet(node_->GetData().state,
+                                  ui::AX_STATE_READ_ONLY)) {
+    if (node_->GetDelegate()->CanSetStringValue())

[tapted, 2016/08/11 04:23:25]

We already call node_->GetData().state a bunch here - is that where the callback
lives? Can we just check for its presence here, or is there a use-case for
putting it on the delegate, which wouldn't be satisfied that way?

[Patti Lor, 2016/08/12 01:57:42]

The naming is a bit confusing - it's not actually node_->GetData().state that's
called (that is a unsigned int), but actually view_->GetAccessibleState(&state),
which gets us a ui::AXViewState. The latter is where the callback lives.
AXPlatformNodeCocoa shouldn't have access to the View it's representing, so I
have opted for putting a method on the delegate here. I agree it's a bit gross
though - I considered adding a optional parameter |dry_run| to the existing
SetStringValue() method, but that also seemed weird. Do you have a better idea?

[tapted, 2016/08/12 03:50:52]

Ah I see. I guess it would be necessary to update the ui::AXNodeData
NativeViewAccessibility::GetData() so that it copies the callback on to  the
AXNodeData struct - but that could lead to lifetime trouble.

+      [axWritableAttributes addObject:NSAccessibilityValueAttribute];

[tapted, 2016/08/11 04:23:25]

I think NSAccessibilityValueAttribute and NSAccessibilitySelectedTextAttribute
should always be paired together (e.g. if all the text is selected,
NSAccessibilitySelectedTextAttribute is equivalent to
NSAccessibilityValueAttribute so they should be writable under the same set of
conditions.)

[Patti Lor, 2016/08/12 01:57:42]

Done.

+    [axWritableAttributes addObjectsFromArray:kWritableAttributes];
+  }
+
+  if (ui::AXViewState::IsFlagSet(node_->GetData().state,
+                                 ui::AX_STATE_FOCUSABLE))
+    [axWritableAttributes addObject:NSAccessibilityFocusedAttribute];
+
+  BOOL settable = [axWritableAttributes containsObject:attributeName];
+  axWritableAttributes.autorelease();

[tapted, 2016/08/11 04:23:25]

(usually we only do scoped_nsobject::autorelease() to return from a function
(i.e. with the value of the autorelease) - otherwise it's usually fine for the
scoped_nsobject just to go out of scope and do a "proper" release)

[Patti Lor, 2016/08/12 01:57:42]

Done, thanks for the FYI!

+  return settable;
+}
+
+- (void)accessibilitySetValue:(id)value forAttribute:(NSString*)attribute {
+  if ([attribute isEqualToString:NSAccessibilityValueAttribute] &&

[tapted, 2016/08/11 04:23:25]

Does NSAccessibilitySelectedTextAttribute already work the same as NSTextField
when it's plumbed through here as well?

[Patti Lor, 2016/08/12 01:57:42]

Yep, writing NSAccessibilitySelectedTextAttribute on a NSTextfield will clear
the existing selection and replace it with new text. To make it work with
MacViews though I think there needs to be a new method on the delegate that will
figure out which substring to replace and replace the whole text with the old
string + substitution. (Actually, this applies to all the writable attributes -
was going to add callbacks and their own methods to the delegate to make them
work.) Is that what you were asking?

[tapted, 2016/08/12 03:50:52]

sorta - I guess the question is whether the callback invoked in
NativeViewAccessibility::SetStringValue() always replaces the current selection
(if there is one), or replaces the entire text regardless of any current
selection. But either way, there should be different handling - e.g. we could
pass through a `bool replace_selection` or similar on SetStringValue and the
callback.

+      [value isKindOfClass:[NSString class]])
+    node_->GetDelegate()->SetStringValue(base::SysNSStringToUTF16(value));
+
+  // TODO(patricialor): Plumb through all the other writable attributes as
+  // specified in accessibilityIsAttributeSettable.
 }
 
 - (id)accessibilityAttributeValue:(NSString*)attribute {