MacViews a11y: Sync VoiceOver cursor with keyboard focus.

content/browser/accessibility/browser_accessibility_cocoa.mm

 // Copyright (c) 2012 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 <execinfo.h>
 #include <stddef.h>
 #include <stdint.h>
 
 #import "content/browser/accessibility/browser_accessibility_cocoa.h"
 
 #include <map>
 
 #include "base/mac/foundation_util.h"
 #include "base/mac/scoped_cftyperef.h"
+#import "base/mac/sdk_forward_declarations.h"

[tapted, 2016/09/28 01:03:19]

nit: remove?

[Patti Lor, 2016/10/04 05:45:14]

Done.

 #include "base/strings/string16.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/app/strings/grit/content_strings.h"
 #include "content/browser/accessibility/browser_accessibility_mac.h"
 #include "content/browser/accessibility/browser_accessibility_manager.h"
 #include "content/browser/accessibility/browser_accessibility_manager_mac.h"
 #include "content/browser/accessibility/one_shot_accessibility_tree_search.h"
 #include "content/public/common/content_client.h"
 #include "third_party/skia/include/core/SkColor.h"
@@ skipping 74 matching lines @@
   int32_t node_id;
   int offset;
   ui::AXTextAffinity affinity;
 };
 
 // VoiceOver uses -1 to mean "no limit" for AXResultsLimit.
 const int kAXResultsLimitNoLimit = -1;
 
 extern "C" {
 
-// See http://openradar.appspot.com/9896491. This SPI has been tested on 10.5,
-// 10.6, and 10.7. It allows accessibility clients to observe events posted on
-// this object.
-void NSAccessibilityUnregisterUniqueIdForUIElement(id element);
-
 // The following are private accessibility APIs required for cursor navigation
 // and text selection. VoiceOver started relying on them in Mac OS X 10.11.
 #if !defined(MAC_OS_X_VERSION_10_11) || \
     MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_11
 
 AXTextMarkerRef AXTextMarkerCreate(CFAllocatorRef allocator,
                                    const UInt8* bytes,
                                    CFIndex length);
 
 const UInt8* AXTextMarkerGetBytePtr(AXTextMarkerRef text_marker);
@@ skipping 479 matching lines @@
   dict = nil;
 }
 
 - (instancetype)initWithObject:(BrowserAccessibility*)accessibility {
   if ((self = [super init]))
     browserAccessibility_ = accessibility;
   return self;
 }
 
 - (void)detach {
-  if (browserAccessibility_)
-    NSAccessibilityUnregisterUniqueIdForUIElement(self);
+  if (!browserAccessibility_)
+    return;
+  NSAccessibilityPostNotification(
+      self, NSAccessibilityUIElementDestroyedNotification);
   browserAccessibility_ = nullptr;
 }
 
 - (NSString*)accessKey {
   if (![self instanceActive])
     return nil;
   return NSStringForStringAttribute(
       browserAccessibility_, ui::AX_ATTR_ACCESS_KEY);
 }
 
@@ skipping 2248 matching lines @@
   return ([self hash] == [object hash]);
 }
 
 - (NSUInteger)hash {
   // Potentially called during dealloc.
   if (![self instanceActive])
     return [super hash];
   return browserAccessibility_->GetId();
 }
 
-- (BOOL)accessibilityShouldUseUniqueId {
+- (BOOL)accessibilityNotifiesWhenDestroyed {
+  // Indicate that BrowserAccessibilityCocoa will post a notification when it's
+  // destroyed (see - detach). This allows VoiceOver to do some internal
+  // things more efficiently.

[tapted, 2016/09/28 01:03:19]

Yeah it would be awesome if we can drop the private API use! But I'm a little
wary of possible regressions.. accessibilityShouldUseUniqueId wasn't exposed for
efficiency but because of tricky lifetime problems, and issues when this was
changed from inheriting from an NSView to inheriting from just an NSObject.
We'll need to go back to original bug to make sure nothing has regressed.

[Patti Lor, 2016/10/04 05:45:14]

The decision to delete the private API stuff here was mostly done because of the
information on the Apple radar filed (http://openradar.appspot.com/9896491),
which said that VO wouldn't acknowledge not-NSView/NSWindow things as
accessibility elements. It's working now, so I thought that probably isn't
needed anymore.

If the lifetime bug you are talking about is this: "This will hopefully set us
up to solve the problem of VoiceOver not acknowledging page transitions.  The
remaining issue is that RWHV's get destroyed on page transitions, thereby
destroying our BrowserAccessibilityManager.  This is likely why VoiceOver won't
honor our AXLoadComplete notification." (stolen from the CL description
https://chromium.googlesource.com/chromium/src/+/cbd4ff785885f593c2ce45346ca9...,
also see https://www.crbug.com/55658), then I think this is fine as well. VO
re-reads the name of the HTML page when I navigate to another page with this CL
patched in, and makes bloopy noises when I click/press enter on a link and again
when the link loads. Adding log messages to ~BrowserAccessibilityManagerMac
doesn't seem to trigger per page transition, either.

   return YES;
 }
 
 @end