From 141a687ddd37b1120703285a11207f1259b6ff4b Mon Sep 17 00:00:00 2001 From: Selva Nair Date: Sat, 7 Jan 2023 23:11:11 -0500 Subject: [PATCH] Set status window as the owner of modal dialogs in same thread For all dialogs in a thread, set its status window in the same thread as the owner. Also set the owner of message boxes appropriately instead of using NULL. This has the side effect of some of the modal message popups blocking access to the status Window until dismissed. Next: Replace Sleep by a wait that pumps messages. Signed-off-by: Selva Nair --- localization.c | 9 +++++++-- localization.h | 1 + main.c | 2 +- misc.c | 4 ++-- openvpn.c | 46 ++++++++++++++++++++++++---------------------- options.c | 8 ++++---- pkcs11.c | 2 +- proxy.c | 2 +- scripts.c | 6 +++--- 9 files changed, 44 insertions(+), 36 deletions(-) diff --git a/localization.c b/localization.c index 8db12ae..50fb1ec 100644 --- a/localization.c +++ b/localization.c @@ -379,15 +379,20 @@ LocalizedDialogResource(const UINT dialogId) INT_PTR LocalizedDialogBoxParam(const UINT dialogId, DLGPROC dialogFunc, const LPARAM param) +{ + return LocalizedDialogBoxParamEx(dialogId, o.hWnd, dialogFunc, param); +} + +INT_PTR +LocalizedDialogBoxParamEx(const UINT dialogId, HWND owner, DLGPROC dialogFunc, const LPARAM param) { LPCDLGTEMPLATE resInfo = LocalizedDialogResource(dialogId); if (resInfo == NULL) return -1; - return DialogBoxIndirectParam(o.hInstance, resInfo, o.hWnd, dialogFunc, param); + return DialogBoxIndirectParam(o.hInstance, resInfo, owner, dialogFunc, param); } - HWND CreateLocalizedDialogParam(const UINT dialogId, DLGPROC dialogFunc, const LPARAM param) { diff --git a/localization.h b/localization.h index 0e44ac1..b83b47c 100644 --- a/localization.h +++ b/localization.h @@ -33,6 +33,7 @@ HICON LoadLocalizedIcon(const UINT); HICON LoadLocalizedSmallIcon(const UINT); LPCDLGTEMPLATE LocalizedDialogResource(const UINT); INT_PTR LocalizedDialogBoxParam(const UINT, DLGPROC, const LPARAM); +INT_PTR LocalizedDialogBoxParamEx(const UINT, HWND parent, DLGPROC, const LPARAM); HWND CreateLocalizedDialogParam(const UINT, DLGPROC, const LPARAM); HWND CreateLocalizedDialog(const UINT, DLGPROC); INT_PTR CALLBACK GeneralSettingsDlgProc(HWND, UINT, WPARAM, LPARAM); diff --git a/main.c b/main.c index bce2596..61e593d 100644 --- a/main.c +++ b/main.c @@ -856,7 +856,7 @@ CloseApplication(HWND hwnd, BOOL ask_user) } /* Ask for confirmation if still connected */ - if (ShowLocalizedMsgEx(MB_YESNO, NULL, _T("Exit OpenVPN"), IDS_NFO_ACTIVE_CONN_EXIT) == IDNO) + if (ShowLocalizedMsgEx(MB_YESNO|MB_TOPMOST, o.hWnd, _T("Exit OpenVPN"), IDS_NFO_ACTIVE_CONN_EXIT) == IDNO) { return; } diff --git a/misc.c b/misc.c index 733705d..2982a53 100644 --- a/misc.c +++ b/misc.c @@ -712,7 +712,7 @@ ImportConfigFile(const TCHAR* source, bool prompt_user) if (c && wcsnicmp(c->config_dir, o.config_dir, wcslen(o.config_dir)) == 0) { /* Ask the user whether to replace the profile or not. */ - if (ShowLocalizedMsgEx(MB_YESNO, NULL, _T(PACKAGE_NAME), IDS_NFO_IMPORT_OVERWRITE, fileName) == IDNO) + if (ShowLocalizedMsgEx(MB_YESNO|MB_TOPMOST, o.hWnd, _T(PACKAGE_NAME), IDS_NFO_IMPORT_OVERWRITE, fileName) == IDNO) { return; } @@ -722,7 +722,7 @@ ImportConfigFile(const TCHAR* source, bool prompt_user) else { if (prompt_user - && ShowLocalizedMsgEx(MB_YESNO, NULL, TEXT(PACKAGE_NAME), + && ShowLocalizedMsgEx(MB_YESNO|MB_TOPMOST, o.hWnd, TEXT(PACKAGE_NAME), IDS_NFO_IMPORT_SOURCE, fileName) == IDNO) { return; diff --git a/openvpn.c b/openvpn.c index edce159..afdb121 100644 --- a/openvpn.c +++ b/openvpn.c @@ -1248,7 +1248,7 @@ OnPassword(connection_t *c, char *msg) free_auth_param (param); return; } - LocalizedDialogBoxParam(ID_DLG_CHALLENGE_RESPONSE, GenericPassDialogFunc, (LPARAM) param); + LocalizedDialogBoxParamEx(ID_DLG_CHALLENGE_RESPONSE, c->hwndStatus, GenericPassDialogFunc, (LPARAM) param); free_dynamic_cr (c); } else if ( (chstr = strstr(msg, "SC:")) && strlen (chstr) > 5) @@ -1256,16 +1256,16 @@ OnPassword(connection_t *c, char *msg) param->flags |= FLAG_CR_TYPE_SCRV1; param->flags |= (*(chstr + 3) != '0') ? FLAG_CR_ECHO : 0; param->str = strdup(chstr + 5); - LocalizedDialogBoxParam(ID_DLG_AUTH_CHALLENGE, UserAuthDialogFunc, (LPARAM) param); + LocalizedDialogBoxParamEx(ID_DLG_AUTH_CHALLENGE, c->hwndStatus, UserAuthDialogFunc, (LPARAM) param); } else { - LocalizedDialogBoxParam(ID_DLG_AUTH, UserAuthDialogFunc, (LPARAM) param); + LocalizedDialogBoxParamEx(ID_DLG_AUTH, c->hwndStatus, UserAuthDialogFunc, (LPARAM) param); } } else if (strstr(msg, "'Private Key'")) { - LocalizedDialogBoxParam(ID_DLG_PASSPHRASE, PrivKeyPassDialogFunc, (LPARAM) c); + LocalizedDialogBoxParamEx(ID_DLG_PASSPHRASE, c->hwndStatus, PrivKeyPassDialogFunc, (LPARAM) c); } else if (strstr(msg, "'HTTP Proxy'")) { @@ -1291,7 +1291,7 @@ OnPassword(connection_t *c, char *msg) free_auth_param(param); return; } - LocalizedDialogBoxParam(ID_DLG_CHALLENGE_RESPONSE, GenericPassDialogFunc, (LPARAM) param); + LocalizedDialogBoxParamEx(ID_DLG_CHALLENGE_RESPONSE, c->hwndStatus, GenericPassDialogFunc, (LPARAM) param); } } @@ -1315,7 +1315,7 @@ OnTimeout(connection_t *c, UNUSED char *msg) c->state = connecting; if (!OpenManagement(c)) { - MessageBoxExW(NULL, L"Failed to open management", _T(PACKAGE_NAME), + MessageBoxExW(c->hwndStatus, L"Failed to open management", _T(PACKAGE_NAME), MB_OK | MB_SETFOREGROUND | MB_ICONERROR | MBOX_RTL_FLAGS, GetGUILanguage()); StopOpenVPN(c); } @@ -1511,7 +1511,7 @@ void OnInfoMsg(connection_t* c, char* msg) free_auth_param(param); return; } - LocalizedDialogBoxParam(ID_DLG_CHALLENGE_RESPONSE, GenericPassDialogFunc, (LPARAM)param); + LocalizedDialogBoxParamEx(ID_DLG_CHALLENGE_RESPONSE, c->hwndStatus, GenericPassDialogFunc, (LPARAM)param); } } @@ -1840,7 +1840,7 @@ OnNeedOk (connection_t *c, char *msg) } const char *fmt; - if (MessageBoxExW(NULL, wstr, L""PACKAGE_NAME, MB_OKCANCEL | MBOX_RTL_FLAGS, GetGUILanguage()) == IDOK) + if (MessageBoxExW(c->hwndStatus, wstr, L""PACKAGE_NAME, MB_OKCANCEL | MBOX_RTL_FLAGS, GetGUILanguage()) == IDOK) { fmt = "needok \'%s\' ok"; } @@ -1983,7 +1983,9 @@ StatusDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) 20, 25, 350, 160, hwndDlg, (HMENU) ID_EDT_LOG, o.hInstance, NULL); if (!hLogWnd) { - ShowLocalizedMsg(IDS_ERR_CREATE_EDIT_LOGWINDOW); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, c->hwndStatus, TEXT(PACKAGE_NAME), IDS_ERR_CREATE_EDIT_LOGWINDOW); + /* We can't continue without a log window */ + StopOpenVPN(c); return FALSE; } @@ -1996,7 +1998,7 @@ StatusDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) .yHeight = 160 }; if (SendMessage(hLogWnd, EM_SETCHARFORMAT, SCF_DEFAULT, (LPARAM) &cfm) == 0) - ShowLocalizedMsg(IDS_ERR_SET_SIZE); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, c->hwndStatus, TEXT(PACKAGE_NAME), IDS_ERR_SET_SIZE); /* display version string as "OpenVPN GUI gui_version/core_version" */ wchar_t version[256]; @@ -2207,7 +2209,7 @@ ThreadOpenVPNStatus(void *p) if (!OpenManagement(c)) { - MessageBoxExW(NULL, L"Failed to open management", _T(PACKAGE_NAME), + MessageBoxExW(c->hwndStatus, L"Failed to open management", _T(PACKAGE_NAME), MB_OK | MB_SETFOREGROUND | MB_ICONERROR | MBOX_RTL_FLAGS, GetGUILanguage()); StopOpenVPN(c); } @@ -2297,7 +2299,7 @@ SetProcessPriority(DWORD *priority) *priority = HIGH_PRIORITY_CLASS; else { - ShowLocalizedMsg(IDS_ERR_UNKNOWN_PRIORITY, o.priority_string); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, o.hWnd, TEXT(PACKAGE_NAME), IDS_ERR_UNKNOWN_PRIORITY, o.priority_string); return FALSE; } return TRUE; @@ -2402,7 +2404,7 @@ StartOpenVPN(connection_t *c) HANDLE hThread = CreateThread(NULL, 0, ThreadOpenVPNStatus, c, CREATE_SUSPENDED, &c->threadId); if (hThread == NULL) { - ShowLocalizedMsg(IDS_ERR_CREATE_THREAD_STATUS); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, o.hWnd, TEXT(PACKAGE_NAME), IDS_ERR_CREATE_THREAD_STATUS); return false; } @@ -2465,7 +2467,7 @@ LaunchOpenVPN(connection_t *c) c->exit_event = CreateEvent(NULL, TRUE, FALSE, exit_event_name); if (c->exit_event == NULL) { - ShowLocalizedMsg(IDS_ERR_CREATE_EVENT, exit_event_name); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, o.hWnd, TEXT(PACKAGE_NAME), IDS_ERR_CREATE_EVENT, exit_event_name); goto out; } @@ -2527,7 +2529,7 @@ LaunchOpenVPN(connection_t *c) if (!res) { - ShowLocalizedMsg (IDS_ERR_WRITE_SERVICE_PIPE); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, o.hWnd, TEXT(PACKAGE_NAME), IDS_ERR_WRITE_SERVICE_PIPE); CloseHandle(c->exit_event); CloseServiceIO(&c->iserv); goto out; @@ -2536,7 +2538,7 @@ LaunchOpenVPN(connection_t *c) #ifdef ENABLE_OVPN3 else if (o.ovpn_engine == OPENVPN_ENGINE_OVPN3) { - ShowLocalizedMsg(IDS_ERR_WRITE_SERVICE_PIPE); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, o.hWnd, TEXT(PACKAGE_NAME), IDS_ERR_WRITE_SERVICE_PIPE); CloseHandle(c->exit_event); CloseServiceIO(&c->iserv); goto out; @@ -2559,13 +2561,13 @@ LaunchOpenVPN(connection_t *c) if (!InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION)) { - ShowLocalizedMsg(IDS_ERR_INIT_SEC_DESC); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, o.hWnd, TEXT(PACKAGE_NAME), IDS_ERR_INIT_SEC_DESC); CloseHandle(c->exit_event); return FALSE; } if (!SetSecurityDescriptorDacl(&sd, TRUE, NULL, FALSE)) { - ShowLocalizedMsg(IDS_ERR_SET_SEC_DESC_ACL); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, o.hWnd, TEXT(PACKAGE_NAME), IDS_ERR_SET_SEC_DESC_ACL); CloseHandle(c->exit_event); return FALSE; } @@ -2588,13 +2590,13 @@ LaunchOpenVPN(connection_t *c) /* Create the pipe for STDIN with only the read end inheritable */ if (!CreatePipe(&hStdInRead, &hStdInWrite, &sa, 0)) { - ShowLocalizedMsg(IDS_ERR_CREATE_PIPE_IN_READ); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, o.hWnd, TEXT(PACKAGE_NAME), IDS_ERR_CREATE_PIPE_IN_READ); CloseHandle(c->exit_event); goto out; } if (!SetHandleInformation(hStdInWrite, HANDLE_FLAG_INHERIT, 0)) { - ShowLocalizedMsg(IDS_ERR_DUP_HANDLE_IN_WRITE); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, o.hWnd, TEXT(PACKAGE_NAME), IDS_ERR_DUP_HANDLE_IN_WRITE); CloseHandle(c->exit_event); goto out; } @@ -2611,7 +2613,7 @@ LaunchOpenVPN(connection_t *c) if (!CreateProcess(o.exe_path, cmdline, NULL, NULL, TRUE, priority | CREATE_NO_WINDOW, NULL, c->config_dir, &si, &pi)) { - ShowLocalizedMsg(IDS_ERR_CREATE_PROCESS, o.exe_path, cmdline, c->config_dir); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, o.hWnd, TEXT(PACKAGE_NAME), IDS_ERR_CREATE_PROCESS, o.exe_path, cmdline, c->config_dir); CloseHandle(c->exit_event); goto out; } @@ -2865,7 +2867,7 @@ out: void ResetSavePasswords(connection_t *c) { - if (ShowLocalizedMsgEx(MB_OKCANCEL, NULL, TEXT(PACKAGE_NAME), IDS_NFO_DELETE_PASS, c->config_name) == IDCANCEL) + if (ShowLocalizedMsgEx(MB_OKCANCEL, o.hWnd, TEXT(PACKAGE_NAME), IDS_NFO_DELETE_PASS, c->config_name) == IDCANCEL) return; DeleteSavedPasswords(c->config_name); c->flags &= ~(FLAG_SAVE_KEY_PASS | FLAG_SAVE_AUTH_PASS); diff --git a/options.c b/options.c index 94e2581..690e1be 100644 --- a/options.c +++ b/options.c @@ -578,7 +578,7 @@ CheckAdvancedDlgParams (HWND hdlg) ExpandString (tmp_path, _countof(tmp_path)); if (PathIsRelativeW (tmp_path)) { - MessageBox (NULL, L"Specified config directory is not an absolute path", + MessageBox (hdlg, L"Specified config directory is not an absolute path", L"Option error", MB_OK); return false; } @@ -587,7 +587,7 @@ CheckAdvancedDlgParams (HWND hdlg) ExpandString (tmp_path, _countof(tmp_path)); if (PathIsRelativeW (tmp_path)) { - MessageBox (NULL, L"Specified log directory is not an absolute path", + MessageBox (hdlg, L"Specified log directory is not an absolute path", L"Option error", MB_OK); return false; } @@ -597,7 +597,7 @@ CheckAdvancedDlgParams (HWND hdlg) /* Restrict the port offset to sensible range -- port used is this + upto ~4000 as connection index */ if (!status || (tmp < 1 || tmp > 61000)) { - MessageBox (NULL, L"Specified port is not valid (must be in the range 1 to 61000)", + MessageBox (hdlg, L"Specified port is not valid (must be in the range 1 to 61000)", L"Option error", MB_OK); return false; } @@ -605,7 +605,7 @@ CheckAdvancedDlgParams (HWND hdlg) tmp = GetDlgItemInt (hdlg, ID_EDT_POPUP_MUTE, &status, FALSE); if (!status || tmp < 0) { - MessageBox (NULL, L"Specified mute interval is not valid (must be a positive integer)", + MessageBox (hdlg, L"Specified mute interval is not valid (must be a positive integer)", L"Option error", MB_OK); return false; } diff --git a/pkcs11.c b/pkcs11.c index 426fda2..3e0d587 100644 --- a/pkcs11.c +++ b/pkcs11.c @@ -257,7 +257,7 @@ OnPkcs11(connection_t *c, UNUSED char *msg) l->selected = (UINT) -1; /* set selection to an invalid index */ /* prompt user to select a certificate */ - if (IDOK == LocalizedDialogBoxParam(ID_DLG_PKCS11_QUERY, QueryPkcs11DialogProc, (LPARAM)c) + if (IDOK == LocalizedDialogBoxParamEx(ID_DLG_PKCS11_QUERY, c->hwndStatus, QueryPkcs11DialogProc, (LPARAM)c) && l->state & STATE_SELECTED && l->selected < l->count) { diff --git a/proxy.c b/proxy.c index 9c0930f..f3ac1c0 100644 --- a/proxy.c +++ b/proxy.c @@ -390,7 +390,7 @@ void QueryProxyAuth(connection_t *c, proxy_t type) { c->proxy_type = type; - LocalizedDialogBoxParam(ID_DLG_PROXY_AUTH, ProxyAuthDialogFunc, (LPARAM) c); + LocalizedDialogBoxParamEx(ID_DLG_PROXY_AUTH, c->hwndStatus, ProxyAuthDialogFunc, (LPARAM) c); } diff --git a/scripts.c b/scripts.c index 47647b6..c3a94d9 100644 --- a/scripts.c +++ b/scripts.c @@ -168,7 +168,7 @@ RunConnectScript(connection_t *c, int run_as_service) env, c->config_dir, &si, &pi)) { PrintDebug(L"CreateProcess: error = %lu", GetLastError()); - ShowLocalizedMsg(IDS_ERR_RUN_CONN_SCRIPT, cmdline); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, c->hwndStatus, TEXT(PACKAGE_NAME), IDS_ERR_RUN_CONN_SCRIPT, cmdline); free(env); return; } @@ -187,14 +187,14 @@ RunConnectScript(connection_t *c, int run_as_service) if (exit_code != STILL_ACTIVE) { if (exit_code != 0) - ShowLocalizedMsg(IDS_ERR_CONN_SCRIPT_FAILED, exit_code); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, c->hwndStatus, TEXT(PACKAGE_NAME), IDS_ERR_CONN_SCRIPT_FAILED, exit_code); goto out; } Sleep(1000); } - ShowLocalizedMsg(IDS_ERR_RUN_CONN_SCRIPT_TIMEOUT, o.connectscript_timeout); + ShowLocalizedMsgEx(MB_OK|MB_ICONERROR, c->hwndStatus, TEXT(PACKAGE_NAME), IDS_ERR_RUN_CONN_SCRIPT_TIMEOUT, o.connectscript_timeout); out: free(env);