[PATCH v2] oleacc: Add support for retrieving an HWND from accNavigate to WindowFromAccessibleObject.
Signed-off-by: Connor McAdams cmcadams@codeweavers.com --- -v2: Fix test failures on 32-bit, make commit subject more clear.
In the prior commit message I wrote about changing behavior around get_accParent, that was inaccurate. The only difference here is that we call the accNavigate method only on the initial IAccessible, and not on any of the IAccessibles returned from get_accParent.
--- dlls/oleacc/main.c | 34 +++++++++-- dlls/oleacc/tests/main.c | 120 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 148 insertions(+), 6 deletions(-)
diff --git a/dlls/oleacc/main.c b/dlls/oleacc/main.c index dced84be7b5..0add9d9549d 100644 --- a/dlls/oleacc/main.c +++ b/dlls/oleacc/main.c @@ -402,20 +402,36 @@ HRESULT WINAPI WindowFromAccessibleObject(IAccessible *acc, HWND *phwnd) { IDispatch *parent; IOleWindow *ow; + VARIANT v, cid; HRESULT hres;
TRACE("%p %p\n", acc, phwnd);
IAccessible_AddRef(acc); - while(1) { - hres = IAccessible_QueryInterface(acc, &IID_IOleWindow, (void**)&ow); - if(SUCCEEDED(hres)) { - hres = IOleWindow_GetWindow(ow, phwnd); - IOleWindow_Release(ow); + + ow = NULL; + hres = IAccessible_QueryInterface(acc, &IID_IOleWindow, (void**)&ow); + if(SUCCEEDED(hres)) { + hres = IOleWindow_GetWindow(ow, phwnd); + IOleWindow_Release(ow); + if(*phwnd) { IAccessible_Release(acc); return hres; } + } + + VariantInit(&v); + variant_init_i4(&cid, CHILDID_SELF); + hres = IAccessible_accNavigate(acc, 10, cid, &v); + if(SUCCEEDED(hres) && V_VT(&v) == VT_I4) + *phwnd = IntToPtr(V_I4(&v)); + + if(ow) { + IAccessible_Release(acc); + return S_OK; + }
+ while(1) { hres = IAccessible_get_accParent(acc, &parent); IAccessible_Release(acc); if(FAILED(hres)) @@ -429,6 +445,14 @@ HRESULT WINAPI WindowFromAccessibleObject(IAccessible *acc, HWND *phwnd) IDispatch_Release(parent); if(FAILED(hres)) return hres; + + hres = IAccessible_QueryInterface(acc, &IID_IOleWindow, (void**)&ow); + if(SUCCEEDED(hres)) { + hres = IOleWindow_GetWindow(ow, phwnd); + IOleWindow_Release(ow); + IAccessible_Release(acc); + return hres; + } } }
diff --git a/dlls/oleacc/tests/main.c b/dlls/oleacc/tests/main.c index 7854764ec08..43dfa039f34 100644 --- a/dlls/oleacc/tests/main.c +++ b/dlls/oleacc/tests/main.c @@ -59,8 +59,10 @@ DEFINE_EXPECT(Accessible_get_accChildCount); DEFINE_EXPECT(Accessible_get_accChild); DEFINE_EXPECT(Accessible_get_accName); DEFINE_EXPECT(Accessible_get_accParent); +DEFINE_EXPECT(Accessible_accNavigate); DEFINE_EXPECT(Accessible_child_get_accName); DEFINE_EXPECT(Accessible_child_get_accParent); +DEFINE_EXPECT(Accessible_child_accNavigate);
static HANDLE (WINAPI *pGetProcessHandleFromHwnd)(HWND);
@@ -91,7 +93,11 @@ static BOOL iface_cmp(IUnknown *iface1, IUnknown *iface2) return unk1 == unk2; }
+static IAccessible Accessible; static IAccessible Accessible_child; +static IOleWindow OleWindow; +static HWND Accessible_accnav_hwnd = NULL; +static HWND OleWindow_hwnd = NULL;
static HRESULT WINAPI Accessible_QueryInterface( IAccessible *iface, REFIID riid, void **ppvObject) @@ -104,6 +110,12 @@ static HRESULT WINAPI Accessible_QueryInterface( return S_OK; }
+ if(IsEqualIID(riid, &IID_IOleWindow) && (iface == &Accessible)) { + *ppvObject = &OleWindow; + IAccessible_AddRef(iface); + return S_OK; + } + if(IsEqualIID(riid, &IID_IEnumVARIANT)) { CHECK_EXPECT(Accessible_QI_IEnumVARIANT); return E_NOINTERFACE; @@ -155,7 +167,12 @@ static HRESULT WINAPI Accessible_get_accParent( IAccessible *iface, IDispatch **ppdispParent) { if(iface == &Accessible_child) + { CHECK_EXPECT(Accessible_child_get_accParent); + if (OleWindow_hwnd) + return IAccessible_QueryInterface(&Accessible, &IID_IDispatch, + (void **)ppdispParent); + } else CHECK_EXPECT(Accessible_get_accParent);
@@ -295,7 +312,21 @@ static HRESULT WINAPI Accessible_accLocation(IAccessible *iface, LONG *pxLeft, static HRESULT WINAPI Accessible_accNavigate(IAccessible *iface, LONG navDir, VARIANT varStart, VARIANT *pvarEnd) { - ok(0, "unexpected call\n"); + if(iface == &Accessible_child) + CHECK_EXPECT(Accessible_child_accNavigate); + else + CHECK_EXPECT(Accessible_accNavigate); + + /* + * Magic number value for retrieving an HWND. Used by DynamicAnnotation + * IAccessible wrapper. + */ + if(navDir == 10) { + V_VT(pvarEnd) = VT_I4; + V_I4(pvarEnd) = HandleToULong(Accessible_accnav_hwnd); + return S_OK; + } + return E_NOTIMPL; }
@@ -358,8 +389,44 @@ static IAccessibleVtbl AccessibleVtbl = { Accessible_put_accValue };
+static HRESULT WINAPI OleWindow_QueryInterface(IOleWindow *iface, REFIID riid, void **obj) +{ + return IAccessible_QueryInterface(&Accessible, riid, obj); +} + +static ULONG WINAPI OleWindow_AddRef(IOleWindow *iface) +{ + return IAccessible_AddRef(&Accessible); +} + +static ULONG WINAPI OleWindow_Release(IOleWindow *iface) +{ + return IAccessible_Release(&Accessible); +} + +static HRESULT WINAPI OleWindow_GetWindow(IOleWindow *iface, HWND *hwnd) +{ + *hwnd = OleWindow_hwnd; + return S_OK; +} + +static HRESULT WINAPI OleWindow_ContextSensitiveHelp(IOleWindow *iface, BOOL f_enter_mode) +{ + ok(0, "unexpected call\n"); + return E_NOTIMPL; +} + +static const IOleWindowVtbl OleWindowVtbl = { + OleWindow_QueryInterface, + OleWindow_AddRef, + OleWindow_Release, + OleWindow_GetWindow, + OleWindow_ContextSensitiveHelp +}; + static IAccessible Accessible = {&AccessibleVtbl}; static IAccessible Accessible_child = {&AccessibleVtbl}; +static IOleWindow OleWindow = {&OleWindowVtbl};
static void test_getroletext(void) { @@ -1826,6 +1893,56 @@ static void test_default_edit_accessible_object(void) DestroyWindow(hwnd); }
+static void test_WindowFromAccessibleObject(void) +{ + HRESULT hr; + HWND hwnd; + + /* Successfully retrieve an HWND from the IOleWindow interface. */ + Accessible_accnav_hwnd = NULL; + OleWindow_hwnd = (HWND)0xdeadf00d; + hwnd = (HWND)0xdeadbeef; + hr = WindowFromAccessibleObject(&Accessible, &hwnd); + ok(hr == S_OK, "got %lx\n", hr); + ok(hwnd == (HWND)0xdeadf00d, "hwnd != 0xdeadf00d!\n"); + + /* Successfully retrieve an HWND from IAccessible::accNavigate. */ + Accessible_accnav_hwnd = (HWND)0xdeadf00d; + OleWindow_hwnd = NULL; + hwnd = (HWND)0xdeadbeef; + SET_EXPECT(Accessible_accNavigate); + hr = WindowFromAccessibleObject(&Accessible, &hwnd); + ok(hr == S_OK, "got %lx\n", hr); + /* This value gets sign-extended on 64-bit. */ + ok(hwnd == IntToPtr(0xdeadf00d), "hwnd != 0xdeadf00d!\n", hwnd); + CHECK_CALLED(Accessible_accNavigate); + + /* Return a NULL HWND from both methods, no accParent call. */ + Accessible_accnav_hwnd = NULL; + OleWindow_hwnd = NULL; + hwnd = (HWND)0xdeadbeef; + SET_EXPECT(Accessible_accNavigate); + hr = WindowFromAccessibleObject(&Accessible, &hwnd); + ok(hr == S_OK, "got %lx\n", hr); + ok(!hwnd, "hwnd %p\n", hwnd); + CHECK_CALLED(Accessible_accNavigate); + + /* Successfully retrieve an HWND from a parent IAccessible's IOleWindow interface. */ + Accessible_accnav_hwnd = NULL; + OleWindow_hwnd = (HWND)0xdeadf00d; + hwnd = (HWND)0xdeadbeef; + SET_EXPECT(Accessible_child_accNavigate); + SET_EXPECT(Accessible_child_get_accParent); + hr = WindowFromAccessibleObject(&Accessible_child, &hwnd); + ok(hr == S_OK, "got %lx\n", hr); + ok(hwnd == (HWND)0xdeadf00d, "hwnd != 0xdeadf00d!\n"); + CHECK_CALLED(Accessible_child_accNavigate); + CHECK_CALLED(Accessible_child_get_accParent); + + Accessible_accnav_hwnd = NULL; + OleWindow_hwnd = NULL; +} + START_TEST(main) { int argc; @@ -1867,6 +1984,7 @@ START_TEST(main) test_AccessibleObjectFromPoint(); test_CreateStdAccessibleObject_classes(); test_default_edit_accessible_object(); + test_WindowFromAccessibleObject();
unregister_window_class(); CoUninitialize();
Hi Connor,
On 5/11/22 22:56, Connor McAdams wrote:
@@ -402,20 +402,36 @@ HRESULT WINAPI WindowFromAccessibleObject(IAccessible *acc, HWND *phwnd) { IDispatch *parent; IOleWindow *ow;
VARIANT v, cid; HRESULT hres;
TRACE("%p %p\n", acc, phwnd);
IAccessible_AddRef(acc);
There's no need to increase acc refcount at this point. It can be done before the while loop.
- while(1) {
hres = IAccessible_QueryInterface(acc, &IID_IOleWindow, (void**)&ow);if(SUCCEEDED(hres)) {hres = IOleWindow_GetWindow(ow, phwnd);IOleWindow_Release(ow);
- ow = NULL;
I don't like setting ow to NULL here. It will be probably better to restructure the code so it's not needed. It leads to some errors in further code.
- hres = IAccessible_QueryInterface(acc, &IID_IOleWindow, (void**)&ow);
- if(SUCCEEDED(hres)) {
hres = IOleWindow_GetWindow(ow, phwnd);IOleWindow_Release(ow);if(*phwnd) {
You should not base anything on *phwnd value at this point. It may be uninitialized if IOleWindow_GetWindow returned error.
IAccessible_Release(acc); return hres; }
- }
- VariantInit(&v);
- variant_init_i4(&cid, CHILDID_SELF);
- hres = IAccessible_accNavigate(acc, 10, cid, &v);
How about defining something like NAVDIR_INTERNAL_HWND and using it instead of a magic constant?
- if(SUCCEEDED(hres) && V_VT(&v) == VT_I4)
*phwnd = IntToPtr(V_I4(&v));- if(ow) {
IAccessible_Release(acc);return S_OK;- }
This looks very suspicious. It depends on phwnd to be set by GetWindow or accNavigate call. It will e.g. not work when both GetWindow and accNavigate returns error. The code is hard to follow while it's written this way.
- while(1) { hres = IAccessible_get_accParent(acc, &parent); IAccessible_Release(acc); if(FAILED(hres))
@@ -429,6 +445,14 @@ HRESULT WINAPI WindowFromAccessibleObject(IAccessible *acc, HWND *phwnd) IDispatch_Release(parent); if(FAILED(hres)) return hres;
hres = IAccessible_QueryInterface(acc, &IID_IOleWindow, (void**)&ow);if(SUCCEEDED(hres)) {hres = IOleWindow_GetWindow(ow, phwnd);IOleWindow_Release(ow);IAccessible_Release(acc);return hres;} }diff --git a/dlls/oleacc/tests/main.c b/dlls/oleacc/tests/main.c index 7854764ec08..43dfa039f34 100644 --- a/dlls/oleacc/tests/main.c +++ b/dlls/oleacc/tests/main.c @@ -59,8 +59,10 @@ DEFINE_EXPECT(Accessible_get_accChildCount); DEFINE_EXPECT(Accessible_get_accChild); DEFINE_EXPECT(Accessible_get_accName); DEFINE_EXPECT(Accessible_get_accParent); +DEFINE_EXPECT(Accessible_accNavigate); DEFINE_EXPECT(Accessible_child_get_accName); DEFINE_EXPECT(Accessible_child_get_accParent); +DEFINE_EXPECT(Accessible_child_accNavigate);
static HANDLE (WINAPI *pGetProcessHandleFromHwnd)(HWND);
@@ -91,7 +93,11 @@ static BOOL iface_cmp(IUnknown *iface1, IUnknown *iface2) return unk1 == unk2; }
+static IAccessible Accessible; static IAccessible Accessible_child; +static IOleWindow OleWindow; +static HWND Accessible_accnav_hwnd = NULL; +static HWND OleWindow_hwnd = NULL;
static variables are already zeroed.
Thanks, Piotr
-
Connor McAdams -
Piotr Caban