PR

Author: yaakovschectman

shell/platform/windows/accessibility_alert.cc

@@ -95,10 +95,9 @@ IFACEMETHODIMP AccessibilityAlert::get_accParent(IDispatch** disp_parent) {
   *disp_parent = parent_;
   if (*disp_parent) {
     (*disp_parent)->AddRef();
+    return S_OK;
   }
-  return S_OK;
-  //*disp_parent = nullptr;
-  // return E_NOTIMPL;
+  return S_FALSE;
 }
 
 // Retrieves information describing the role of the specified object.
@@ -127,6 +126,7 @@ IFACEMETHODIMP AccessibilityAlert::get_accValue(VARIANT var_id, BSTR* value) {
   *value = SysAllocString(text_.c_str());
   return S_OK;
 }
+
 IFACEMETHODIMP AccessibilityAlert::put_accValue(VARIANT var_id,
                                                 BSTR new_value) {
   return E_NOTIMPL;
@@ -137,9 +137,11 @@ IFACEMETHODIMP AccessibilityAlert::get_accSelection(VARIANT* selected) {
   selected->vt = VT_EMPTY;
   return E_NOTIMPL;
 }
+
 IFACEMETHODIMP AccessibilityAlert::accSelect(LONG flags_sel, VARIANT var_id) {
   return E_NOTIMPL;
 }
+
 IFACEMETHODIMP AccessibilityAlert::get_accHelpTopic(BSTR* help_file,
                                                     VARIANT var_id,
                                                     LONG* topic_id) {

shell/platform/windows/accessibility_alert.h

@@ -32,19 +32,6 @@ class AccessibilityAlert : public CComObjectRootEx<CComMultiThreadModel>,
                             VARIANT* child) override;
 
   // Performs the object's default action.
-  IFACEMETHODIMP accDoDefaultAction(VARIANT var_id) override;
-
-  // Retrieves the specified object's current screen location.
-  IFACEMETHODIMP accLocation(LONG* physical_pixel_left,
-                             LONG* physical_pixel_top,
-                             LONG* width,
-                             LONG* height,
-                             VARIANT var_id) override;
-
-  // Traverses to another UI element and retrieves the object.
-  IFACEMETHODIMP accNavigate(LONG nav_dir,
-                             VARIANT start,
-                             VARIANT* end) override;
 
   // Retrieves an IDispatch interface pointer for the specified child.
   IFACEMETHODIMP get_accChild(VARIANT var_child,
@@ -61,11 +48,6 @@ class AccessibilityAlert : public CComObjectRootEx<CComMultiThreadModel>,
   IFACEMETHODIMP get_accDescription(VARIANT var_id, BSTR* desc) override;
 
   // Retrieves the object that has the keyboard focus.
-  IFACEMETHODIMP get_accFocus(VARIANT* focus_child) override;
-
-  // Retrieves the specified object's shortcut.
-  IFACEMETHODIMP get_accKeyboardShortcut(VARIANT var_id,
-                                         BSTR* access_key) override;
 
   // Retrieves the name of the specified object.
   IFACEMETHODIMP get_accName(VARIANT var_id, BSTR* name) override;
@@ -86,15 +68,27 @@ class AccessibilityAlert : public CComObjectRootEx<CComMultiThreadModel>,
   // Setting the value is not typically used by screen readers, but it's
   // used frequently by automation software.
   IFACEMETHODIMP get_accValue(VARIANT var_id, BSTR* value) override;
-  IFACEMETHODIMP put_accValue(VARIANT var_id, BSTR new_value) override;
 
   // IAccessible methods not implemented.
+  IFACEMETHODIMP accLocation(LONG* physical_pixel_left,
+                             LONG* physical_pixel_top,
+                             LONG* width,
+                             LONG* height,
+                             VARIANT var_id) override;
+  IFACEMETHODIMP accNavigate(LONG nav_dir,
+                             VARIANT start,
+                             VARIANT* end) override;
+  IFACEMETHODIMP accDoDefaultAction(VARIANT var_id) override;
+  IFACEMETHODIMP get_accFocus(VARIANT* focus_child) override;
+  IFACEMETHODIMP get_accKeyboardShortcut(VARIANT var_id,
+                                         BSTR* access_key) override;
   IFACEMETHODIMP get_accSelection(VARIANT* selected) override;
   IFACEMETHODIMP accSelect(LONG flags_sel, VARIANT var_id) override;
   IFACEMETHODIMP get_accHelpTopic(BSTR* help_file,
                                   VARIANT var_id,
                                   LONG* topic_id) override;
   IFACEMETHODIMP put_accName(VARIANT var_id, BSTR put_name) override;
+  IFACEMETHODIMP put_accValue(VARIANT var_id, BSTR new_value) override;
 
   AccessibilityAlert();
   ~AccessibilityAlert() = default;

shell/platform/windows/accessibility_root_node.cc

@@ -24,7 +24,7 @@ IAccessible* AccessibilityRootNode::GetTargetAndChildID(VARIANT* var_id) {
   LONG& child_id = var_id->lVal;
   if (V_VT(var_id) != VT_I4) {
     child_id = kInvalidChildId;
-    return FALSE;
+    return nullptr;
   }
   child_id = V_I4(var_id);
   if (!window_accessible_) {
@@ -38,6 +38,7 @@ IAccessible* AccessibilityRootNode::GetTargetAndChildID(VARIANT* var_id) {
     child_id = CHILDID_SELF;
     return alert_accessible_;
   }
+  // A negative child ID can be used to refer to an AX node directly by its ID.
   if (child_id < 0) {
     return window_accessible_;
   }

shell/platform/windows/window.cc

@@ -666,7 +666,7 @@ void Window::CreateAccessibilityRootNode() {
   ui::win::CreateATLModuleIfNeeded();
   CComObject<AccessibilityRootNode>* instance = nullptr;
   HRESULT hr = CComObject<AccessibilityRootNode>::CreateInstance(&instance);
-  if (!SUCCEEDED(hr)) {
+  if (!SUCCEEDED(hr) || !instance) {
     FML_LOG(FATAL) << "Failed to create accessibility root node";
   }
   instance->AddRef();