Clear IsThemeDictionary flag when ResourceDictionary gains an owner#11727
Open
amarinov-msft wants to merge 2 commits into
Open
Clear IsThemeDictionary flag when ResourceDictionary gains an owner#11727amarinov-msft wants to merge 2 commits into
amarinov-msft wants to merge 2 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a regression where Application.Resources / Window.Resources / element Resources dictionaries could be incorrectly flagged as theme dictionaries when first constructed during system resource parsing, leading to incorrect SystemResources caching and potential NullReferenceException during style application.
Changes:
- Clear the
IsThemeDictionaryflag inResourceDictionary.AddOwnerwhen the owner is anApplication,FrameworkElement, orFrameworkContentElement. - Rely on the existing
IsThemeDictionarysetter propagation to ensure merged dictionaries get the corrected (cleared) state.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ry.AddOwner Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #11714
Description
Application.Resources (and Window/FrameworkElement.Resources) could be incorrectly marked as a theme dictionary, after which any dictionary merged into it inherited the same flag. This led to wrong resources being cached in
SystemResourcesand, in some cases, aNullReferenceExceptioninStyleHelper.GetInstanceValue()when a misclassified style was applied to a container.Root cause: the
ResourceDictionaryconstructor inheritsIsThemeDictionaryfrom the ambientSystemResources.IsSystemResourcesParsingflag. The Resources property getters lazily construct their dictionary on first access, so if that first access happens while system resources are being parsed, the dictionary is permanently flagged as a theme dictionary.Fix: in
ResourceDictionary.AddOwner, clear IsThemeDictionarywhen the owner is an Application, FrameworkElement, or FrameworkContentElement. The getters callAddOwnerimmediately after construction, so the flag is corrected before it can be observed or propagated. TheIsThemeDictionarysetter already propagates the cleared value to merged dictionaries.Customer Impact
Applications can crash with a
NullReferenceExceptionduring normal UI operations (e.g. adding items to anItemsControl) whenever the application/window resource dictionary happens to be created while system resources are being parsed. The failure is timing-dependent and hard to diagnose, and there is no reliable workaround.Regression
Yes, it is regressed in .NET 9. The constructor heuristic and the lazy getters are unchanged from previous releases, but the .NET 9 Fluent theming feature (ThemeMode / ThemeManager) introduced new reads of the public
Application.Current.Resources/Window.Resourcesgetters from within resource-change/theme-sync handling. These getters can now run while system resources are parsing and lazily create the dictionary at the wrong moment, exposing the latent misclassification. It does not reproduce on .NET 8.Testing
Tested against the issue's minimal repro. The debug assertion that Application.Resources had become a theme dictionary no longer fires, and the button click no longer throws.
Risk
Low. The change is a single, narrowly scoped assignment at the point an Application / FrameworkElement / FrameworkContentElement becomes an owner. For the common case the flag is already false, so the setter's change-guard makes it a no-op with negligible overhead. Genuine theme/generic dictionaries are unaffected because they never receive these owner types. The setter's existing propagation to merged dictionaries keeps state consistent.
Microsoft Reviewers: Open in CodeFlow