Re: [PATCH v3] oleacc: Add support for retrieving an HWND from accNavigate to WindowFromAccessibleObject.

[Connor McAdams]

On Mon, May 16, 2022 at 04:17:10PM +0200, Piotr Caban wrote:
> Hi Connor,
>
> On 5/13/22 18:43, Connor McAdams wrote:
> > @@ -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);
>
> I don't understand this part. Why do you need to check OleWindow_hwnd? It
> would probably make sense to return parent unconditionally.

Largely to gate returning a parent interface in other tests that call
get_accParent on Accessible_child. The idea was to only return a parent
interface for the WindowFromAccessibleObject tests where we're trying to
retrieve an HWND from the parent's IOleWindow interface.
> > +#define NAVDIR_INTERNAL_HWND 10
> >   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 == NAVDIR_INTERNAL_HWND) {
> > -
> >    V_VT(pvarEnd) = VT_I4;
> > -
> >    V_I4(pvarEnd) = HandleToULong(Accessible_accnav_hwnd);
> > -
> >    return S_OK;
>
> S_FALSE or E_INVALIDARG looks like a better return value when
> Accessible_accnav_hwnd is NULL (unless there's value in testing S_OK). How
> about changing Accessible_accnav_hwnd name to Accessible_hwnd?

I was mainly testing whether or not it'd accept returning a NULL hwnd,
or continue trying to use other methods to ascertain the HWND if NULL is
returned. I can try other return codes. As it is, it seems that if the
IAccessible doesn't have an IOleWindow interface, and the accNav method
returns a NULL hwnd, it goes up the parent chain. If the accNav method
returns a non-NULL hwnd, it returns immediately.
> > +static HRESULT WINAPI OleWindow_GetWindow(IOleWindow *iface, HWND *hwnd)
> > +{
> > - *hwnd = OleWindow_hwnd;
> > - return S_OK;
>
> Again, I don't know if there's value in testing NULL HWND and S_OK return.
> If not, documentation suggests returning E_FAIL.
>
> Thanks,
> Piotr

Sure. I can do E_FAIL here instead if the hwnd is NULL. I hadn't looked
at the IOleWindow documentation, guess I should have. :)

Thanks for the review.