Fix layering issues with CascadiaSettings::_createNewProfile (#11447)

`CascadiaSettings::_createNewProfile` failed to call `_FinalizeInheritance`.
This commits fixes the issue and adds a stern warning for future me.

## PR Checklist
* [x] Closes #11392
* [x] I work here
* [x] Tests added/passed

## Validation Steps Performed

* Open settings UI
* Modify font size in base layer
* _Don't_ save
* Duplicate any profile with default font size
* Ensure the duplicated profile shows the modified base layer font size ✔️
This commit is contained in:
Leonard Hecker 2021-10-07 18:30:34 +02:00 committed by GitHub
parent 694c6b263f
commit 84e7ec4f96
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 27 additions and 8 deletions

View File

@ -272,20 +272,30 @@ namespace SettingsModelLocalTests
void ProfileTests::DuplicateProfileTest()
{
static constexpr std::string_view userProfiles{ R"({
"profiles": [
{
"name": "profile0",
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"backgroundImage": "file:///some/path",
"hidden": false,
}
]
"profiles": {
"defaults": {
"font": {
"size": 123
}
},
"list": [
{
"name": "profile0",
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"backgroundImage": "file:///some/path",
"hidden": false,
}
]
}
})" };
const auto settings = winrt::make_self<implementation::CascadiaSettings>(userProfiles);
const auto profile = settings->AllProfiles().GetAt(0);
const auto duplicatedProfile = settings->DuplicateProfile(profile);
// GH#11392: Ensure duplicated profiles properly inherit the base layer, even for nested objects.
VERIFY_ARE_EQUAL(123, duplicatedProfile.FontInfo().FontSize());
duplicatedProfile.Guid(profile.Guid());
duplicatedProfile.Name(profile.Name());

View File

@ -18,6 +18,13 @@ using namespace winrt::Microsoft::Terminal::Control;
using namespace winrt::Windows::Foundation::Collections;
using namespace Microsoft::Console;
// Creating a child of a profile requires us to copy certain
// required attributes. This method handles those attributes.
//
// NOTE however that it doesn't call _FinalizeInheritance() for you! Don't forget that!
//
// At the time of writing only one caller needs to call _FinalizeInheritance(),
// which is why this unsafety wasn't further abstracted away.
winrt::com_ptr<Profile> Model::implementation::CreateChild(const winrt::com_ptr<Profile>& parent)
{
auto profile = winrt::make_self<Profile>();
@ -371,6 +378,7 @@ winrt::com_ptr<Profile> CascadiaSettings::_createNewProfile(const std::wstring_v
LOG_IF_FAILED(CoCreateGuid(&guid));
auto profile = CreateChild(_baseLayerProfile);
profile->_FinalizeInheritance();
profile->Guid(guid);
profile->Name(winrt::hstring{ name });
return profile;

View File

@ -596,6 +596,7 @@ void SettingsLoader::_addParentProfile(const winrt::com_ptr<implementation::Prof
//
// When a user modifies a profile they shouldn't modify the (static/constant)
// inbox profile of course. That's why we need to call CreateChild here.
// But we don't need to call _FinalizeInheritance() yet as this is handled by SettingsLoader::FinalizeLayering().
settings.profiles.emplace_back(CreateChild(profile));
}
}