[PATCH v2 0/1] MR2068: Draft: comdlg32: Enable visual styles when showing IFileDialog.
Required for !1857 to not break when comctl32 version 6 isn't requested by application (in particular, found this in [qapitrace](https://github.com/apitrace/apitrace)). Manifest resource id and assembly identity name match with Windows. For isolation purposes, activation context is disabled while emitting events. -- v2: comdlg32: Enable visual styles when showing IFileDialog. https://gitlab.winehq.org/wine/wine/-/merge_requests/2068
From: Vladislav Timonin <timoninvlad(a)yandex.ru> --- dlls/comdlg32/cdlg.h | 1 + dlls/comdlg32/cdlg32.c | 16 ++++++- dlls/comdlg32/comdlg32.manifest | 16 +++++++ dlls/comdlg32/comdlg32.rc | 3 ++ dlls/comdlg32/itemdlg.c | 84 +++++++++++++++++++++++++++++++++ 5 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 dlls/comdlg32/comdlg32.manifest diff --git a/dlls/comdlg32/cdlg.h b/dlls/comdlg32/cdlg.h index 1ddd157b880..e47d685fac3 100644 --- a/dlls/comdlg32/cdlg.h +++ b/dlls/comdlg32/cdlg.h @@ -27,6 +27,7 @@ #define COMDLG32_Atom MAKEINTATOM(0xa000) /* MS uses this one to identify props */ extern HINSTANCE COMDLG32_hInstance DECLSPEC_HIDDEN; +extern HANDLE COMDLG32_hActCtx DECLSPEC_HIDDEN; void COMDLG32_SetCommDlgExtendedError(DWORD err) DECLSPEC_HIDDEN; LPVOID COMDLG32_AllocMem(int size) __WINE_ALLOC_SIZE(1) DECLSPEC_HIDDEN; diff --git a/dlls/comdlg32/cdlg32.c b/dlls/comdlg32/cdlg32.c index f9619cc6a49..aebb3475674 100644 --- a/dlls/comdlg32/cdlg32.c +++ b/dlls/comdlg32/cdlg32.c @@ -40,6 +40,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(commdlg); DECLSPEC_HIDDEN HINSTANCE COMDLG32_hInstance = 0; +HANDLE COMDLG32_hActCtx = INVALID_HANDLE_VALUE; static DWORD COMDLG32_TlsIndex = TLS_OUT_OF_INDEXES; @@ -72,6 +73,9 @@ BOOL WINAPI DllMain(HINSTANCE hInstance, DWORD Reason, LPVOID Reserved) switch(Reason) { case DLL_PROCESS_ATTACH: + { + ACTCTXW actctx = {0}; + COMDLG32_hInstance = hInstance; DisableThreadLibraryCalls(hInstance); @@ -79,11 +83,21 @@ BOOL WINAPI DllMain(HINSTANCE hInstance, DWORD Reason, LPVOID Reserved) /* SHELL */ GPA(COMDLG32_SHSimpleIDListFromPathAW, SHELL32_hInstance, (LPCSTR)162); - break; + actctx.cbSize = sizeof(actctx); + actctx.hModule = COMDLG32_hInstance; + actctx.lpResourceName = MAKEINTRESOURCEW(123); + actctx.dwFlags = ACTCTX_FLAG_HMODULE_VALID | ACTCTX_FLAG_RESOURCE_NAME_VALID; + COMDLG32_hActCtx = CreateActCtxW(&actctx); + if (COMDLG32_hActCtx == INVALID_HANDLE_VALUE) + ERR("failed to create activation context, last error %lu\n", GetLastError()); + + break; + } case DLL_PROCESS_DETACH: if (Reserved) break; if (COMDLG32_TlsIndex != TLS_OUT_OF_INDEXES) TlsFree(COMDLG32_TlsIndex); + ReleaseActCtx(&COMDLG32_hActCtx); break; } return TRUE; diff --git a/dlls/comdlg32/comdlg32.manifest b/dlls/comdlg32/comdlg32.manifest new file mode 100644 index 00000000000..f9e7b7cc115 --- /dev/null +++ b/dlls/comdlg32/comdlg32.manifest @@ -0,0 +1,16 @@ +<?xml version="1.0" encoding="UTF-8" standalone="yes"?> +<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0"> +<assemblyIdentity type="win32" name="Microsoft.Windows.Shell.comdlg32" version="0.0.0.0"/> +<dependency> + <dependentAssembly> + <assemblyIdentity + type="win32" + name="Microsoft.Windows.Common-Controls" + version="6.0.0.0" + processorArchitecture="*" + publicKeyToken="6595b64144ccf1df" + language="*" + /> + </dependentAssembly> +</dependency> +</assembly> diff --git a/dlls/comdlg32/comdlg32.rc b/dlls/comdlg32/comdlg32.rc index a5d09faa563..474bafd8760 100644 --- a/dlls/comdlg32/comdlg32.rc +++ b/dlls/comdlg32/comdlg32.rc @@ -602,3 +602,6 @@ NETWORK ICON network.ico /* @makedep: fontpics.bmp */ 38 BITMAP fontpics.bmp + +/* @makedep: comdlg32.manifest */ +123 RT_MANIFEST comdlg32.manifest diff --git a/dlls/comdlg32/itemdlg.c b/dlls/comdlg32/itemdlg.c index 95778c2f358..1bb77fa430c 100644 --- a/dlls/comdlg32/itemdlg.c +++ b/dlls/comdlg32/itemdlg.c @@ -145,6 +145,8 @@ typedef struct FileDialogImpl { DWORD opendropdown_selection; GUID client_guid; + + HANDLE user_actctx; } FileDialogImpl; /************************************************************************** @@ -152,10 +154,14 @@ typedef struct FileDialogImpl { */ static HRESULT events_OnFileOk(FileDialogImpl *This) { + ULONG_PTR ctx_cookie; events_client *cursor; HRESULT hr = S_OK; TRACE("%p\n", This); + if (This->user_actctx != INVALID_HANDLE_VALUE) + ActivateActCtx(This->user_actctx, &ctx_cookie); + LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry) { TRACE("Notifying %p\n", cursor); @@ -164,6 +170,9 @@ static HRESULT events_OnFileOk(FileDialogImpl *This) break; } + if (This->user_actctx != INVALID_HANDLE_VALUE) + DeactivateActCtx(0, ctx_cookie); + if(hr == E_NOTIMPL) hr = S_OK; @@ -172,10 +181,14 @@ static HRESULT events_OnFileOk(FileDialogImpl *This) static HRESULT events_OnFolderChanging(FileDialogImpl *This, IShellItem *folder) { + ULONG_PTR ctx_cookie; events_client *cursor; HRESULT hr = S_OK; TRACE("%p (%p)\n", This, folder); + if (This->user_actctx != INVALID_HANDLE_VALUE) + ActivateActCtx(This->user_actctx, &ctx_cookie); + LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry) { TRACE("Notifying %p\n", cursor); @@ -184,6 +197,9 @@ static HRESULT events_OnFolderChanging(FileDialogImpl *This, IShellItem *folder) break; } + if (This->user_actctx != INVALID_HANDLE_VALUE) + DeactivateActCtx(0, ctx_cookie); + if(hr == E_NOTIMPL) hr = S_OK; @@ -192,47 +208,72 @@ static HRESULT events_OnFolderChanging(FileDialogImpl *This, IShellItem *folder) static void events_OnFolderChange(FileDialogImpl *This) { + ULONG_PTR ctx_cookie; events_client *cursor; TRACE("%p\n", This); + if (This->user_actctx != INVALID_HANDLE_VALUE) + ActivateActCtx(This->user_actctx, &ctx_cookie); + LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry) { TRACE("Notifying %p\n", cursor); IFileDialogEvents_OnFolderChange(cursor->pfde, (IFileDialog*)&This->IFileDialog2_iface); } + + if (This->user_actctx != INVALID_HANDLE_VALUE) + DeactivateActCtx(0, ctx_cookie); } static void events_OnSelectionChange(FileDialogImpl *This) { + ULONG_PTR ctx_cookie; events_client *cursor; TRACE("%p\n", This); + if (This->user_actctx != INVALID_HANDLE_VALUE) + ActivateActCtx(This->user_actctx, &ctx_cookie); + LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry) { TRACE("Notifying %p\n", cursor); IFileDialogEvents_OnSelectionChange(cursor->pfde, (IFileDialog*)&This->IFileDialog2_iface); } + + if (This->user_actctx != INVALID_HANDLE_VALUE) + DeactivateActCtx(0, ctx_cookie); } static void events_OnTypeChange(FileDialogImpl *This) { + ULONG_PTR ctx_cookie; events_client *cursor; TRACE("%p\n", This); + if (This->user_actctx != INVALID_HANDLE_VALUE) + ActivateActCtx(This->user_actctx, &ctx_cookie); + LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry) { TRACE("Notifying %p\n", cursor); IFileDialogEvents_OnTypeChange(cursor->pfde, (IFileDialog*)&This->IFileDialog2_iface); } + + if (This->user_actctx != INVALID_HANDLE_VALUE) + DeactivateActCtx(0, ctx_cookie); } static HRESULT events_OnOverwrite(FileDialogImpl *This, IShellItem *shellitem) { + ULONG_PTR ctx_cookie; events_client *cursor; HRESULT hr = S_OK; FDE_OVERWRITE_RESPONSE response = FDEOR_DEFAULT; TRACE("%p %p\n", This, shellitem); + if (This->user_actctx != INVALID_HANDLE_VALUE) + ActivateActCtx(This->user_actctx, &ctx_cookie); + LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry) { TRACE("Notifying %p\n", cursor); @@ -242,6 +283,9 @@ static HRESULT events_OnOverwrite(FileDialogImpl *This, IShellItem *shellitem) break; } + if (This->user_actctx != INVALID_HANDLE_VALUE) + DeactivateActCtx(0, ctx_cookie); + if(hr == E_NOTIMPL) hr = S_OK; @@ -274,9 +318,13 @@ static inline HRESULT get_cctrl_event(IFileDialogEvents *pfde, IFileDialogContro static HRESULT cctrl_event_OnButtonClicked(FileDialogImpl *This, DWORD ctl_id) { + ULONG_PTR ctx_cookie; events_client *cursor; TRACE("%p\n", This); + if (This->user_actctx != INVALID_HANDLE_VALUE) + ActivateActCtx(This->user_actctx, &ctx_cookie); + LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry) { IFileDialogControlEvents *pfdce; @@ -288,14 +336,21 @@ static HRESULT cctrl_event_OnButtonClicked(FileDialogImpl *This, DWORD ctl_id) } } + if (This->user_actctx != INVALID_HANDLE_VALUE) + DeactivateActCtx(0, ctx_cookie); + return S_OK; } static HRESULT cctrl_event_OnItemSelected(FileDialogImpl *This, DWORD ctl_id, DWORD item_id) { + ULONG_PTR ctx_cookie; events_client *cursor; TRACE("%p %li %li\n", This, ctl_id, item_id); + if (This->user_actctx != INVALID_HANDLE_VALUE) + ActivateActCtx(This->user_actctx, &ctx_cookie); + LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry) { IFileDialogControlEvents *pfdce; @@ -307,14 +362,21 @@ static HRESULT cctrl_event_OnItemSelected(FileDialogImpl *This, DWORD ctl_id, DW } } + if (This->user_actctx != INVALID_HANDLE_VALUE) + DeactivateActCtx(0, ctx_cookie); + return S_OK; } static HRESULT cctrl_event_OnCheckButtonToggled(FileDialogImpl *This, DWORD ctl_id, BOOL checked) { + ULONG_PTR ctx_cookie; events_client *cursor; TRACE("%p\n", This); + if (This->user_actctx != INVALID_HANDLE_VALUE) + ActivateActCtx(This->user_actctx, &ctx_cookie); + LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry) { IFileDialogControlEvents *pfdce; @@ -326,15 +388,22 @@ static HRESULT cctrl_event_OnCheckButtonToggled(FileDialogImpl *This, DWORD ctl_ } } + if (This->user_actctx != INVALID_HANDLE_VALUE) + DeactivateActCtx(0, ctx_cookie); + return S_OK; } static HRESULT cctrl_event_OnControlActivating(FileDialogImpl *This, DWORD ctl_id) { + ULONG_PTR ctx_cookie; events_client *cursor; TRACE("%p\n", This); + if (This->user_actctx != INVALID_HANDLE_VALUE) + ActivateActCtx(This->user_actctx, &ctx_cookie); + LIST_FOR_EACH_ENTRY(cursor, &This->events_clients, events_client, entry) { IFileDialogControlEvents *pfdce; @@ -346,6 +415,9 @@ static HRESULT cctrl_event_OnControlActivating(FileDialogImpl *This, } } + if (This->user_actctx != INVALID_HANDLE_VALUE) + DeactivateActCtx(0, ctx_cookie); + return S_OK; } @@ -2243,12 +2315,22 @@ static INT_PTR CALLBACK itemdlg_dlgproc(HWND hwnd, UINT umessage, WPARAM wparam, static HRESULT create_dialog(FileDialogImpl *This, HWND parent) { INT_PTR res; + ULONG_PTR actctx_cookie = 0; + + GetCurrentActCtx(&This->user_actctx); + if (!ActivateActCtx(COMDLG32_hActCtx, &actctx_cookie)) + ERR("failed to activate activation context, last error %lx\n", GetLastError()); SetLastError(0); res = DialogBoxParamW(COMDLG32_hInstance, MAKEINTRESOURCEW(NEWFILEOPENV3ORD), parent, itemdlg_dlgproc, (LPARAM)This); This->dlg_hwnd = NULL; + + if (actctx_cookie != 0) + if (!DeactivateActCtx(0, actctx_cookie)) + ERR("failed to deactivate activation context, last error %lx\n", GetLastError()); + if(res == -1) { ERR("Failed to show dialog (LastError: %ld)\n", GetLastError()); @@ -4642,6 +4724,8 @@ static HRESULT FileDialog_constructor(IUnknown *pUnkOuter, REFIID riid, void **p return E_FAIL; } + fdimpl->user_actctx = INVALID_HANDLE_VALUE; + hr = IFileDialog2_QueryInterface(&fdimpl->IFileDialog2_iface, riid, ppv); IFileDialog2_Release(&fdimpl->IFileDialog2_iface); return hr; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2068
Jinoh Kang (@iamahuman) commented about dlls/comdlg32/itemdlg.c:
static HRESULT create_dialog(FileDialogImpl *This, HWND parent) { INT_PTR res; + ULONG_PTR actctx_cookie = 0; + + GetCurrentActCtx(&This->user_actctx); + if (!ActivateActCtx(COMDLG32_hActCtx, &actctx_cookie)) + ERR("failed to activate activation context, last error %lx\n", GetLastError());
SetLastError(0); res = DialogBoxParamW(COMDLG32_hInstance, MAKEINTRESOURCEW(NEWFILEOPENV3ORD), parent, itemdlg_dlgproc, (LPARAM)This); This->dlg_hwnd = NULL; + + if (actctx_cookie != 0) You shouldn't rely on `0` being an invalid cookie value.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2068#note_22479
Jinoh Kang (@iamahuman) commented about dlls/comdlg32/itemdlg.c:
static HRESULT create_dialog(FileDialogImpl *This, HWND parent) { INT_PTR res; + ULONG_PTR actctx_cookie = 0; + + GetCurrentActCtx(&This->user_actctx); In case of nested `Show()` calls, you should either:
1. Fail with `E_UNEXPECTED`, or 2. Backup the previous `user_actctx` value before calling `GetCurrentActCtx`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2068#note_22480
Jinoh Kang (@iamahuman) commented about dlls/comdlg32/itemdlg.c:
break; }
+ if (This->user_actctx != INVALID_HANDLE_VALUE) + DeactivateActCtx(0, ctx_cookie);
You should only deactivate if `ActivateActCtx` succeeded. I suggest writing activation/deactivation helpers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2068#note_22481
participants (3)
-
Jinoh Kang (＠iamahuman) -
Vladislav Timonin
-
Vladislav Timonin (＠vt)