Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Legacy mode + shortcut propsheet may reset Default Terminal setting #17053

Open
DHowett opened this issue Apr 12, 2024 · 2 comments
Open

Legacy mode + shortcut propsheet may reset Default Terminal setting #17053

DHowett opened this issue Apr 12, 2024 · 2 comments
Labels
Area-DefApp Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conhost For issues in the Console codebase zInbox-Bug Ignore me!
Milestone

Comments

@DHowett
Copy link
Member

DHowett commented Apr 12, 2024

While messing around on Windows 10 I came across an 'automatic' reset of the Let Windows Decide setting. I don't know if this relates to what triggered the initial report. Tested on Win10 21H2 and 22H2 (the latter with or without Windows Terminal installed). Don't have a Win11 system to test on. I have a console-using application with a Desktop shortcut. Right clicking the shortcut and selecting Properties shows a Properties dialog with all of the Console specific tabs. The Options tab has the option 'Use legacy console'. To reproduce:

1 - open Settings/developers and set default terminal option to Windows Console Host.
2 - close the settings window (reopening and checking verifies it has stayed 'set')
3 - Access the Properties menu by right clicking on the shortcut
4 - Navigate to Options, and toggle (either select or deselect) the use Legacy console option.
5 - close the Properties dialog.
6 - reopen developer Settings. Note that terminal option has been reset to Let Windows Decide.

Repeated this on systems with and without Windows Terminal installed. On windows 10 it seems 'let windows decide' still reverts to conhost.exe. If this also happens on win11, I believe it's expected to chose Windows Terminal. So it seems this could cause users to have their expected terminal switched unknowingly. Note that if I go to the properties menu within the open application to set the Legacy property, this change does not happen. I don't know what's different about accessing the properties from a shortcut vs from within an open application.

Originally posted by @NRJank in #15654 (comment)

@DHowett DHowett changed the title Legacy propsheet + shortcut properties may reset Default Terminal setting Legacy mode + shortcut propsheet may reset Default Terminal setting Apr 12, 2024
@DHowett
Copy link
Member Author

DHowett commented Apr 12, 2024

@NRJank thanks so much for writing this up! I promoted it to a separate bug. The interaction between our new settings, legacy mode, and shortcuts is a place that deserves to be better-explored

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Apr 12, 2024
@zadjii-msft
Copy link
Member

This isn't the culprit, defterm settings aren't saved there:

// 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)
{

Here we get them into the propsheet

//
// Find the available default console/terminal packages
//
LOG_IF_FAILED(DelegationConfig::s_GetAvailablePackages(g_availablePackages, g_selectedPackage));

and here we set it:

LOG_IF_FAILED(DelegationConfig::s_SetDefaultByPackage(g_selectedPackage));

@zadjii-msft zadjii-msft added Product-Conhost For issues in the Console codebase Area-DefApp labels Apr 16, 2024
@carlos-zamora carlos-zamora added Issue-Bug It either shouldn't be doing this or needs an investigation. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Apr 17, 2024
@carlos-zamora carlos-zamora added this to the Backlog milestone Apr 17, 2024
@carlos-zamora carlos-zamora added the zInbox-Bug Ignore me! label Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-DefApp Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conhost For issues in the Console codebase zInbox-Bug Ignore me!
Projects
None yet
Development

No branches or pull requests

3 participants