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

Remove startOnUserLogin from the settings; use OS APIs only #18530

Merged
merged 12 commits into from
Feb 20, 2025
5 changes: 0 additions & 5 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -2480,11 +2480,6 @@
"minimum": 1,
"type": "integer"
},
"startOnUserLogin": {
"default": false,
"description": "When set to true, this enables the launch of Terminal at startup. Setting this to false will disable the startup task entry. If the Terminal startup task entry is disabled either by org policy or by user action this setting will have no effect.",
"type": "boolean"
},
"firstWindowPreference": {
"default": "defaultProfile",
"description": "Defines what behavior the terminal takes when it starts. \"defaultProfile\" will have the terminal launch with one tab of the default profile, and \"persistedWindowLayout\" will cause the terminal to save its layout on close and reload it on open.",
Expand Down
38 changes: 0 additions & 38 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ static winrt::hstring _GetErrorText(SettingsLoadErrors error)
return _GetMessageText(static_cast<uint32_t>(error), settingsLoadErrorsLabels);
}

static constexpr std::wstring_view StartupTaskName = L"StartTerminalOnLoginTask";

namespace winrt::TerminalApp::implementation
{
// Function Description:
Expand Down Expand Up @@ -353,40 +351,6 @@ namespace winrt::TerminalApp::implementation
}
CATCH_LOG()

safe_void_coroutine AppLogic::_ApplyStartupTaskStateChange()
try
{
// First, make sure we're running in a packaged context. This method
// won't work, and will crash mysteriously if we're running unpackaged.
if (!IsPackaged())
{
co_return;
}

const auto tryEnableStartupTask = _settings.GlobalSettings().StartOnUserLogin();
const auto task = co_await StartupTask::GetAsync(StartupTaskName);

switch (task.State())
{
case StartupTaskState::Disabled:
if (tryEnableStartupTask)
{
co_await task.RequestEnableAsync();
}
break;
case StartupTaskState::DisabledByUser:
// TODO: GH#6254: define UX for other StartupTaskStates
break;
case StartupTaskState::Enabled:
if (!tryEnableStartupTask)
{
task.Disable();
}
break;
}
}
CATCH_LOG();

// Method Description:
// - Reloads the settings from the settings.json file.
// - When this is called the first time, this initializes our settings. See
Expand Down Expand Up @@ -446,7 +410,6 @@ namespace winrt::TerminalApp::implementation
// TerminalSettings object.

_ApplyLanguageSettingChange();
_ApplyStartupTaskStateChange();
_ProcessLazySettingsChanges();

auto warnings{ winrt::multi_threaded_vector<SettingsLoadWarnings>() };
Expand All @@ -473,7 +436,6 @@ namespace winrt::TerminalApp::implementation
// Both LoadSettings and ReloadSettings are supposed to call this function,
// but LoadSettings skips it, so that the UI starts up faster.
// Now that the UI is present we can do them with a less significant UX impact.
_ApplyStartupTaskStateChange();
_ProcessLazySettingsChanges();

FILETIME creationTime, exitTime, kernelTime, userTime, now;
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ namespace winrt::TerminalApp::implementation
TerminalApp::ContentManager _contentManager{ winrt::make<implementation::ContentManager>() };

void _ApplyLanguageSettingChange() noexcept;
safe_void_coroutine _ApplyStartupTaskStateChange();

[[nodiscard]] HRESULT _TryLoadSettings() noexcept;
void _ProcessLazySettingsChanges();
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalSettingsEditor/Launch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "Launch.h"
#include "Launch.g.cpp"
#include "EnumEntry.h"
#include "LaunchViewModel.h"

#include <LibraryResources.h>

Expand Down Expand Up @@ -40,5 +41,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void Launch::OnNavigatedTo(const NavigationEventArgs& e)
{
_ViewModel = e.Parameter().as<Editor::LaunchViewModel>();
auto innerViewModel{ winrt::get_self<Editor::implementation::LaunchViewModel>(_ViewModel) };
/* coroutine dispatch */ innerViewModel->PrepareStartOnUserLoginSettings();
}
}
7 changes: 5 additions & 2 deletions src/cascadia/TerminalSettingsEditor/Launch.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,11 @@
</local:SettingContainer>

<!-- Start on User Login -->
<local:SettingContainer x:Uid="Globals_StartOnUserLogin">
<ToggleSwitch IsOn="{x:Bind ViewModel.StartOnUserLogin, Mode=TwoWay}"
<local:SettingContainer x:Uid="Globals_StartOnUserLogin"
HelpText="{x:Bind ViewModel.StartOnUserLoginStatefulHelpText, Mode=OneWay}"
Visibility="{x:Bind ViewModel.StartOnUserLoginAvailable, Mode=OneTime}">
<ToggleSwitch IsEnabled="{x:Bind ViewModel.StartOnUserLoginConfigurable, Mode=OneWay}"
IsOn="{x:Bind ViewModel.StartOnUserLogin, Mode=TwoWay}"
Style="{StaticResource ToggleSwitchInExpanderStyle}" />
</local:SettingContainer>

Expand Down
84 changes: 84 additions & 0 deletions src/cascadia/TerminalSettingsEditor/LaunchViewModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ using namespace winrt::Windows::Foundation;
using namespace winrt::Microsoft::Terminal::Settings::Model;
using namespace winrt::Windows::UI::Xaml::Data;

static constexpr std::wstring_view StartupTaskName = L"StartTerminalOnLoginTask";

namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
// For ComboBox an empty SelectedItem string denotes no selection.
Expand Down Expand Up @@ -365,4 +367,86 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
return _Settings.DefaultTerminals();
}

bool LaunchViewModel::StartOnUserLoginAvailable()
{
return IsPackaged();
}

safe_void_coroutine LaunchViewModel::PrepareStartOnUserLoginSettings()
{
if (!StartOnUserLoginAvailable())
{
co_return;
}

auto strongThis{ get_strong() };
auto task{ co_await winrt::Windows::ApplicationModel::StartupTask::GetAsync(StartupTaskName) };
_startOnUserLoginTask = std::move(task);
_NotifyChanges(L"StartOnUserLoginConfigurable", L"StartOnUserLoginStatefulHelpText", L"StartOnUserLogin");
}

bool LaunchViewModel::StartOnUserLoginConfigurable()
{
if (!_startOnUserLoginTask)
{
return false;
}
namespace WAM = winrt::Windows::ApplicationModel;
const auto state{ _startOnUserLoginTask.State() };
// Terminal cannot change the state of the login task if it is any of the "ByUser" or "ByPolicy" states.
return state == WAM::StartupTaskState::Disabled || state == WAM::StartupTaskState::Enabled;
}

winrt::hstring LaunchViewModel::StartOnUserLoginStatefulHelpText()
{
if (_startOnUserLoginTask)
{
namespace WAM = winrt::Windows::ApplicationModel;
switch (_startOnUserLoginTask.State())
{
case WAM::StartupTaskState::EnabledByPolicy:
case WAM::StartupTaskState::DisabledByPolicy:
return winrt::hstring{ L"\uE72E " } /*lock icon*/ + RS_(L"Globals_StartOnUserLogin_UnavailableByPolicy");
case WAM::StartupTaskState::DisabledByUser:
return RS_(L"Globals_StartOnUserLogin_DisabledByUser");
case WAM::StartupTaskState::Enabled:
case WAM::StartupTaskState::Disabled:
default:
break; // fall through to the common case (no task, not configured, etc.)
}
}
return RS_(L"Globals_StartOnUserLogin/HelpText");
}

bool LaunchViewModel::StartOnUserLogin()
{
if (!_startOnUserLoginTask)
{
return false;
}
namespace WAM = winrt::Windows::ApplicationModel;
const auto state{ _startOnUserLoginTask.State() };
return state == WAM::StartupTaskState::Enabled || state == WAM::StartupTaskState::EnabledByPolicy;
}

safe_void_coroutine LaunchViewModel::StartOnUserLogin(bool enable)
{
if (!_startOnUserLoginTask)
{
co_return;
}

auto strongThis{ get_strong() };
if (enable)
{
co_await _startOnUserLoginTask.RequestEnableAsync();
}
else
{
_startOnUserLoginTask.Disable();
}
// Any of these could have changed in response to an attempt to enable (e.g. it was disabled in task manager since our last check)
_NotifyChanges(L"StartOnUserLoginConfigurable", L"StartOnUserLoginStatefulHelpText", L"StartOnUserLogin");
}
}
11 changes: 10 additions & 1 deletion src/cascadia/TerminalSettingsEditor/LaunchViewModel.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "LaunchViewModel.g.h"
#include "ViewModelHelpers.h"
#include "Utils.h"
#include <cppwinrt_utils.h>

namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
Expand Down Expand Up @@ -51,10 +52,16 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
GETSET_BINDABLE_ENUM_SETTING(WindowingBehavior, Model::WindowingMode, _Settings.GlobalSettings().WindowingBehavior);

PERMANENT_OBSERVABLE_PROJECTED_SETTING(_Settings.GlobalSettings(), CenterOnLaunch);
PERMANENT_OBSERVABLE_PROJECTED_SETTING(_Settings.GlobalSettings(), StartOnUserLogin);
PERMANENT_OBSERVABLE_PROJECTED_SETTING(_Settings.GlobalSettings(), InitialRows);
PERMANENT_OBSERVABLE_PROJECTED_SETTING(_Settings.GlobalSettings(), InitialCols);

bool StartOnUserLoginAvailable();
safe_void_coroutine PrepareStartOnUserLoginSettings();
bool StartOnUserLoginConfigurable();
winrt::hstring StartOnUserLoginStatefulHelpText();
bool StartOnUserLogin();
safe_void_coroutine StartOnUserLogin(bool enable);

private:
Model::CascadiaSettings _Settings;
winrt::Windows::Foundation::Collections::IObservableVector<winrt::hstring> _languageList;
Expand All @@ -63,6 +70,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

winrt::Windows::Foundation::Collections::IObservableVector<winrt::Microsoft::Terminal::Settings::Editor::EnumEntry> _LaunchModeList;
winrt::Windows::Foundation::Collections::IMap<Model::LaunchMode, winrt::Microsoft::Terminal::Settings::Editor::EnumEntry> _LaunchModeMap;

winrt::Windows::ApplicationModel::StartupTask _startOnUserLoginTask{ nullptr };
};
};

Expand Down
8 changes: 5 additions & 3 deletions src/cascadia/TerminalSettingsEditor/LaunchViewModel.idl
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ namespace Microsoft.Terminal.Settings.Editor
{
runtimeclass LaunchViewModel : Windows.UI.Xaml.Data.INotifyPropertyChanged
{
LaunchViewModel(Microsoft.Terminal.Settings.Model.CascadiaSettings settings);

static String LanguageDisplayConverter(String tag);
Boolean LanguageSelectorAvailable { get; };
Windows.Foundation.Collections.IObservableVector<String> LanguageList { get; };
Expand Down Expand Up @@ -41,8 +39,12 @@ namespace Microsoft.Terminal.Settings.Editor
IObservableVector<Microsoft.Terminal.Settings.Editor.EnumEntry> WindowingBehaviorList { get; };

PERMANENT_OBSERVABLE_PROJECTED_SETTING(Boolean, CenterOnLaunch);
PERMANENT_OBSERVABLE_PROJECTED_SETTING(Boolean, StartOnUserLogin);
PERMANENT_OBSERVABLE_PROJECTED_SETTING(Int32, InitialRows);
PERMANENT_OBSERVABLE_PROJECTED_SETTING(Int32, InitialCols);

Boolean StartOnUserLogin { get; set; };
Boolean StartOnUserLoginAvailable { get; };
Boolean StartOnUserLoginConfigurable { get; };
String StartOnUserLoginStatefulHelpText { get; };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2332,4 +2332,12 @@
<value>None</value>
<comment>Text displayed when the tab title is not defined.</comment>
</data>
<data name="Globals_StartOnUserLogin_DisabledByUser" xml:space="preserve">
<value>Automatic startup has been disabled in the Startup Apps section of Windows settings.</value>
<comment>{Locked="Windows"}This is displayed in concordance with Globals_StartOnUserLogin if the user has disabled the setting outside of the application.</comment>
</data>
<data name="Globals_StartOnUserLogin_UnavailableByPolicy" xml:space="preserve">
<value>This option is managed by enterprise policy and cannot be changed here.</value>
<comment>This is displayed in concordance with Globals_StartOnUserLogin if the enterprise administrator has taken control of this setting.</comment>
</data>
</root>
93 changes: 52 additions & 41 deletions src/cascadia/TerminalSettingsEditor/SettingContainer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
L"HelpText",
xaml_typename<hstring>(),
xaml_typename<Editor::SettingContainer>(),
PropertyMetadata{ box_value(L"") });
PropertyMetadata{ box_value(L""), PropertyChangedCallback{ &SettingContainer::_OnHelpTextChanged } });
}
if (!_FontIconGlyphProperty)
{
Expand Down Expand Up @@ -126,48 +126,15 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
get_self<SettingContainer>(obj)->_UpdateOverrideSystem();
}

void SettingContainer::OnApplyTemplate()
void SettingContainer::_OnHelpTextChanged(const DependencyObject& d, const DependencyPropertyChangedEventArgs& /*args*/)
{
if (const auto& child{ GetTemplateChild(L"ResetButton") })
{
if (const auto& button{ child.try_as<Controls::Button>() })
{
// Apply click handler for the reset button.
// When clicked, we dispatch the bound ClearSettingValue event,
// resulting in inheriting the setting value from the parent.
button.Click([=](auto&&, auto&&) {
ClearSettingValue.raise(*this, nullptr);

// move the focus to the child control
if (const auto& content{ Content() })
{
if (const auto& control{ content.try_as<Controls::Control>() })
{
control.Focus(FocusState::Programmatic);
return;
}
else if (const auto& panel{ content.try_as<Controls::Panel>() })
{
for (const auto& panelChild : panel.Children())
{
if (const auto& panelControl{ panelChild.try_as<Controls::Control>() })
{
panelControl.Focus(FocusState::Programmatic);
return;
}
}
}
// if we get here, we didn't find something to reasonably focus to.
}
});

// apply name (automation property)
Automation::AutomationProperties::SetName(child, RS_(L"SettingContainer_OverrideMessageBaseLayer"));
}
}

_UpdateOverrideSystem();
// update visibility for override message and reset button
const auto& obj{ d.try_as<Editor::SettingContainer>() };
get_self<SettingContainer>(obj)->_UpdateHelpText();
}

void SettingContainer::_UpdateHelpText()
{
// Get the correct base to apply automation properties to
std::vector<DependencyObject> base;
base.reserve(2);
Expand Down Expand Up @@ -215,6 +182,50 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
}
}

void SettingContainer::OnApplyTemplate()
{
if (const auto& child{ GetTemplateChild(L"ResetButton") })
{
if (const auto& button{ child.try_as<Controls::Button>() })
{
// Apply click handler for the reset button.
// When clicked, we dispatch the bound ClearSettingValue event,
// resulting in inheriting the setting value from the parent.
button.Click([=](auto&&, auto&&) {
ClearSettingValue.raise(*this, nullptr);

// move the focus to the child control
if (const auto& content{ Content() })
{
if (const auto& control{ content.try_as<Controls::Control>() })
{
control.Focus(FocusState::Programmatic);
return;
}
else if (const auto& panel{ content.try_as<Controls::Panel>() })
{
for (const auto& panelChild : panel.Children())
{
if (const auto& panelControl{ panelChild.try_as<Controls::Control>() })
{
panelControl.Focus(FocusState::Programmatic);
return;
}
}
}
// if we get here, we didn't find something to reasonably focus to.
}
});

// apply name (automation property)
Automation::AutomationProperties::SetName(child, RS_(L"SettingContainer_OverrideMessageBaseLayer"));
}
}

_UpdateOverrideSystem();
_UpdateHelpText();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 This line is new

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So yes! SettingContainer did not support dynamic updates to HelpText!

}

void SettingContainer::SetExpanded(bool expanded)
{
if (const auto& child{ GetTemplateChild(L"Expander") })
Expand Down
Loading
Loading