From b97db6303000f2a3c670862c0771069dfcca9972 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 2 Oct 2019 18:04:59 -0500 Subject: [PATCH] Prevent the v1 propsheet from zeroing colors, causing black text on black background. (#2651) * This fixes the registry path What's happening is the console is writing the Forcev2 setting, then the v1 console is ignoring those settings, then when you check the checkbox to save the v2 settings, we'll write the zeros out. That's obviously bad. So we'll only write the v2 settings back to the registry if the propsheet was launched from a v2 console. This does not fix the shortcut path. That'll be the next commit. * Fix the shortcut loading too fixes #2319 * remove the redundant property I added * add some notes to the bx.ps1 change --- src/propsheet/PropSheetHandler.cpp | 6 ++ src/propsheet/console.cpp | 5 +- src/propsheet/console.h | 2 + src/propsheet/globals.cpp | 3 + src/propsheet/registry.cpp | 93 ++++++++++++++------------ src/propslib/ShortcutSerialization.cpp | 38 +++++++---- src/propslib/ShortcutSerialization.hpp | 2 +- tools/bx.ps1 | 16 ++++- tools/opencon.cmd | 1 + 9 files changed, 106 insertions(+), 60 deletions(-) diff --git a/src/propsheet/PropSheetHandler.cpp b/src/propsheet/PropSheetHandler.cpp index d4b93c886..c6f2c5e41 100644 --- a/src/propsheet/PropSheetHandler.cpp +++ b/src/propsheet/PropSheetHandler.cpp @@ -94,6 +94,12 @@ private: { g_fHostedInFileProperties = TRUE; gpStateInfo = &g_csi; + + // Initialize the fIsV2Console with whatever the current v2 seeting is + // in the registry. Usually this is set by conhost, but in this path, + // we're being launched straight from explorer. See GH#2319, GH#2651 + gpStateInfo->fIsV2Console = GetConsoleBoolValue(CONSOLE_REGISTRY_FORCEV2, TRUE); + InitRegistryValues(gpStateInfo); gpStateInfo->Defaults = TRUE; GetRegistryValues(gpStateInfo); diff --git a/src/propsheet/console.cpp b/src/propsheet/console.cpp index cf3683dc8..6a4048751 100644 --- a/src/propsheet/console.cpp +++ b/src/propsheet/console.cpp @@ -97,7 +97,10 @@ void SaveConsoleSettingsIfNeeded(const HWND hwnd) if (gpStateInfo->LinkTitle != NULL) { SetGlobalRegistryValues(); - if (!NT_SUCCESS(ShortcutSerialization::s_SetLinkValues(gpStateInfo, g_fEastAsianSystem, g_fForceV2))) + if (!NT_SUCCESS(ShortcutSerialization::s_SetLinkValues(gpStateInfo, + g_fEastAsianSystem, + g_fForceV2, + gpStateInfo->fIsV2Console))) { WCHAR szMessage[MAX_PATH + 100]; WCHAR awchBuffer[MAX_PATH] = { 0 }; diff --git a/src/propsheet/console.h b/src/propsheet/console.h index 8094ce31d..7a98f78c2 100644 --- a/src/propsheet/console.h +++ b/src/propsheet/console.h @@ -225,3 +225,5 @@ const unsigned int TERMINAL_PAGE_INDEX = 4; // number of property sheet pages static const int V1_NUMBER_OF_PAGES = 4; static const int NUMBER_OF_PAGES = 5; + +BOOL GetConsoleBoolValue(__in PCWSTR pszValueName, __in BOOL fDefault); diff --git a/src/propsheet/globals.cpp b/src/propsheet/globals.cpp index b7c59171c..01de6df00 100644 --- a/src/propsheet/globals.cpp +++ b/src/propsheet/globals.cpp @@ -9,6 +9,9 @@ LONG gcxScreen; LONG gcyScreen; BOOL g_fForceV2; +// If we didn't launch as a v2 console window, we don't want to persist v2 +// settings when we close, as they'll get zero'd. Use this to track the initial +// launch state. BOOL g_fEditKeys; BYTE g_bPreviewOpacity = 0x00; //sentinel value for initial test on dialog entry. Once initialized, won't be less than TRANSPARENCY_RANGE_MIN diff --git a/src/propsheet/registry.cpp b/src/propsheet/registry.cpp index 2914ae099..5c7a501d1 100644 --- a/src/propsheet/registry.cpp +++ b/src/propsheet/registry.cpp @@ -946,52 +946,59 @@ VOID SetRegistryValues( SetGlobalRegistryValues(); - // Save cursor type and color - dwValue = pStateInfo->CursorType; - LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, - hTitleKey, - CONSOLE_REGISTRY_CURSORTYPE, - REG_DWORD, - (BYTE*)&dwValue, - sizeof(dwValue))); + // Only save the "Terminal" settings if we launched as a v2 propsheet. The + // v1 console doesn't know anything about these settings, and their value + // will be incorrectly zero'd if we save in this state. + // See microsoft/terminal#2319 for more details. + if (gpStateInfo->fIsV2Console) + { + // Save cursor type and color + dwValue = pStateInfo->CursorType; + LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, + hTitleKey, + CONSOLE_REGISTRY_CURSORTYPE, + REG_DWORD, + (BYTE*)&dwValue, + sizeof(dwValue))); - dwValue = pStateInfo->CursorColor; - LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, - hTitleKey, - CONSOLE_REGISTRY_CURSORCOLOR, - REG_DWORD, - (BYTE*)&dwValue, - sizeof(dwValue))); + dwValue = pStateInfo->CursorColor; + LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, + hTitleKey, + CONSOLE_REGISTRY_CURSORCOLOR, + REG_DWORD, + (BYTE*)&dwValue, + sizeof(dwValue))); - dwValue = pStateInfo->InterceptCopyPaste; - LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, - hTitleKey, - CONSOLE_REGISTRY_INTERCEPTCOPYPASTE, - REG_DWORD, - (BYTE*)&dwValue, - sizeof(dwValue))); + dwValue = pStateInfo->InterceptCopyPaste; + LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, + hTitleKey, + CONSOLE_REGISTRY_INTERCEPTCOPYPASTE, + REG_DWORD, + (BYTE*)&dwValue, + sizeof(dwValue))); - dwValue = pStateInfo->TerminalScrolling; - LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, - hTitleKey, - CONSOLE_REGISTRY_TERMINALSCROLLING, - REG_DWORD, - (BYTE*)&dwValue, - sizeof(dwValue))); - dwValue = pStateInfo->DefaultForeground; - LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, - hTitleKey, - CONSOLE_REGISTRY_DEFAULTFOREGROUND, - REG_DWORD, - (BYTE*)&dwValue, - sizeof(dwValue))); - dwValue = pStateInfo->DefaultBackground; - LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, - hTitleKey, - CONSOLE_REGISTRY_DEFAULTBACKGROUND, - REG_DWORD, - (BYTE*)&dwValue, - sizeof(dwValue))); + dwValue = pStateInfo->TerminalScrolling; + LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, + hTitleKey, + CONSOLE_REGISTRY_TERMINALSCROLLING, + REG_DWORD, + (BYTE*)&dwValue, + sizeof(dwValue))); + dwValue = pStateInfo->DefaultForeground; + LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, + hTitleKey, + CONSOLE_REGISTRY_DEFAULTFOREGROUND, + REG_DWORD, + (BYTE*)&dwValue, + sizeof(dwValue))); + dwValue = pStateInfo->DefaultBackground; + LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, + hTitleKey, + CONSOLE_REGISTRY_DEFAULTBACKGROUND, + REG_DWORD, + (BYTE*)&dwValue, + sizeof(dwValue))); + } // // Close the registry keys diff --git a/src/propslib/ShortcutSerialization.cpp b/src/propslib/ShortcutSerialization.cpp index a514de7c0..39d21cc1a 100644 --- a/src/propslib/ShortcutSerialization.cpp +++ b/src/propslib/ShortcutSerialization.cpp @@ -409,16 +409,19 @@ void ShortcutSerialization::s_GetLinkTitle(_In_ PCWSTR pwszShortcutFilename, return (SUCCEEDED(hr)) ? STATUS_SUCCESS : STATUS_UNSUCCESSFUL; } -/** -Writes the console properties out to the link it was opened from. -Arguments: - pStateInfo - pointer to structure containing information -Return Value: - A status code if something failed or S_OK -*/ +// Function Description: +// - Writes the console properties out to the link it was opened from. +// Arguments: +// - pStateInfo: pointer to structure containing information +// - writeTerminalSettings: If true, persist the "Terminal" properties only +// present in the v2 console. This should be false if called from a v11 +// console. See GH#2319 +// Return Value: +// - A status code if something failed or S_OK [[nodiscard]] NTSTATUS ShortcutSerialization::s_SetLinkValues(_In_ PCONSOLE_STATE_INFO pStateInfo, const BOOL fEastAsianSystem, - const BOOL fForceV2) + const BOOL fForceV2, + const bool writeTerminalSettings) { IShellLinkW* psl; IPersistFile* ppf; @@ -488,12 +491,21 @@ Return Value: s_SetLinkPropertyBoolValue(pps, PKEY_Console_CtrlKeyShortcutsDisabled, pStateInfo->fCtrlKeyShortcutsDisabled); s_SetLinkPropertyBoolValue(pps, PKEY_Console_LineSelection, pStateInfo->fLineSelection); s_SetLinkPropertyByteValue(pps, PKEY_Console_WindowTransparency, pStateInfo->bWindowTransparency); - s_SetLinkPropertyDwordValue(pps, PKEY_Console_CursorType, pStateInfo->CursorType); - s_SetLinkPropertyDwordValue(pps, PKEY_Console_CursorColor, pStateInfo->CursorColor); s_SetLinkPropertyBoolValue(pps, PKEY_Console_InterceptCopyPaste, pStateInfo->InterceptCopyPaste); - s_SetLinkPropertyDwordValue(pps, PKEY_Console_DefaultForeground, pStateInfo->DefaultForeground); - s_SetLinkPropertyDwordValue(pps, PKEY_Console_DefaultBackground, pStateInfo->DefaultBackground); - s_SetLinkPropertyBoolValue(pps, PKEY_Console_TerminalScrolling, pStateInfo->TerminalScrolling); + + // Only save the "Terminal" settings if we launched as a v2 + // propsheet. The v1 console doesn't know anything about + // these settings, and their value will be incorrectly + // zero'd if we save in this state. + // See microsoft/terminal#2319 for more details. + if (writeTerminalSettings) + { + s_SetLinkPropertyDwordValue(pps, PKEY_Console_CursorType, pStateInfo->CursorType); + s_SetLinkPropertyDwordValue(pps, PKEY_Console_CursorColor, pStateInfo->CursorColor); + s_SetLinkPropertyDwordValue(pps, PKEY_Console_DefaultForeground, pStateInfo->DefaultForeground); + s_SetLinkPropertyDwordValue(pps, PKEY_Console_DefaultBackground, pStateInfo->DefaultBackground); + s_SetLinkPropertyBoolValue(pps, PKEY_Console_TerminalScrolling, pStateInfo->TerminalScrolling); + } hr = pps->Commit(); pps->Release(); } diff --git a/src/propslib/ShortcutSerialization.hpp b/src/propslib/ShortcutSerialization.hpp index 27183b09c..a21e57386 100644 --- a/src/propslib/ShortcutSerialization.hpp +++ b/src/propslib/ShortcutSerialization.hpp @@ -24,7 +24,7 @@ Revision History: class ShortcutSerialization { public: - [[nodiscard]] static NTSTATUS s_SetLinkValues(_In_ PCONSOLE_STATE_INFO pStateInfo, const BOOL fEastAsianSystem, const BOOL fForceV2); + [[nodiscard]] static NTSTATUS s_SetLinkValues(_In_ PCONSOLE_STATE_INFO pStateInfo, const BOOL fEastAsianSystem, const BOOL fForceV2, const bool writeTerminalSettings); [[nodiscard]] static NTSTATUS s_GetLinkConsoleProperties(_Inout_ PCONSOLE_STATE_INFO pStateInfo); [[nodiscard]] static NTSTATUS s_GetLinkValues(_Inout_ PCONSOLE_STATE_INFO pStateInfo, _Out_ BOOL* const pfReadConsoleProperties, diff --git a/tools/bx.ps1 b/tools/bx.ps1 index 4332c7bb6..4bf0596a3 100644 --- a/tools/bx.ps1 +++ b/tools/bx.ps1 @@ -9,18 +9,30 @@ if ($projects.length -eq 0) } $projectPath = $projects.FullName -$msBuildCondition = "'%(ProjectReference.Identity)' == '$projectPath.metaproj'" # Parse the solution's metaproj file. [xml]$Metaproj = Get-Content "$env:OPENCON\OpenConsole.sln.metaproj" $targets = $Metaproj.Project.Target -# Filter to project targets that match out metaproj file. +# Most projects are in OpenConsole.sln.metaproj as ".*proj.metaproj". +# We'll filter to search for these first and foremost. +$msBuildCondition = "'%(ProjectReference.Identity)' == '$projectPath.metaproj'" + +# Filter to project targets that match our metaproj file. # For Conhost\Server, this will match: # [Conhost\Server, Conhost\Server:Clean, Conhost\Server:Rebuild, Conhost\Server:Publish] $matchingTargets = $targets | Where-Object { $_.MSBuild.Condition -eq $msBuildCondition } +# If we didn't find a target, it's possible that the project didn't have a +# .metaproj in OpenConsole.sln.metaproj. Try filtering again, but leave off the +# .metaproj extension. +if ($matchingTargets.length -eq 0) +{ + $conditionNoMeta = "'%(ProjectReference.Identity)' == '$projectPath'" + $matchingTargets = $targets | Where-Object { $_.MSBuild.Condition -eq $conditionNoMeta } +} + # Further filter to the targets that dont have a suffix (like ":Clean") $matchingTargets = $matchingTargets | Where-Object { $hasProperty = $_.MsBuild.PSobject.Properties.name -match "Targets" ; return -Not $hasProperty } diff --git a/tools/opencon.cmd b/tools/opencon.cmd index 40746db9b..172a37231 100644 --- a/tools/opencon.cmd +++ b/tools/opencon.cmd @@ -22,4 +22,5 @@ set copy_dir=OpenConsole\%_r% (xcopy /Y %_last_build%\Nihilist.exe %TEMP%\%copy_dir%\Nihilist.exe*) > nul (xcopy /Y %_last_build%\console.dll %TEMP%\%copy_dir%\console.dll*) > nul +echo Launching `%TEMP%\%copy_dir%\OpenConsole.exe %*`... start %TEMP%\%copy_dir%\OpenConsole.exe %*