Skip to content

Commit

Permalink
Prevent the v1 propsheet from zeroing colors, causing black text on b…
Browse files Browse the repository at this point in the history 
…lack 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
  • Loading branch information
@zadjii-msft @carlos-zamora
zadjii-msft authored and carlos-zamora committed Oct 2, 2019
1 parent 64c98db commit b97db63
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 60 deletions.
6 changes: 6 additions & 0 deletions src/propsheet/PropSheetHandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ class ConsolePropertySheetHandler WrlFinal : public RuntimeClass<RuntimeClassFla
{
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);
Expand Down
5 changes: 4 additions & 1 deletion src/propsheet/console.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
2 changes: 2 additions & 0 deletions src/propsheet/console.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
3 changes: 3 additions & 0 deletions src/propsheet/globals.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
93 changes: 50 additions & 43 deletions src/propsheet/registry.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 25 additions & 13 deletions src/propslib/ShortcutSerialization.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down
2 changes: 1 addition & 1 deletion src/propslib/ShortcutSerialization.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 14 additions & 2 deletions tools/bx.ps1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<project>.*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 }

Expand Down
1 change: 1 addition & 0 deletions tools/opencon.cmd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 %*

0 comments on commit b97db63

Please sign in to comment.