[PATCH 1/2] oleacc: Implement AccessibleObjectFromEvent.

[Connor McAdams]

Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com>
---
 dlls/oleacc/main.c      | 39 +++++++++++++++++++++++++++++++++++++++
 dlls/oleacc/oleacc.spec |  2 +-
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/dlls/oleacc/main.c b/dlls/oleacc/main.c
index f6b66a8bcab..fd85518436c 100644
--- a/dlls/oleacc/main.c
+++ b/dlls/oleacc/main.c
@@ -331,6 +331,45 @@ HRESULT WINAPI AccessibleObjectFromPoint( POINT ptScreen, IAccessible** ppacc, V
     return E_NOTIMPL;
 }
 
+HRESULT WINAPI AccessibleObjectFromEvent( HWND hwnd, DWORD dwObjectID, DWORD dwChildID,
+                             IAccessible** ppacc, VARIANT* pvarChild )
+{
+    IDispatch *child;
+    VARIANT cid;
+    HRESULT hr;
+
+    TRACE("%p %d %d %p %p\n", hwnd, dwObjectID, dwChildID, ppacc, pvarChild);
+
+    hr = AccessibleObjectFromWindow(hwnd, dwObjectID, &IID_IAccessible, (void **)ppacc);
+    if (FAILED(hr))
+        return hr;
+
+    V_VT(&cid) = VT_I4;
+    V_I4(&cid) = dwChildID;
+    hr = IAccessible_get_accChild(*ppacc, cid, &child);
+    if (FAILED(hr))
+        FIXME("get_accChild failed with %#x!\n", hr);
+
+    V_VT(pvarChild) = VT_I4;
+    if (child)
+    {
+        IAccessible *acc;
+
+        if (SUCCEEDED(IDispatch_QueryInterface(child, &IID_IAccessible, (void **)&acc)))
+        {
+            IAccessible_Release(*ppacc);
+            *ppacc = acc;
+        }
+
+        IDispatch_Release(child);
+        V_I4(pvarChild) = CHILDID_SELF;
+    }
+    else
+        V_I4(pvarChild) = dwChildID;
+
+    return S_OK;
+}
+
 HRESULT WINAPI AccessibleObjectFromWindow( HWND hwnd, DWORD dwObjectID,
                              REFIID riid, void** ppvObject )
 {
diff --git a/dlls/oleacc/oleacc.spec b/dlls/oleacc/oleacc.spec
index 4cff0b8effc..9ad6e915965 100644
--- a/dlls/oleacc/oleacc.spec
+++ b/dlls/oleacc/oleacc.spec
@@ -1,5 +1,5 @@
 @ stdcall AccessibleChildren(ptr long long ptr ptr)
-@ stub AccessibleObjectFromEvent
+@ stdcall AccessibleObjectFromEvent(ptr long long ptr ptr)
 @ stdcall AccessibleObjectFromPoint(int64 ptr ptr)
 @ stdcall AccessibleObjectFromWindow(ptr long ptr ptr)
 @ stdcall CreateStdAccessibleObject(ptr long ptr ptr)
-- 
2.25.1

[PATCH 2/2] oleacc: Add tests for AccessibleObjectFromEvent.

[Connor McAdams]

Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com>
---
 dlls/oleacc/tests/main.c | 970 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 970 insertions(+)

diff --git a/dlls/oleacc/tests/main.c b/dlls/oleacc/tests/main.c
index ce44e06abe4..5823b4336ba 100644
--- a/dlls/oleacc/tests/main.c
+++ b/dlls/oleacc/tests/main.c
@@ -304,6 +304,756 @@ static IAccessibleVtbl AccessibleVtbl = {
 
 static IAccessible Accessible = {&AccessibleVtbl};
 
+/*
+ * Create an accessibility tree that looks like this:
+ * --ACC_TREE_OBJ_ID_ROOT (Full object)
+ * ----ACC_TREE_OBJ_ID_CHILD_0 (Full object)
+ * ----ACC_TREE_OBJ_ID_CHILD_1 (Full object)
+ * ------ACC_TREE_OBJ_ID_CHILD_1_0 (Simple element)
+ * ------ACC_TREE_OBJ_ID_CHILD_1_1 (Full object)
+ * ----ACC_TREE_OBJ_ID_CHILD_2 (Simple element)
+ */
+enum {
+    ACC_TREE_OBJ_ID_NONE = -1,
+    ACC_TREE_OBJ_ID_ROOT = 1,
+    ACC_TREE_OBJ_ID_CHILD_0,
+    ACC_TREE_OBJ_ID_CHILD_1,
+    ACC_TREE_OBJ_ID_CHILD_1_0,
+    ACC_TREE_OBJ_ID_CHILD_1_1,
+    ACC_TREE_OBJ_ID_CHILD_2,
+};
+
+typedef struct {
+    const WCHAR *name;
+
+    INT parent_id, child_id, child_pos;
+    BOOL is_simple_elem;
+    UINT child_count;
+} acc_tree_object_info;
+
+typedef struct {
+    IAccessible IAccessible_iface;
+    IEnumVARIANT IEnumVARIANT_iface;
+
+    LONG ref;
+
+    const acc_tree_object_info *info;
+
+    HWND hwnd;
+    UINT enum_pos;
+
+    IAccessible *parent;
+    IAccessible **children;
+} AccTreeObject;
+
+AccTreeObject *object_tree = NULL;
+
+const acc_tree_object_info acc_from_event_obj_tree[] = {
+    { .name = L"acc_tree_root",
+      .parent_id = ACC_TREE_OBJ_ID_NONE,
+      .child_id = ACC_TREE_OBJ_ID_ROOT,
+      .child_pos = 0,
+      .child_count = 3,
+      .is_simple_elem = FALSE,
+    },
+    { .name = L"acc_tree_child_0",
+      .parent_id = ACC_TREE_OBJ_ID_ROOT,
+      .child_id = ACC_TREE_OBJ_ID_CHILD_0,
+      .child_pos = 0,
+      .child_count = 0,
+      .is_simple_elem = FALSE,
+    },
+    { .name = L"acc_tree_child_1",
+      .parent_id = ACC_TREE_OBJ_ID_ROOT,
+      .child_id = ACC_TREE_OBJ_ID_CHILD_1,
+      .child_pos = 1,
+      .child_count = 2,
+      .is_simple_elem = FALSE,
+    },
+    { .name = L"acc_tree_child_1_0",
+      .parent_id = ACC_TREE_OBJ_ID_CHILD_1,
+      .child_id = ACC_TREE_OBJ_ID_CHILD_1_0,
+      .child_pos = 0,
+      .child_count = 0,
+      .is_simple_elem = TRUE,
+    },
+    { .name = L"acc_tree_child_1_1",
+      .parent_id = ACC_TREE_OBJ_ID_CHILD_1,
+      .child_id = ACC_TREE_OBJ_ID_CHILD_1_1,
+      .child_pos = 1,
+      .child_count = 0,
+      .is_simple_elem = FALSE,
+    },
+    { .name = L"acc_tree_child_2",
+      .parent_id = ACC_TREE_OBJ_ID_ROOT,
+      .child_id = ACC_TREE_OBJ_ID_CHILD_2,
+      .child_pos = 2,
+      .child_count = 0,
+      .is_simple_elem = TRUE,
+    },
+};
+
+/*
+ * AccTreeObj IAccessible Vtbl.
+ */
+static inline AccTreeObject* impl_from_AccTreeObject(IAccessible *iface)
+{
+    return CONTAINING_RECORD(iface, AccTreeObject, IAccessible_iface);
+}
+
+static IAccessible *get_acc_tree_obj_child(IAccessible *iface, VARIANT child_id)
+{
+    AccTreeObject *obj = impl_from_AccTreeObject(iface);
+    IAccessible *acc = NULL;
+    LONG i;
+
+    if (V_VT(&child_id) != VT_I4)
+        return NULL;
+
+    if (V_I4(&child_id) == CHILDID_SELF)
+        return iface;
+
+    for (i = 0; i < obj->info->child_count; i++)
+    {
+        AccTreeObject *child = impl_from_AccTreeObject(obj->children[i]);
+
+        if (child->info->child_id == V_I4(&child_id))
+        {
+            acc = &child->IAccessible_iface;
+            break;
+        }
+    }
+
+    return acc;
+}
+
+static void check_acc_tree_obj_for_child(IAccessible *acc,
+        INT child_id, IAccessible **found_acc, VARIANT *found_vid)
+{
+    IDispatch *disp_child;
+    VARIANT vid;
+    HRESULT hr;
+
+    V_VT(&vid) = VT_I4;
+    V_I4(&vid) = child_id;
+    hr = IAccessible_get_accChild(acc, vid, &disp_child);
+    if (SUCCEEDED(hr))
+    {
+        /*
+         * If S_FALSE is returned, the childID was found, but it's a simple
+         * element.
+         */
+        if (hr == S_FALSE)
+        {
+            *found_acc = acc;
+            V_VT(found_vid) = VT_I4;
+            V_I4(found_vid) = child_id;
+        }
+        else
+        {
+            IDispatch_QueryInterface(disp_child, &IID_IAccessible,
+                    (void **)found_acc);
+            V_VT(found_vid) = VT_I4;
+            V_I4(found_vid) = CHILDID_SELF;
+            IDispatch_Release(disp_child);
+        }
+    }
+}
+
+static void search_acc_tree_for_child_enumVARIANT(IAccessible *acc,
+        INT child_id, IAccessible **found_acc, VARIANT *found_vid)
+{
+    LONG child_cnt, i, rcv;
+    VARIANT *children;
+    HRESULT hr;
+
+    IAccessible_get_accChildCount(acc, &child_cnt);
+    if (!child_cnt)
+        return;
+
+    check_acc_tree_obj_for_child(acc, child_id, found_acc, found_vid);
+    if (*found_acc)
+        return;
+
+    children = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
+            sizeof(*children) * child_cnt);
+    hr = AccessibleChildren(acc, 0, child_cnt, children, &rcv);
+    ok(SUCCEEDED(hr), "AccessibleChildren failed with %#x!\n", hr);
+
+    /*
+     * We only care about VT_DISPATCH VARIANT's here, because simple elements
+     * cannot have children, and if they were a match it would've been found
+     * in check_acc_tree_obj_for_child().
+     */
+    for (i = 0; i < child_cnt; i++)
+    {
+        VARIANT *res = &children[i];
+        IAccessible *acc_child;
+
+        if (V_VT(res) != VT_DISPATCH)
+            continue;
+
+        if (!(*found_acc))
+        {
+            hr = IDispatch_QueryInterface(V_DISPATCH(res), &IID_IAccessible, (void **)&acc_child);
+            if (SUCCEEDED(hr))
+                search_acc_tree_for_child_enumVARIANT(acc_child, child_id, found_acc, found_vid);
+
+            if (*found_acc != acc_child)
+                IAccessible_Release(acc_child);
+        }
+
+        IDispatch_Release(V_DISPATCH(res));
+    }
+
+    HeapFree(GetProcessHeap(), 0, (void *)children);
+}
+
+static void search_acc_tree_for_child_navigate(IAccessible *acc,
+        INT child_id, IAccessible **found_acc, VARIANT *found_vid)
+{
+    VARIANT vid, res;
+    HRESULT hr;
+
+    check_acc_tree_obj_for_child(acc, child_id, found_acc, found_vid);
+    if (*found_acc)
+        return;
+
+    V_VT(&vid) = VT_I4;
+    V_I4(&vid) = CHILDID_SELF;
+    hr = IAccessible_accNavigate(acc, NAVDIR_FIRSTCHILD, vid, &res);
+    if (FAILED(hr))
+        return;
+
+    while (SUCCEEDED(hr) && V_VT(&res) != VT_EMPTY && !(*found_acc))
+    {
+        switch (V_VT(&res))
+        {
+        case VT_I4:
+            vid = res;
+            hr = IAccessible_accNavigate(acc, NAVDIR_NEXT, vid, &res);
+            break;
+
+        case VT_DISPATCH:
+        {
+            IAccessible *acc_child;
+
+            hr = IDispatch_QueryInterface(V_DISPATCH(&res), &IID_IAccessible, (void **)&acc_child);
+            IDispatch_Release(V_DISPATCH(&res));
+
+            if (SUCCEEDED(hr))
+            {
+                search_acc_tree_for_child_navigate(acc_child, child_id, found_acc, found_vid);
+
+                if (!(*found_acc))
+                {
+                    V_VT(&vid) = VT_I4;
+                    V_I4(&vid) = CHILDID_SELF;
+                    hr = IAccessible_accNavigate(acc_child, NAVDIR_NEXT, vid, &res);
+                }
+
+                if (*found_acc != acc_child)
+                    IAccessible_Release(acc_child);
+            }
+            break;
+        }
+
+        /*
+         * Shouldn't ever reach here, if type isn't VT_I4 or VT_DISPATCH,
+         * we've got a problem.
+         */
+        default:
+            return;
+        }
+    }
+}
+
+HRESULT WINAPI AccTreeObject_QueryInterface(IAccessible *iface, REFIID riid, void **ppv)
+{
+    AccTreeObject *This = impl_from_AccTreeObject(iface);
+
+    if(IsEqualIID(riid, &IID_IAccessible) ||
+            IsEqualIID(riid, &IID_IDispatch) ||
+            IsEqualIID(riid, &IID_IUnknown)) {
+        *ppv = iface;
+    }else if(IsEqualIID(riid, &IID_IEnumVARIANT)) {
+        *ppv = &This->IEnumVARIANT_iface;
+    }else {
+        *ppv = NULL;
+        return E_NOINTERFACE;
+    }
+
+    IAccessible_AddRef(iface);
+    return S_OK;
+}
+
+ULONG WINAPI AccTreeObject_AddRef(IAccessible *iface)
+{
+    AccTreeObject *This = impl_from_AccTreeObject(iface);
+    return InterlockedIncrement(&This->ref);
+}
+
+ULONG WINAPI AccTreeObject_Release(IAccessible *iface)
+{
+    AccTreeObject *This = impl_from_AccTreeObject(iface);
+    return InterlockedDecrement(&This->ref);
+}
+
+HRESULT WINAPI AccTreeObject_GetTypeInfoCount(
+        IAccessible *iface, UINT *pctinfo)
+{
+    *pctinfo = 0;
+
+    return S_OK;
+}
+
+HRESULT WINAPI AccTreeObject_GetTypeInfo(IAccessible *iface,
+        UINT iTInfo, LCID lcid, ITypeInfo **ppTInfo)
+{
+    *ppTInfo = NULL;
+
+    return S_OK;
+}
+
+HRESULT WINAPI AccTreeObject_GetIDsOfNames(IAccessible *iface, REFIID riid,
+        LPOLESTR *rgszNames, UINT cNames, LCID lcid, DISPID *rgDispId)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_Invoke(IAccessible *iface, DISPID dispIdMember,
+        REFIID riid, LCID lcid, WORD wFlags, DISPPARAMS *pDispParams,
+        VARIANT *pVarResult, EXCEPINFO *pExcepInfo, UINT *puArgErr)
+{
+    return S_OK;
+}
+
+HRESULT WINAPI AccTreeObject_get_accParent(
+        IAccessible *iface, IDispatch **ppdispParent)
+{
+    AccTreeObject *This = impl_from_AccTreeObject(iface);
+
+    *ppdispParent = NULL;
+    if (This->parent)
+    {
+        IAccessible_QueryInterface(This->parent, &IID_IDispatch, (void **)ppdispParent);
+	return S_OK;
+    }
+
+    return S_FALSE;
+}
+
+HRESULT WINAPI AccTreeObject_get_accChildCount(
+        IAccessible *iface, LONG *pcountChildren)
+{
+    AccTreeObject *This = impl_from_AccTreeObject(iface);
+
+    *pcountChildren = This->info->child_count;
+
+    return S_OK;
+}
+
+HRESULT WINAPI AccTreeObject_get_accChild(IAccessible *iface,
+        VARIANT varChildID, IDispatch **ppdispChild)
+{
+    IAccessible *child;
+    AccTreeObject *obj;
+
+    *ppdispChild = NULL;
+    if (!(child = get_acc_tree_obj_child(iface, varChildID)))
+        return E_INVALIDARG;
+
+    obj = impl_from_AccTreeObject(child);
+    if (obj->info->is_simple_elem)
+        return S_FALSE;
+
+    IAccessible_QueryInterface(child, &IID_IDispatch, (void **)ppdispChild);
+    return S_OK;
+}
+
+HRESULT WINAPI AccTreeObject_get_accName(IAccessible *iface,
+        VARIANT varID, BSTR *pszName)
+{
+    IAccessible *child;
+    AccTreeObject *obj;
+
+    *pszName = NULL;
+    if (!(child = get_acc_tree_obj_child(iface, varID)))
+        return E_INVALIDARG;
+
+    obj = impl_from_AccTreeObject(child);
+    *pszName = SysAllocString(obj->info->name);
+
+    return S_OK;
+}
+
+HRESULT WINAPI AccTreeObject_get_accValue(IAccessible *iface,
+        VARIANT varID, BSTR *pszValue)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_get_accDescription(IAccessible *iface,
+        VARIANT varID, BSTR *pszDescription)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_get_accRole(IAccessible *iface,
+        VARIANT varID, VARIANT *pvarRole)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_get_accState(IAccessible *iface,
+        VARIANT varID, VARIANT *pvarState)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_get_accHelp(IAccessible *iface,
+        VARIANT varID, BSTR *pszHelp)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_get_accHelpTopic(IAccessible *iface,
+        BSTR *pszHelpFile, VARIANT varID, LONG *pidTopic)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_get_accKeyboardShortcut(IAccessible *iface,
+        VARIANT varID, BSTR *pszKeyboardShortcut)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_get_accFocus(IAccessible *iface, VARIANT *pvarID)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_get_accSelection(
+        IAccessible *iface, VARIANT *pvarID)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_get_accDefaultAction(IAccessible *iface,
+        VARIANT varID, BSTR *pszDefaultAction)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_accSelect(IAccessible *iface,
+        LONG flagsSelect, VARIANT varID)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_accLocation(IAccessible *iface, LONG *pxLeft,
+        LONG *pyTop, LONG *pcxWidth, LONG *pcyHeight, VARIANT varID)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_accNavigate(IAccessible *iface,
+        LONG navDir, VARIANT varStart, VARIANT *pvarEnd)
+{
+    AccTreeObject *This = impl_from_AccTreeObject(iface);
+    AccTreeObject *obj_start, *child;
+    IAccessible *acc_start;
+    HRESULT hr = S_FALSE;
+
+    V_VT(pvarEnd) = VT_EMPTY;
+    if (!(acc_start = get_acc_tree_obj_child(iface, varStart)))
+        return E_INVALIDARG;
+
+    child = NULL;
+    obj_start = impl_from_AccTreeObject(acc_start);
+    switch (navDir)
+    {
+    case NAVDIR_FIRSTCHILD:
+    case NAVDIR_LASTCHILD:
+    {
+        UINT child_cnt = obj_start->info->child_count;
+
+        /* Cannot start with a child ID offset when getting first/last children. */
+        if ((V_I4(&varStart) == CHILDID_SELF) && child_cnt)
+        {
+            if (navDir == NAVDIR_FIRSTCHILD)
+                child = impl_from_AccTreeObject(obj_start->children[0]);
+            else
+                child = impl_from_AccTreeObject(obj_start->children[child_cnt - 1]);
+        }
+        break;
+    }
+
+    case NAVDIR_NEXT:
+    case NAVDIR_PREVIOUS:
+    {
+        AccTreeObject *parent;
+        INT next_pos;
+
+        if (!obj_start->parent)
+            break;
+
+        if (V_I4(&varStart) == CHILDID_SELF)
+            parent = impl_from_AccTreeObject(obj_start->parent);
+        else
+            parent = This;
+
+        if (navDir == NAVDIR_PREVIOUS)
+            next_pos = obj_start->info->child_pos - 1;
+        else
+            next_pos = obj_start->info->child_pos + 1;
+
+        if ((next_pos < 0) || (parent->info->child_count <= next_pos))
+            break;
+
+        child = impl_from_AccTreeObject(parent->children[next_pos]);
+        break;
+    }
+
+    default:
+        break;
+    }
+
+    if (child)
+    {
+        if (child->info->is_simple_elem)
+        {
+            V_VT(pvarEnd) = VT_I4;
+            V_I4(pvarEnd) = child->info->child_id;
+        }
+        else
+        {
+            V_VT(pvarEnd) = VT_DISPATCH;
+            IAccessible_QueryInterface(&child->IAccessible_iface,
+                    &IID_IDispatch, (void **)&V_DISPATCH(pvarEnd));
+        }
+
+        hr = S_OK;
+    }
+
+    return hr;
+}
+
+HRESULT WINAPI AccTreeObject_accHitTest(IAccessible *iface,
+        LONG xLeft, LONG yTop, VARIANT *pvarID)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_accDoDefaultAction(
+        IAccessible *iface, VARIANT varID)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_put_accName(IAccessible *iface,
+        VARIANT varID, BSTR pszName)
+{
+    return E_NOTIMPL;
+}
+
+HRESULT WINAPI AccTreeObject_put_accValue(IAccessible *iface,
+        VARIANT varID, BSTR pszValue)
+{
+    return E_NOTIMPL;
+}
+
+IAccessibleVtbl AccTreeObjAccessibleVtbl = {
+    AccTreeObject_QueryInterface,
+    AccTreeObject_AddRef,
+    AccTreeObject_Release,
+    AccTreeObject_GetTypeInfoCount,
+    AccTreeObject_GetTypeInfo,
+    AccTreeObject_GetIDsOfNames,
+    AccTreeObject_Invoke,
+    AccTreeObject_get_accParent,
+    AccTreeObject_get_accChildCount,
+    AccTreeObject_get_accChild,
+    AccTreeObject_get_accName,
+    AccTreeObject_get_accValue,
+    AccTreeObject_get_accDescription,
+    AccTreeObject_get_accRole,
+    AccTreeObject_get_accState,
+    AccTreeObject_get_accHelp,
+    AccTreeObject_get_accHelpTopic,
+    AccTreeObject_get_accKeyboardShortcut,
+    AccTreeObject_get_accFocus,
+    AccTreeObject_get_accSelection,
+    AccTreeObject_get_accDefaultAction,
+    AccTreeObject_accSelect,
+    AccTreeObject_accLocation,
+    AccTreeObject_accNavigate,
+    AccTreeObject_accHitTest,
+    AccTreeObject_accDoDefaultAction,
+    AccTreeObject_put_accName,
+    AccTreeObject_put_accValue
+};
+
+/*
+ * AccTreeObj enumVARIANT Vtbl.
+ */
+inline AccTreeObject* impl_from_AccTreeObject_EnumVARIANT(IEnumVARIANT *iface)
+{
+    return CONTAINING_RECORD(iface, AccTreeObject, IEnumVARIANT_iface);
+}
+
+HRESULT WINAPI AccTreeObject_EnumVARIANT_QueryInterface(IEnumVARIANT *iface, REFIID riid, void **ppv)
+{
+    AccTreeObject *This = impl_from_AccTreeObject_EnumVARIANT(iface);
+    return IAccessible_QueryInterface(&This->IAccessible_iface, riid, ppv);
+}
+
+ULONG WINAPI AccTreeObject_EnumVARIANT_AddRef(IEnumVARIANT *iface)
+{
+    AccTreeObject *This = impl_from_AccTreeObject_EnumVARIANT(iface);
+    return IAccessible_AddRef(&This->IAccessible_iface);
+}
+
+ULONG WINAPI AccTreeObject_EnumVARIANT_Release(IEnumVARIANT *iface)
+{
+    AccTreeObject *This = impl_from_AccTreeObject_EnumVARIANT(iface);
+    return IAccessible_Release(&This->IAccessible_iface);
+}
+
+HRESULT WINAPI AccTreeObject_EnumVARIANT_Next(IEnumVARIANT *iface,
+        ULONG celt, VARIANT *rgVar, ULONG *pCeltFetched)
+{
+    AccTreeObject *This = impl_from_AccTreeObject_EnumVARIANT(iface);
+    ULONG fetched = 0;
+    UINT i;
+
+    for (i = This->enum_pos; i < This->info->child_count; i++)
+    {
+        AccTreeObject *child = impl_from_AccTreeObject(This->children[i]);
+
+        if (child->info->is_simple_elem)
+        {
+            V_VT(&rgVar[i]) = VT_I4;
+            V_I4(&rgVar[i]) = child->info->child_id;
+        }
+        else
+        {
+            V_VT(&rgVar[i]) = VT_DISPATCH;
+            IAccessible_QueryInterface(&child->IAccessible_iface, &IID_IDispatch,
+                    (void **)&V_DISPATCH(&rgVar[i]));
+        }
+
+	fetched++;
+    }
+
+    *pCeltFetched = fetched;
+
+    return celt == fetched ? S_OK : S_FALSE;
+}
+
+HRESULT WINAPI AccTreeObject_EnumVARIANT_Skip(IEnumVARIANT *iface, ULONG celt)
+{
+    AccTreeObject *This = impl_from_AccTreeObject_EnumVARIANT(iface);
+
+    if ((celt + This->enum_pos) < This->info->child_count)
+    {
+	This->enum_pos += celt;
+	return S_OK;
+    }
+
+    return S_FALSE;
+}
+
+HRESULT WINAPI AccTreeObject_EnumVARIANT_Reset(IEnumVARIANT *iface)
+{
+    AccTreeObject *This = impl_from_AccTreeObject_EnumVARIANT(iface);
+
+    This->enum_pos = 0;
+
+    return S_OK;
+}
+
+HRESULT WINAPI AccTreeObject_EnumVARIANT_Clone(IEnumVARIANT *iface, IEnumVARIANT **ppEnum)
+{
+    return E_NOTIMPL;
+}
+
+const IEnumVARIANTVtbl AccTreeObjEnumVARIANTVtbl = {
+    AccTreeObject_EnumVARIANT_QueryInterface,
+    AccTreeObject_EnumVARIANT_AddRef,
+    AccTreeObject_EnumVARIANT_Release,
+    AccTreeObject_EnumVARIANT_Next,
+    AccTreeObject_EnumVARIANT_Skip,
+    AccTreeObject_EnumVARIANT_Reset,
+    AccTreeObject_EnumVARIANT_Clone
+};
+
+static HRESULT create_acc_obj_tree(const acc_tree_object_info *info,
+        unsigned int count)
+{
+    AccTreeObject *tree_objs;
+    unsigned int i;
+
+    object_tree = NULL;
+
+    tree_objs = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
+            sizeof(*tree_objs) * count);
+    if (!tree_objs)
+        return E_OUTOFMEMORY;
+
+    for (i = 0; i < count; i++)
+    {
+        const acc_tree_object_info *cur_obj = &info[i];
+        AccTreeObject *obj = &tree_objs[i];
+
+        obj->IAccessible_iface.lpVtbl = &AccTreeObjAccessibleVtbl;
+        obj->IEnumVARIANT_iface.lpVtbl = &AccTreeObjEnumVARIANTVtbl;
+        obj->ref = 1;
+        obj->info = cur_obj;
+
+        if (cur_obj->child_count)
+        {
+            obj->children = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
+                    sizeof(*obj->children) * cur_obj->child_count);
+            if (!obj->children)
+                return E_OUTOFMEMORY;
+        }
+
+        if (cur_obj->parent_id != ACC_TREE_OBJ_ID_NONE)
+        {
+            AccTreeObject *parent = &tree_objs[cur_obj->parent_id - 1];
+
+            obj->parent = &parent->IAccessible_iface;
+            parent->children[cur_obj->child_pos] = &obj->IAccessible_iface;
+        }
+    }
+
+    object_tree = tree_objs;
+
+    return S_OK;
+}
+
+static void free_acc_obj_tree(unsigned int count)
+{
+    unsigned int i;
+
+    if (!object_tree)
+        return;
+
+    for (i = 0; i < count; i++)
+    {
+        AccTreeObject *obj = &object_tree[i];
+
+        if (obj->children)
+            HeapFree(GetProcessHeap(), 0, obj->children);
+        IAccessible_Release(&obj->IAccessible_iface);
+    }
+
+    HeapFree(GetProcessHeap(), 0, object_tree);
+    object_tree = NULL;
+}
+
 static void test_getroletext(void)
 {
     INT ret, role;
@@ -564,7 +1314,13 @@ static LRESULT WINAPI test_window_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARA
         if(lparam == (DWORD)OBJID_CURSOR)
             return E_UNEXPECTED;
         if(lparam == (DWORD)OBJID_CLIENT)
+        {
+            if (object_tree)
+                return LresultFromObject(&IID_IAccessible, wparam,
+                        (IUnknown *)&object_tree->IAccessible_iface);
+
             return LresultFromObject(&IID_IUnknown, wparam, &Object);
+        }
         if(lparam == (DWORD)OBJID_WINDOW)
             return 0;
 
@@ -621,6 +1377,219 @@ static void test_AccessibleObjectFromWindow(void)
     DestroyWindow(hwnd);
 }
 
+static void test_AccessibleObjectFromEvent(void)
+{
+    IAccessible *acc, *acc_child;
+    const WCHAR *expected_name;
+    BSTR obj_name;
+    VARIANT cid;
+    HRESULT hr;
+    HWND hwnd;
+
+    hwnd = CreateWindowA("oleacc_test", "test", WS_OVERLAPPEDWINDOW,
+            0, 0, 0, 0, NULL, NULL, NULL, NULL);
+    ok(hwnd != NULL, "CreateWindow failed\n");
+
+    hr = create_acc_obj_tree(acc_from_event_obj_tree,
+            ARRAY_SIZE(acc_from_event_obj_tree));
+    if (FAILED(hr))
+    {
+        trace("Failed to create accessible object tree, hr %#x, aborting test.\n", hr);
+        DestroyWindow(hwnd);
+        return;
+    }
+
+    hr = AccessibleObjectFromEvent(NULL, OBJID_CLIENT, CHILDID_SELF, &acc, &cid);
+    ok(hr == E_FAIL, "got %#x\n", hr);
+
+    hr = AccessibleObjectFromEvent(hwnd, OBJID_CLIENT, CHILDID_SELF, NULL, &cid);
+    ok(hr == E_INVALIDARG, "got %#x\n", hr);
+
+    /* Same behavior as AccessibleObjectFromWindow. */
+    hr = AccessibleObjectFromEvent(hwnd, OBJID_CURSOR, CHILDID_SELF, &acc, &cid);
+    ok(hr == E_UNEXPECTED, "got %#x\n", hr);
+
+    /* Get a full object child. */
+    hr = AccessibleObjectFromEvent(hwnd, OBJID_CLIENT, ACC_TREE_OBJ_ID_CHILD_0, &acc, &cid);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(V_VT(&cid) == VT_I4, "got %#x, expected %#x\n", V_VT(&cid), VT_I4);
+    ok(V_I4(&cid) == CHILDID_SELF, "got %#x, expected %#x\n", V_I4(&cid), CHILDID_SELF);
+
+    expected_name = acc_from_event_obj_tree[ACC_TREE_OBJ_ID_CHILD_0 - 1].name;
+    hr = IAccessible_get_accName(acc, cid, &obj_name);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(!lstrcmpW(obj_name, expected_name), "expected %s, got %s\n",
+            wine_dbgstr_w(expected_name), wine_dbgstr_w(obj_name));
+    SysFreeString(obj_name);
+    IAccessible_Release(acc);
+
+    /* Get a 'simple element' child. */
+    hr = AccessibleObjectFromEvent(hwnd, OBJID_CLIENT, ACC_TREE_OBJ_ID_CHILD_2, &acc, &cid);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(V_VT(&cid) == VT_I4, "got %#x, expected %#x\n", V_VT(&cid), VT_I4);
+    ok(V_I4(&cid) == ACC_TREE_OBJ_ID_CHILD_2, "got %#x, expected %#x\n", V_I4(&cid), ACC_TREE_OBJ_ID_CHILD_2);
+
+    expected_name = acc_from_event_obj_tree[ACC_TREE_OBJ_ID_CHILD_2 - 1].name;
+    hr = IAccessible_get_accName(acc, cid, &obj_name);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(!lstrcmpW(obj_name, expected_name), "expected %s, got %s\n",
+            wine_dbgstr_w(expected_name), wine_dbgstr_w(obj_name));
+    SysFreeString(obj_name);
+
+    /*
+     * Make sure the IAccessible returned for a simple element is the
+     * root IAccessible.
+     */
+    V_I4(&cid) = CHILDID_SELF;
+    expected_name = acc_from_event_obj_tree[ACC_TREE_OBJ_ID_ROOT - 1].name;
+    hr = IAccessible_get_accName(acc, cid, &obj_name);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(!lstrcmpW(obj_name, expected_name), "expected %s, got %s\n",
+            wine_dbgstr_w(expected_name), wine_dbgstr_w(obj_name));
+    SysFreeString(obj_name);
+    IAccessible_Release(acc);
+
+    /*
+     * Invalid child ID's are treated by AccessibleObjectFromEvent as simple
+     * elements, regardless of if get_accChild returns a failure code instead
+     * of S_FALSE to indicate a simple element. The IAccessible returned is
+     * the root object, and child ID is still set to the invalid child ID.
+     */
+    V_I4(&cid) = 0;
+    V_VT(&cid) = VT_EMPTY;
+    hr = AccessibleObjectFromEvent(hwnd, OBJID_CLIENT, ACC_TREE_OBJ_ID_CHILD_2 + 1, &acc, &cid);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(V_VT(&cid) == VT_I4, "got %#x, expected %#x\n", V_VT(&cid), VT_I4);
+    ok(V_I4(&cid) == (ACC_TREE_OBJ_ID_CHILD_2 + 1), "got %#x, expected %#x\n",
+            V_I4(&cid), ACC_TREE_OBJ_ID_CHILD_2 + 1);
+
+    V_VT(&cid) = VT_I4;
+    V_I4(&cid) = CHILDID_SELF;
+    expected_name = acc_from_event_obj_tree[ACC_TREE_OBJ_ID_ROOT - 1].name;
+    hr = IAccessible_get_accName(acc, cid, &obj_name);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(!lstrcmpW(obj_name, expected_name), "expected %s, got %s\n",
+            wine_dbgstr_w(expected_name), wine_dbgstr_w(obj_name));
+    SysFreeString(obj_name);
+    IAccessible_Release(acc);
+
+    /*
+     * AccessibleObjectFromEvent will not search an object tree for a child,
+     * so if a given child ID is not returned by the get_accChild method on the
+     * root IAccessible, it won't be found by AccessibleObjectFromEvent. This
+     * is an issue in things such as web browsers, where one HWND is used as a
+     * root object with a complex tree of objects within it.
+     */
+    V_I4(&cid) = 0;
+    V_VT(&cid) = VT_EMPTY;
+    hr = AccessibleObjectFromEvent(hwnd, OBJID_CLIENT, ACC_TREE_OBJ_ID_CHILD_1_0, &acc, &cid);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(V_VT(&cid) == VT_I4, "got %#x, expected %#x\n", V_VT(&cid), VT_I4);
+    ok(V_I4(&cid) == (ACC_TREE_OBJ_ID_CHILD_1_0), "got %#x, expected %#x\n",
+            V_I4(&cid), ACC_TREE_OBJ_ID_CHILD_1_0);
+
+    V_VT(&cid) = VT_I4;
+    V_I4(&cid) = CHILDID_SELF;
+    expected_name = acc_from_event_obj_tree[ACC_TREE_OBJ_ID_ROOT - 1].name;
+    hr = IAccessible_get_accName(acc, cid, &obj_name);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(!lstrcmpW(obj_name, expected_name), "expected %s, got %s\n",
+            wine_dbgstr_w(expected_name), wine_dbgstr_w(obj_name));
+    SysFreeString(obj_name);
+    IAccessible_Release(acc);
+
+    V_I4(&cid) = 0;
+    V_VT(&cid) = VT_EMPTY;
+    hr = AccessibleObjectFromEvent(hwnd, OBJID_CLIENT, ACC_TREE_OBJ_ID_CHILD_1_1, &acc, &cid);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(V_VT(&cid) == VT_I4, "got %#x, expected %#x\n", V_VT(&cid), VT_I4);
+    ok(V_I4(&cid) == (ACC_TREE_OBJ_ID_CHILD_1_1), "got %#x, expected %#x\n",
+            V_I4(&cid), ACC_TREE_OBJ_ID_CHILD_1_1);
+
+    V_VT(&cid) = VT_I4;
+    V_I4(&cid) = CHILDID_SELF;
+    expected_name = acc_from_event_obj_tree[ACC_TREE_OBJ_ID_ROOT - 1].name;
+    hr = IAccessible_get_accName(acc, cid, &obj_name);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(!lstrcmpW(obj_name, expected_name), "expected %s, got %s\n",
+            wine_dbgstr_w(expected_name), wine_dbgstr_w(obj_name));
+    SysFreeString(obj_name);
+    IAccessible_Release(acc);
+
+    /*
+     * Show that ACC_TREE_OBJ_ID_CHILD_1's children are actually reachable
+     * from the root IAccessible using both possible methods of traversing
+     * the IAccessible tree (accNavigate and AccessibleChildren).
+     */
+    hr = AccessibleObjectFromEvent(hwnd, OBJID_CLIENT, CHILDID_SELF, &acc, &cid);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(V_VT(&cid) == VT_I4, "got %#x, expected %#x\n", V_VT(&cid), VT_I4);
+    ok(V_I4(&cid) == CHILDID_SELF, "got %#x, expected %#x\n", V_I4(&cid), CHILDID_SELF);
+
+    /* IAccessible_accNavigate method. */
+    V_I4(&cid) = 0;
+    V_VT(&cid) = VT_EMPTY;
+    acc_child = NULL;
+    search_acc_tree_for_child_navigate(acc, ACC_TREE_OBJ_ID_CHILD_1_0, &acc_child, &cid);
+    ok(!!acc_child, "Failed to find child id %#x in tree!\n", ACC_TREE_OBJ_ID_CHILD_1_0);
+
+    expected_name = acc_from_event_obj_tree[ACC_TREE_OBJ_ID_CHILD_1_0 - 1].name;
+    hr = IAccessible_get_accName(acc_child, cid, &obj_name);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(!lstrcmpW(obj_name, expected_name), "expected %s, got %s\n",
+            wine_dbgstr_w(expected_name), wine_dbgstr_w(obj_name));
+    SysFreeString(obj_name);
+    IAccessible_Release(acc_child);
+
+    V_I4(&cid) = 0;
+    V_VT(&cid) = VT_EMPTY;
+    acc_child = NULL;
+    search_acc_tree_for_child_navigate(acc, ACC_TREE_OBJ_ID_CHILD_1_1, &acc_child, &cid);
+    ok(!!acc_child, "Failed to find child id %#x in tree!\n", ACC_TREE_OBJ_ID_CHILD_1_1);
+
+    expected_name = acc_from_event_obj_tree[ACC_TREE_OBJ_ID_CHILD_1_1 - 1].name;
+    hr = IAccessible_get_accName(acc_child, cid, &obj_name);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(!lstrcmpW(obj_name, expected_name), "expected %s, got %s\n",
+            wine_dbgstr_w(expected_name), wine_dbgstr_w(obj_name));
+    SysFreeString(obj_name);
+    IAccessible_Release(acc_child);
+
+    /* AccessibleChildren (IEnumVARIANT) method. */
+    V_I4(&cid) = 0;
+    V_VT(&cid) = VT_EMPTY;
+    acc_child = NULL;
+    search_acc_tree_for_child_enumVARIANT(acc, ACC_TREE_OBJ_ID_CHILD_1_0, &acc_child, &cid);
+    ok(!!acc_child, "Failed to find child id %#x in tree!\n", ACC_TREE_OBJ_ID_CHILD_1_0);
+
+    expected_name = acc_from_event_obj_tree[ACC_TREE_OBJ_ID_CHILD_1_0 - 1].name;
+    hr = IAccessible_get_accName(acc_child, cid, &obj_name);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(!lstrcmpW(obj_name, expected_name), "expected %s, got %s\n",
+            wine_dbgstr_w(expected_name), wine_dbgstr_w(obj_name));
+    SysFreeString(obj_name);
+    IAccessible_Release(acc_child);
+
+    V_I4(&cid) = 0;
+    V_VT(&cid) = VT_EMPTY;
+    acc_child = NULL;
+    search_acc_tree_for_child_enumVARIANT(acc, ACC_TREE_OBJ_ID_CHILD_1_1, &acc_child, &cid);
+    ok(!!acc_child, "Failed to find child id %#x in tree!\n", ACC_TREE_OBJ_ID_CHILD_1_1);
+
+    expected_name = acc_from_event_obj_tree[ACC_TREE_OBJ_ID_CHILD_1_1 - 1].name;
+    hr = IAccessible_get_accName(acc_child, cid, &obj_name);
+    ok(SUCCEEDED(hr), "got %#x\n", hr);
+    ok(!lstrcmpW(obj_name, expected_name), "expected %s, got %s\n",
+            wine_dbgstr_w(expected_name), wine_dbgstr_w(obj_name));
+    SysFreeString(obj_name);
+    IAccessible_Release(acc_child);
+
+    IAccessible_Release(acc);
+
+    DestroyWindow(hwnd);
+    free_acc_obj_tree(ARRAY_SIZE(acc_from_event_obj_tree));
+}
+
 static void test_GetProcessHandleFromHwnd(void)
 {
     HANDLE proc;
@@ -1061,6 +2030,7 @@ START_TEST(main)
     test_GetProcessHandleFromHwnd();
     test_default_client_accessible_object();
     test_AccessibleChildren(&Accessible);
+    test_AccessibleObjectFromEvent();
 
     unregister_window_class();
     CoUninitialize();
-- 
2.25.1

[Huw Davies]

On Tue, Aug 10, 2021 at 08:42:56PM -0400, Connor McAdams wrote:
> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com>
> ---
>  dlls/oleacc/main.c      | 39 +++++++++++++++++++++++++++++++++++++++
>  dlls/oleacc/oleacc.spec |  2 +-
>  2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/oleacc/main.c b/dlls/oleacc/main.c
> index f6b66a8bcab..fd85518436c 100644
> --- a/dlls/oleacc/main.c
> +++ b/dlls/oleacc/main.c
> @@ -331,6 +331,45 @@ HRESULT WINAPI AccessibleObjectFromPoint( POINT ptScreen, IAccessible** ppacc, V
>      return E_NOTIMPL;
>  }
>
> +HRESULT WINAPI AccessibleObjectFromEvent( HWND hwnd, DWORD dwObjectID, DWORD dwChildID,
> +                             IAccessible** ppacc, VARIANT* pvarChild )

Not essential (and not done in most of this file) but we typically
drop the "dwppsz" prefix nonsense from variable names.  This function
is a little tricky as there are lots of uses of "child".  The 2nd param
could be "object_id", 3rd: "child_id", 4th: "acc" (maybe), 5th:
prehaps "child_out".  For the 1st, "hwnd" is pretty common or you
could use "win"/"wnd"/"window".
> +{
> +    IDispatch *child;
> +    VARIANT cid;
> +    HRESULT hr;
> +
> +    TRACE("%p %d %d %p %p\n", hwnd, dwObjectID, dwChildID, ppacc, pvarChild);
> +
> +    hr = AccessibleObjectFromWindow(hwnd, dwObjectID, &IID_IAccessible, (void **)ppacc);
> +    if (FAILED(hr))
> +        return hr;

I think having a separate variable for the window's IAccesible iface
makes sense here ("window_acc"/"parent_acc"?), rather than using the
supplied one.  You need a separate variable in the "if (child)" case
below anyway.
> +
> +    V_VT(&cid) = VT_I4;
> +    V_I4(&cid) = dwChildID;

It probably makes sense to add a helper to init these variants,
something like:
void variant_i4_init( VARIANT *v, int val );

Perhaps rename "cid" to "child_id_variant" (as I already mentioned this
function's overuse of "child" is tricky!)
> +    hr = IAccessible_get_accChild(*ppacc, cid, &child);
> +    if (FAILED(hr))
> +        FIXME("get_accChild failed with %#x!\n", hr);

It's unclear to me, having not spent any time looking at this API,
what failure here means.  Should we just return the failure (after
releasing "acc")?  The FIXME() implies it's not handled and yet
presumably that corresponds to the !child case below, so it seems like
you're trying to handle it?  Could this be tested?
> +
> +    V_VT(pvarChild) = VT_I4;
> +    if (child)
> +    {
> +        IAccessible *acc;
> +
> +        if (SUCCEEDED(IDispatch_QueryInterface(child, &IID_IAccessible, (void **)&acc)))
> +        {
> +            IAccessible_Release(*ppacc);
> +            *ppacc = acc;
> +        }
> +
> +        IDispatch_Release(child);
> +        V_I4(pvarChild) = CHILDID_SELF;
> +    }
> +    else
> +        V_I4(pvarChild) = dwChildID;

This block would be cleaned up using the variant init helper and
"window_acc".

Huw.

[Connor McAdams]

On Fri, Aug 27, 2021 at 09:26:09AM +0100, Huw Davies wrote:
> On Tue, Aug 10, 2021 at 08:42:56PM -0400, Connor McAdams wrote:
> > Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com>
> > ---
> >  dlls/oleacc/main.c      | 39 +++++++++++++++++++++++++++++++++++++++
> >  dlls/oleacc/oleacc.spec |  2 +-
> >  2 files changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/dlls/oleacc/main.c b/dlls/oleacc/main.c
> > index f6b66a8bcab..fd85518436c 100644
> > --- a/dlls/oleacc/main.c
> > +++ b/dlls/oleacc/main.c
> > @@ -331,6 +331,45 @@ HRESULT WINAPI AccessibleObjectFromPoint( POINT ptScreen, IAccessible** ppacc, V
> >      return E_NOTIMPL;
> >  }
> >
> > +HRESULT WINAPI AccessibleObjectFromEvent( HWND hwnd, DWORD dwObjectID, DWORD dwChildID,
> > +                             IAccessible** ppacc, VARIANT* pvarChild )
>
> Not essential (and not done in most of this file) but we typically
> drop the "dwppsz" prefix nonsense from variable names.  This function
> is a little tricky as there are lots of uses of "child".  The 2nd param
> could be "object_id", 3rd: "child_id", 4th: "acc" (maybe), 5th:
> prehaps "child_out".  For the 1st, "hwnd" is pretty common or you
> could use "win"/"wnd"/"window".
> > +{
> > +    IDispatch *child;
> > +    VARIANT cid;
> > +    HRESULT hr;
> > +
> > +    TRACE("%p %d %d %p %p\n", hwnd, dwObjectID, dwChildID, ppacc, pvarChild);
> > +
> > +    hr = AccessibleObjectFromWindow(hwnd, dwObjectID, &IID_IAccessible, (void **)ppacc);
> > +    if (FAILED(hr))
> > +        return hr;
>
> I think having a separate variable for the window's IAccesible iface
> makes sense here ("window_acc"/"parent_acc"?), rather than using the
> supplied one.  You need a separate variable in the "if (child)" case
> below anyway.
> > +
> > +    V_VT(&cid) = VT_I4;
> > +    V_I4(&cid) = dwChildID;
>
> It probably makes sense to add a helper to init these variants,
> something like:
> void variant_i4_init( VARIANT *v, int val );
>
> Perhaps rename "cid" to "child_id_variant" (as I already mentioned this
> function's overuse of "child" is tricky!)
> > +    hr = IAccessible_get_accChild(*ppacc, cid, &child);
> > +    if (FAILED(hr))
> > +        FIXME("get_accChild failed with %#x!\n", hr);
>
> It's unclear to me, having not spent any time looking at this API,
> what failure here means.  Should we just return the failure (after
> releasing "acc")?  The FIXME() implies it's not handled and yet
> presumably that corresponds to the !child case below, so it seems like
> you're trying to handle it?  Could this be tested?

So, according to MSDN, get_accChild is supposed to return 'S_FALSE' if
the child ID that is passed in represents a 'simple element', which is
an element that has its data retrieved from its parent IAccessible. If
the child ID represents a full IAccessible 'child', it returns S_OK and
an IDispatch interface. However, I've found through testing, that even
if get_accChild returns a failure code, AccessibleObjectFromEvent
doesn't fail, it just behaves as though it went through the simple
element path (S_FALSE) path. A FIXME may not be the best way to signify
this, but I feel like it's a little bit useful to have some indication
if this method returned a failure code for logging. Maybe a WARN or a
TRACE or something might be more appropriate. There is a test for this
in my current tests.
> > +
> > +    V_VT(pvarChild) = VT_I4;
> > +    if (child)
> > +    {
> > +        IAccessible *acc;
> > +
> > +        if (SUCCEEDED(IDispatch_QueryInterface(child, &IID_IAccessible, (void **)&acc)))
> > +        {
> > +            IAccessible_Release(*ppacc);
> > +            *ppacc = acc;
> > +        }
> > +
> > +        IDispatch_Release(child);
> > +        V_I4(pvarChild) = CHILDID_SELF;
> > +    }
> > +    else
> > +        V_I4(pvarChild) = dwChildID;
>
> This block would be cleaned up using the variant init helper and
> "window_acc".
>
> Huw.

Thanks for the review, and I'll get to work on cleaning things up for a
v2.

[Huw Davies]

On Fri, Aug 27, 2021 at 10:29:35AM -0400, Connor McAdams wrote:
> On Fri, Aug 27, 2021 at 09:26:09AM +0100, Huw Davies wrote:
> > On Tue, Aug 10, 2021 at 08:42:56PM -0400, Connor McAdams wrote:
> > > +    hr = IAccessible_get_accChild(*ppacc, cid, &child);
> > > +    if (FAILED(hr))
> > > +        FIXME("get_accChild failed with %#x!\n", hr);
> >
> > It's unclear to me, having not spent any time looking at this API,
> > what failure here means.  Should we just return the failure (after
> > releasing "acc")?  The FIXME() implies it's not handled and yet
> > presumably that corresponds to the !child case below, so it seems like
> > you're trying to handle it?  Could this be tested?
>
> So, according to MSDN, get_accChild is supposed to return 'S_FALSE' if
> the child ID that is passed in represents a 'simple element', which is
> an element that has its data retrieved from its parent IAccessible. If
> the child ID represents a full IAccessible 'child', it returns S_OK and
> an IDispatch interface. However, I've found through testing, that even
> if get_accChild returns a failure code, AccessibleObjectFromEvent
> doesn't fail, it just behaves as though it went through the simple
> element path (S_FALSE) path. A FIXME may not be the best way to signify
> this, but I feel like it's a little bit useful to have some indication
> if this method returned a failure code for logging. Maybe a WARN or a
> TRACE or something might be more appropriate. There is a test for this
> in my current tests.

Right, so definitely not a FIXME then, since you're apparently handling
it in the same way as Windows does.  Probably just a TRACE would do.

That being the case, you should NULL-init child and not rely on
the failing call to do it.

Be wary of "over-testing"; if this is something that is likely to
occur in practice that's fine, but if it's not then don't bother
probing it.

Huw.