[PATCH 2/7] oleacc: Add get_state function for edit client accessible object.

[Connor McAdams]

On Wed, Sep 22, 2021 at 07:19:44PM +0200, Piotr Caban wrote:
> Hi Connor,
> 
> On 9/22/21 6:20 PM, Connor McAdams wrote:
> > +typedef struct {
> > +    HRESULT (*get_state)(IAccessible *, VARIANT, VARIANT *);
> > +} WinClassVtbl;
> I think it's better to pass Client* as first argument. Please also avoid
> camel-case in new code.
> 
> > -static HRESULT WINAPI Client_get_accState(IAccessible *iface, VARIANT varID, VARIANT *pvarState)
> > +static void default_client_get_state(IAccessible *iface, VARIANT *var_state)
> >   {
> >       Client *This = impl_from_Client(iface);
> >       GUITHREADINFO info;
> >       LONG style;
> > -    TRACE("(%p)->(%s %p)\n", This, debugstr_variant(&varID), pvarState);
> > -
> > -    if(convert_child_id(&varID) != CHILDID_SELF) {
> > -        V_VT(pvarState) = VT_EMPTY;
> > -        return E_INVALIDARG;
> > -    }
> > -
> > -    V_VT(pvarState) = VT_I4;
> > -    V_I4(pvarState) = 0;
> > +    V_VT(var_state) = VT_I4;
> > +    V_I4(var_state) = 0;
> >       style = GetWindowLongW(This->hwnd, GWL_STYLE);
> >       if(style & WS_DISABLED)
> > -        V_I4(pvarState) |= STATE_SYSTEM_UNAVAILABLE;
> > +        V_I4(var_state) |= STATE_SYSTEM_UNAVAILABLE;
> >       else if(IsWindow(This->hwnd))
> > -        V_I4(pvarState) |= STATE_SYSTEM_FOCUSABLE;
> > +        V_I4(var_state) |= STATE_SYSTEM_FOCUSABLE;
> >       info.cbSize = sizeof(info);
> >       if(GetGUIThreadInfo(0, &info) && info.hwndFocus == This->hwnd)
> > -        V_I4(pvarState) |= STATE_SYSTEM_FOCUSED;
> > +        V_I4(var_state) |= STATE_SYSTEM_FOCUSED;
> >       if(!(style & WS_VISIBLE))
> > -        V_I4(pvarState) |= STATE_SYSTEM_INVISIBLE;
> > +        V_I4(var_state) |= STATE_SYSTEM_INVISIBLE;
> > +}
> > +
> > +static HRESULT WINAPI Client_get_accState(IAccessible *iface, VARIANT varID, VARIANT *pvarState)
> > +{
> > +    Client *This = impl_from_Client(iface);
> > +
> > +    TRACE("(%p)->(%s %p)\n", This, debugstr_variant(&varID), pvarState);
> > +
> > +    if (This->vtbl.get_state)
> > +        return This->vtbl.get_state(iface, varID, pvarState);
> > +
> > +    if(convert_child_id(&varID) != CHILDID_SELF) {
> > +        V_VT(pvarState) = VT_EMPTY;
> > +        return E_INVALIDARG;
> > +    }
> > +
> > +    default_client_get_state(iface, pvarState);
> > +
> >       return S_OK;
> >   }
> Isn't it enough to call vtbl.get_state near the end of the function? I'm
> asking about something along these lines (I've also changed get_state
> header):
> static HRESULT WINAPI Client_get_accState(IAccessible *iface, VARIANT varID,
> VARIANT *pvarState)
> {
> ...
>     info.cbSize = sizeof(info);
>     if(GetGUIThreadInfo(0, &info) && info.hwndFocus == This->hwnd)
>         V_I4(pvarState) |= STATE_SYSTEM_FOCUSED;
>     if(!(style & WS_VISIBLE))
>         V_I4(pvarState) |= STATE_SYSTEM_INVISIBLE;
> 
>     if(This->vtbl.get_state)
>         return This->vtbl.get_state(This, &V_I4(pvarState))
>     return S_OK;
> }
>

Yeah, I see what you're saying. That makes more sense. I guess in my
mind, I was thinking of the functions in the vtbl as more of an
"override" instead of an extension.

> Thanks,
> Piotr

Thanks for the review!