Re: [PATCH v2 1/7] oleacc: Add default edit client accessible object creation function.

[Connor McAdams]

On Thu, Sep 23, 2021 at 04:12:37PM +0200, Piotr Caban wrote:
> Hi Connor,
>
> It looks like the code for handling class specific behavior may use some
> more changes.
> - The list of client / window classes that needs special handling is
>
> different. I think it's better to keep it separated (attached patch 1).
> Please note that the list may be incomplete.
> - Instead of adding more arguments to create function add init function to
>
> the vtbl (patch 2).
> - Change get_state function so it can be used for listbox (patch 3).
>
> What do you think about it?
>
> Thanks,
> Piotr

Yeah, I'm thinking this approach looks better at a glance. I'm going to
try and do a little bit of testing/research on the general behavior of
each window class tomorrow to try and figure out which ones behave more
like the default and which ones are more quirky.

Thanks!
> From e5de42525c84af47bb591734d1897b19da915692 Mon Sep 17 00:00:00 2001
> From: Piotr Caban 
> Date: Thu, 23 Sep 2021 15:25:11 +0200
> Subject: [PATCH 1/3] oleacc: Reorganize class specific behaviour handling.
> To: wine-devel
>
> dlls/oleacc/client.c         | 36 ++++++++++++++++
>  dlls/oleacc/main.c           | 82 +++++-------------------------------
>  dlls/oleacc/oleacc_private.h |  7 +++
>  dlls/oleacc/window.c         |  9 ++++
>  4 files changed, 62 insertions(+), 72 deletions(-)
>
> diff --git a/dlls/oleacc/client.c b/dlls/oleacc/client.c
> index 93f6b590f78..333a95dc388 100644
> --- a/dlls/oleacc/client.c
> +++ b/dlls/oleacc/client.c
> @@ -19,6 +19,7 @@
>  #define COBJMACROS
>
> #include "oleacc_private.h"
> +#include "commctrl.h"
>
> #include "wine/debug.h"
>  #include "wine/heap.h"
> @@ -650,6 +651,39 @@ static const IEnumVARIANTVtbl ClientEnumVARIANTVtbl = {
>      Client_EnumVARIANT_Clone
>  };
>
> +static const struct win_class_data classes[] = {
> - {WC_LISTBOXW,           0x10000, TRUE},
> - {L"#32768",             0x10001, TRUE}, /* menu */
> - {WC_BUTTONW,            0x10002, TRUE},
> - {WC_STATICW,            0x10003, TRUE},
> - {WC_EDITW,              0x10004, TRUE},
> - {WC_COMBOBOXW,          0x10005, TRUE},
> - {L"#32770",             0x10006, TRUE}, /* dialog */
> - {L"#32771",             0x10007, TRUE}, /* winswitcher */
> - {L"MDIClient",          0x10008, TRUE},
> - {L"#32769",             0x10009, TRUE}, /* desktop */
> - {WC_SCROLLBARW,         0x1000a, TRUE},
> - {STATUSCLASSNAMEW,      0x1000b, TRUE},
> - {TOOLBARCLASSNAMEW,     0x1000c, TRUE},
> - {PROGRESS_CLASSW,       0x1000d, TRUE},
> - {ANIMATE_CLASSW,        0x1000e, TRUE},
> - {WC_TABCONTROLW,        0x1000f, TRUE},
> - {HOTKEY_CLASSW,         0x10010, TRUE},
> - {WC_HEADERW,            0x10011, TRUE},
> - {TRACKBAR_CLASSW,       0x10012, TRUE},
> - {WC_LISTVIEWW,          0x10013, TRUE},
> - {UPDOWN_CLASSW,         0x10016, TRUE},
> - {TOOLTIPS_CLASSW,       0x10018, TRUE},
> - {WC_TREEVIEWW,          0x10019, TRUE},
> - {MONTHCAL_CLASSW,       0, TRUE},
> - {DATETIMEPICK_CLASSW,   0, TRUE},
> - {WC_IPADDRESSW,         0, TRUE},
> - {L"RICHEDIT",           0x1001c, TRUE},
> - {L"RichEdit20A",        0, TRUE},
> - {L"RichEdit20W",        0, TRUE},
> - {NULL}
>
> +};
> -
>
> HRESULT create_client_object(HWND hwnd, const IID *iid, void **obj)
>  {
>      Client *client;
> @@ -662,6 +696,8 @@ HRESULT create_client_object(HWND hwnd, const IID *iid, void **obj)
>      if(!client)
>          return E_OUTOFMEMORY;
> - find_class_data(hwnd, classes);
> - client->IAccessible_iface.lpVtbl = &ClientVtbl;
>    client->IOleWindow_iface.lpVtbl = &ClientOleWindowVtbl;
>    client->IEnumVARIANT_iface.lpVtbl = &ClientEnumVARIANTVtbl;
>
> diff --git a/dlls/oleacc/main.c b/dlls/oleacc/main.c
> index db9f646988c..3ce616ae0c1 100644
> --- a/dlls/oleacc/main.c
> +++ b/dlls/oleacc/main.c
> @@ -20,13 +20,6 @@
>
> #define COBJMACROS
>
> -#include <stdarg.h>
> -#include "windef.h"
> -#include "winbase.h"
> -#include "ole2.h"
> -#include "commctrl.h"
> -#include "rpcproxy.h"
> -
>
> #include "initguid.h"
>  #include "oleacc_private.h"
>  #include "resource.h"
> @@ -37,50 +30,11 @@ WINE_DEFAULT_DEBUG_CHANNEL(oleacc);
>
> static const WCHAR lresult_atom_prefix[] = {'w','i','n','e','_','o','l','e','a','c','c',':'};
>
> -typedef HRESULT (*accessible_create)(HWND, const IID*, void**);
> -
>
> extern HRESULT WINAPI OLEACC_DllGetClassObject(REFCLSID, REFIID, void**) DECLSPEC_HIDDEN;
>  extern BOOL WINAPI OLEACC_DllMain(HINSTANCE, DWORD, void*) DECLSPEC_HIDDEN;
>  extern HRESULT WINAPI OLEACC_DllRegisterServer(void) DECLSPEC_HIDDEN;
>  extern HRESULT WINAPI OLEACC_DllUnregisterServer(void) DECLSPEC_HIDDEN;
>
> -static struct {
> - const WCHAR *name;
> - DWORD idx;
> - accessible_create create_client;
> - accessible_create create_window;
>
> -} builtin_classes[] = {
> - {WC_LISTBOXW,           0x10000, NULL, NULL},
> - {L"#32768",             0x10001, NULL, NULL}, /* menu */
> - {WC_BUTTONW,            0x10002, NULL, NULL},
> - {WC_STATICW,            0x10003, NULL, NULL},
> - {WC_EDITW,              0x10004, NULL, NULL},
> - {WC_COMBOBOXW,          0x10005, NULL, NULL},
> - {L"#32770",             0x10006, NULL, NULL}, /* dialog */
> - {L"#32771",             0x10007, NULL, NULL}, /* winswitcher */
> - {L"MDIClient",          0x10008, NULL, NULL},
> - {L"#32769",             0x10009, NULL, NULL}, /* desktop */
> - {WC_SCROLLBARW,         0x1000a, NULL, NULL},
> - {STATUSCLASSNAMEW,      0x1000b, NULL, NULL},
> - {TOOLBARCLASSNAMEW,     0x1000c, NULL, NULL},
> - {PROGRESS_CLASSW,       0x1000d, NULL, NULL},
> - {ANIMATE_CLASSW,        0x1000e, NULL, NULL},
> - {WC_TABCONTROLW,        0x1000f, NULL, NULL},
> - {HOTKEY_CLASSW,         0x10010, NULL, NULL},
> - {WC_HEADERW,            0x10011, NULL, NULL},
> - {TRACKBAR_CLASSW,       0x10012, NULL, NULL},
> - {WC_LISTVIEWW,          0x10013, NULL, NULL},
> - {UPDOWN_CLASSW,         0x10016, NULL, NULL},
> - {TOOLTIPS_CLASSW,       0x10018, NULL, NULL},
> - {WC_TREEVIEWW,          0x10019, NULL, NULL},
> - {MONTHCAL_CLASSW,       0,       NULL, NULL},
> - {DATETIMEPICK_CLASSW,   0,       NULL, NULL},
> - {WC_IPADDRESSW,         0,       NULL, NULL},
> - {L"RICHEDIT",           0x1001c, NULL, NULL},
> - {L"RichEdit20A",        0,       NULL, NULL},
> - {L"RichEdit20W",        0,       NULL, NULL},
>
> -};
> -
>
> static HINSTANCE oleacc_handle = 0;
>
> int convert_child_id(VARIANT *v)
> @@ -94,7 +48,7 @@ int convert_child_id(VARIANT *v)
>      }
>  }
>
> -static accessible_create get_builtin_accessible_obj(HWND hwnd, LONG objid)
> +const struct win_class_data* find_class_data(HWND hwnd, const struct win_class_data *classes)
>  {
>      WCHAR class_name[64];
>      int i, idx;
> @@ -103,31 +57,21 @@ static accessible_create get_builtin_accessible_obj(HWND hwnd, LONG objid)
>          return NULL;
>      TRACE("got window class: %s\n", debugstr_w(class_name));
> - for(i=0; i<ARRAY_SIZE(builtin_classes); i++) {
> -
>    if(!wcsicmp(class_name, builtin_classes[i].name)) {
> -
>        accessible_create ret;
> -
> -
>        ret = (objid==OBJID_CLIENT ?
> -
>                builtin_classes[i].create_client :
> -
>                builtin_classes[i].create_window);
> -
>        if(!ret)
> - for(i=0; classes[i].name; i++) {
> -
>    if(!wcsicmp(class_name, classes[i].name)) {
> -
>        if(classes[i].stub)
>            FIXME("unhandled window class: %s\n", debugstr_w(class_name));
> -
>        return ret;
> -
>        return &classes[i];
>    }
>
> }
>
> idx = SendMessageW(hwnd, WM_GETOBJECT, 0, OBJID_QUERYCLASSNAMEIDX);
>    if(idx) {
> -
>    for(i=0; i<ARRAY_SIZE(builtin_classes); i++) {
> -
>        if(idx == builtin_classes[i].idx) {
> -
>            accessible_create ret;
> -
> -
>            ret = (objid==OBJID_CLIENT ?
> -
>                    builtin_classes[i].create_client :
> -
>                    builtin_classes[i].create_window);
> -
>            if(!ret)
> -
>                FIXME("unhandled class name idx: %x\n", idx);
> -
>            return ret;
> -
>    for(i=0; classes[i].name; i++) {
> -
>        if(idx == classes[i].idx) {
> -
>            if(classes[i].stub)
> -
>                FIXME("unhandled window class: %s\n", debugstr_w(class_name));
> -
>            return &classes[i];
>        }
>    }
>
> @@ -140,19 +84,13 @@ static accessible_create get_builtin_accessible_obj(HWND hwnd, LONG objid)
>  HRESULT WINAPI CreateStdAccessibleObject( HWND hwnd, LONG idObject,
>          REFIID riidInterface, void** ppvObject )
>  {
> -
>
> accessible_create create;
> -
>
> TRACE("%p %d %s %p\n", hwnd, idObject,
>          debugstr_guid( riidInterface ), ppvObject );
>
> switch(idObject) {
>    case OBJID_CLIENT:
> -
>    create = get_builtin_accessible_obj(hwnd, idObject);
> -
>    if(create) return create(hwnd, riidInterface, ppvObject);
>    return create_client_object(hwnd, riidInterface, ppvObject);
>
> case OBJID_WINDOW:
> -
>    create = get_builtin_accessible_obj(hwnd, idObject);
> -
>    if(create) return create(hwnd, riidInterface, ppvObject);
>    return create_window_object(hwnd, riidInterface, ppvObject);
>
> default:
>        FIXME("unhandled object id: %d\n", idObject);
>
> diff --git a/dlls/oleacc/oleacc_private.h b/dlls/oleacc/oleacc_private.h
> index 32561ef3d2f..64926b6abb7 100644
> --- a/dlls/oleacc/oleacc_private.h
> +++ b/dlls/oleacc/oleacc_private.h
> @@ -18,6 +18,13 @@
>
> #include "oleacc_classes.h"
>
> +struct win_class_data {
> - const WCHAR *name;
> - DWORD idx;
> - BOOL stub;
>
> +};
> +const struct win_class_data* find_class_data(HWND, const struct win_class_data*) DECLSPEC_HIDDEN;
> -
>
> HRESULT create_client_object(HWND, const IID*, void**) DECLSPEC_HIDDEN;
>  HRESULT create_window_object(HWND, const IID*, void**) DECLSPEC_HIDDEN;
>  HRESULT get_accpropservices_factory(REFIID, void**) DECLSPEC_HIDDEN;
> diff --git a/dlls/oleacc/window.c b/dlls/oleacc/window.c
> index 387ed4bc9f7..aa433d624cf 100644
> --- a/dlls/oleacc/window.c
> +++ b/dlls/oleacc/window.c
> @@ -19,6 +19,7 @@
>  #define COBJMACROS
>
> #include "oleacc_private.h"
> +#include "commctrl.h"
>
> #include "wine/debug.h"
>  #include "wine/heap.h"
> @@ -416,6 +417,12 @@ static const IEnumVARIANTVtbl WindowEnumVARIANTVtbl = {
>      Window_EnumVARIANT_Clone
>  };
>
> +static const struct win_class_data classes[] = {
> - {WC_LISTBOXW,           0x10000, TRUE},
> - {L"#32768",             0x10001, TRUE}, /* menu */
> - {NULL}
>
> +};
> -
>
> HRESULT create_window_object(HWND hwnd, const IID *iid, void **obj)
>  {
>      Window *window;
> @@ -428,6 +435,8 @@ HRESULT create_window_object(HWND hwnd, const IID *iid, void **obj)
>      if(!window)
>          return E_OUTOFMEMORY;
> - find_class_data(hwnd, classes);
> - window->IAccessible_iface.lpVtbl = &WindowVtbl;
>    window->IOleWindow_iface.lpVtbl = &WindowOleWindowVtbl;
>    window->IEnumVARIANT_iface.lpVtbl = &WindowEnumVARIANTVtbl;
>
> -- 
> 2.32.0
> From ff6ebfe14faee0aaca86fa8e8f38eae8e996a4cd Mon Sep 17 00:00:00 2001
> From: Connor McAdams 
> Date: Wed, 22 Sep 2021 16:12:35 -0400
> Subject: [PATCH 2/3] oleacc: Add get_accRole implementation for edit client
>  accessible object.
> To: wine-devel
> Signed-off-by: Connor McAdams
>
> dlls/oleacc/client.c         | 32 ++++++++++++++++++++++++++++----
>  dlls/oleacc/oleacc_private.h |  1 +
>  2 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/dlls/oleacc/client.c b/dlls/oleacc/client.c
> index 333a95dc388..4a0471a2657 100644
> --- a/dlls/oleacc/client.c
> +++ b/dlls/oleacc/client.c
> @@ -26,6 +26,7 @@
>
> WINE_DEFAULT_DEBUG_CHANNEL(oleacc);
>
> +typedef struct win_class_vtbl win_class_vtbl;
>  typedef struct {
>      IAccessible IAccessible_iface;
>      IOleWindow IOleWindow_iface;
> @@ -35,8 +36,15 @@ typedef struct {
>  HWND hwnd;
>  HWND enum_pos;
> - INT role;
> -
> - const win_class_vtbl *vtbl;
>
> } Client;
>
> +struct win_class_vtbl {
> - void (*init)(Client*);
>
> +};
> -
>
> static inline Client* impl_from_Client(IAccessible *iface)
>  {
>      return CONTAINING_RECORD(iface, Client, IAccessible_iface);
> @@ -221,7 +229,7 @@ static HRESULT WINAPI Client_get_accRole(IAccessible *iface, VARIANT varID, VARI
>      }
>  V_VT(pvarRole) = VT_I4;
> - V_I4(pvarRole) = ROLE_SYSTEM_CLIENT;
> - V_I4(pvarRole) = This->role;
> return S_OK;
>
> }
>
> @@ -651,12 +659,21 @@ static const IEnumVARIANTVtbl ClientEnumVARIANTVtbl = {
>      Client_EnumVARIANT_Clone
>  };
>
> +static void edit_init(Client *client)
> +{
> - client->role = ROLE_SYSTEM_TEXT;
>
> +}
> -
>
> +static const win_class_vtbl edit_vtbl = {
> - edit_init,
>
> +};
> -
>
> static const struct win_class_data classes[] = {
>      {WC_LISTBOXW,           0x10000, TRUE},
>      {L"#32768",             0x10001, TRUE}, /* menu */
>      {WC_BUTTONW,            0x10002, TRUE},
>      {WC_STATICW,            0x10003, TRUE},
> - {WC_EDITW,              0x10004, TRUE},
> - {WC_EDITW,              0x10004, TRUE, &edit_vtbl},
> {WC_COMBOBOXW,          0x10005, TRUE},
> {L"#32770",             0x10006, TRUE}, /* dialog */
> {L"#32771",             0x10007, TRUE}, /* winswitcher */
>
> @@ -686,8 +703,9 @@ static const struct win_class_data classes[] = {
>
> HRESULT create_client_object(HWND hwnd, const IID *iid, void **obj)
>  {
> - const struct win_class_data *data;
> Client *client;
> - HRESULT hres;
> -
>
> HRESULT hres = S_OK;
>
> if(!IsWindow(hwnd))
>     return E_FAIL;
>
> @@ -696,7 +714,7 @@ HRESULT create_client_object(HWND hwnd, const IID *iid, void **obj)
>      if(!client)
>          return E_OUTOFMEMORY;
> - find_class_data(hwnd, classes);
> -
>
> data = find_class_data(hwnd, classes);
>
> client->IAccessible_iface.lpVtbl = &ClientVtbl;
> client->IOleWindow_iface.lpVtbl = &ClientOleWindowVtbl;
>
> @@ -704,6 +722,12 @@ HRESULT create_client_object(HWND hwnd, const IID *iid, void **obj)
>      client->ref = 1;
>      client->hwnd = hwnd;
>      client->enum_pos = 0;
> -
>
> client->role = ROLE_SYSTEM_CLIENT;
> -
> -
>
> if(data)
> -
>    client->vtbl = data->vtbl;
> -
>
> if(client->vtbl && client->vtbl->init)
> -
>    client->vtbl->init(client);
>
> hres = IAccessible_QueryInterface(&client->IAccessible_iface, iid, obj);
>    IAccessible_Release(&client->IAccessible_iface);
>
> diff --git a/dlls/oleacc/oleacc_private.h b/dlls/oleacc/oleacc_private.h
> index 64926b6abb7..a01fc3615fe 100644
> --- a/dlls/oleacc/oleacc_private.h
> +++ b/dlls/oleacc/oleacc_private.h
> @@ -22,6 +22,7 @@ struct win_class_data {
>      const WCHAR *name;
>      DWORD idx;
>      BOOL stub;
> - const void *vtbl;
>
> };
>  const struct win_class_data* find_class_data(HWND, const struct win_class_data*) DECLSPEC_HIDDEN;
>
> -- 
> 2.32.0
> From f96c16d931c5c3840c40924ed2c9607af1dea3d2 Mon Sep 17 00:00:00 2001
> From: Connor McAdams 
> Date: Wed, 22 Sep 2021 16:12:36 -0400
> Subject: [PATCH 3/3] oleacc: Add get_accState function for edit client
>  accessible object.
> To: wine-devel
> Signed-off-by: Connor McAdams
>
> dlls/oleacc/client.c | 60 +++++++++++++++++++++++++++++++++-----------
>  1 file changed, 45 insertions(+), 15 deletions(-)
>
> diff --git a/dlls/oleacc/client.c b/dlls/oleacc/client.c
> index 4a0471a2657..12058f5545a 100644
> --- a/dlls/oleacc/client.c
> +++ b/dlls/oleacc/client.c
> @@ -18,6 +18,7 @@
>
> #define COBJMACROS
>
> +#include <assert.h>
>  #include "oleacc_private.h"
>  #include "commctrl.h"
>
> @@ -43,6 +44,7 @@ typedef struct {
>
> struct win_class_vtbl {
>      void (*init)(Client*);
> - HRESULT (*get_state)(Client*, VARIANT, VARIANT*);
>
> };
>
> static inline Client* impl_from_Client(IAccessible *iface)
> @@ -233,36 +235,44 @@ static HRESULT WINAPI Client_get_accRole(IAccessible *iface, VARIANT varID, VARI
>      return S_OK;
>  }
>
> -static HRESULT WINAPI Client_get_accState(IAccessible *iface, VARIANT varID, VARIANT *pvarState)
> +static HRESULT client_get_state(Client *client, VARIANT id, VARIANT *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;
> - if(convert_child_id(&id) != CHILDID_SELF) {
> -
>    V_VT(state) = VT_EMPTY;
>    return E_INVALIDARG;
> }
> - V_VT(pvarState) = VT_I4;
> - V_I4(pvarState) = 0;
> - V_VT(state) = VT_I4;
> - V_I4(state) = 0;
> - style = GetWindowLongW(This->hwnd, GWL_STYLE);
> - style = GetWindowLongW(client->hwnd, GWL_STYLE);
> if(style & WS_DISABLED)
> -
>    V_I4(pvarState) |= STATE_SYSTEM_UNAVAILABLE;
> - else if(IsWindow(This->hwnd))
> -
>    V_I4(pvarState) |= STATE_SYSTEM_FOCUSABLE;
> -
>    V_I4(state) |= STATE_SYSTEM_UNAVAILABLE;
> -
>
> else if(IsWindow(client->hwnd))
> -
>    V_I4(state) |= STATE_SYSTEM_FOCUSABLE;
>
> info.cbSize = sizeof(info);
> - if(GetGUIThreadInfo(0, &info) && info.hwndFocus == This->hwnd)
> -
>    V_I4(pvarState) |= STATE_SYSTEM_FOCUSED;
> - if(GetGUIThreadInfo(0, &info) && info.hwndFocus == client->hwnd)
> -
>    V_I4(state) |= STATE_SYSTEM_FOCUSED;
> if(!(style & WS_VISIBLE))
> -
>    V_I4(pvarState) |= STATE_SYSTEM_INVISIBLE;
> -
>    V_I4(state) |= STATE_SYSTEM_INVISIBLE;
> return S_OK;
>
> }
>
> +static HRESULT WINAPI Client_get_accState(IAccessible *iface, VARIANT id, VARIANT *state)
> +{
> - Client *This = impl_from_Client(iface);
> -
> - TRACE("(%p)->(%s %p)\n", This, debugstr_variant(&id), state);
> -
> - if(This->vtbl && This->vtbl->get_state)
> -
>    return This->vtbl->get_state(This, id, state);
> - return client_get_state(This, id, state);
>
> +}
> -
>
> static HRESULT WINAPI Client_get_accHelp(IAccessible *iface, VARIANT varID, BSTR *pszHelp)
>  {
>      Client *This = impl_from_Client(iface);
> @@ -664,8 +674,28 @@ static void edit_init(Client *client)
>      client->role = ROLE_SYSTEM_TEXT;
>  }
>
> +static HRESULT edit_get_state(Client *client, VARIANT id, VARIANT *state)
> +{
> - HRESULT hres;
> - LONG style;
> -
> - hres = client_get_state(client, id, state);
> - if(FAILED(hres))
> -
>    return hres;
> -
> - assert(V_VT(state) == VT_I4);
> -
> - style = GetWindowLongW(client->hwnd, GWL_STYLE);
> - if(style & ES_READONLY)
> -
>    V_I4(state) |= STATE_SYSTEM_READONLY;
> - if(style & ES_PASSWORD)
> -
>    V_I4(state) |= STATE_SYSTEM_PROTECTED;
> - return S_OK;
>
> +}
> -
>
> static const win_class_vtbl edit_vtbl = {
>      edit_init,
> - edit_get_state,
>
> };
> static const struct win_class_data classes[] = {
>
> 2.32.0