Fix cast object of type 'System.Windows.Media.Color' to type 'System.Windows.Expression' exception#4302
Fix cast object of type 'System.Windows.Media.Color' to type 'System.Windows.Expression' exception#4302Jack251970 wants to merge 5 commits intodevfrom
Conversation
…validation on Windows theme change Co-authored-by: Jack251970 <53996452+Jack251970@users.noreply.github.com>
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplace unsafe direct brush/style assignments with guarded creation and freezing of brushes, add a helper to derive caret brush values safely, make CopyStyle static, and conditionally refresh the frame on theme change only for System color scheme. (≈33 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This pull request suppresses a spurious error dialog that appears when users change Windows themes or accent colors. The error is caused by a WPF framework bug where SystemResources.InvalidateTreeResources incorrectly attempts to clone Color structs as if they were Freezable or Expression objects, resulting in an InvalidCastException that the application cannot prevent.
Changes:
- Added
IsRecoverableSystemResourceExceptionmethod to detect the specific WPF system resource invalidation exception - Integrated the new exception check into
ErrorReporting.Reportto suppress the error dialog while still logging the exception - Added comprehensive documentation explaining the WPF framework issue
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Flow.Launcher/Helper/ExceptionHelper.cs | Added new method to detect InvalidCastException from WPF's SystemResources.InvalidateTreeResources |
| Flow.Launcher/Helper/ErrorReporting.cs | Integrated the new exception check to prevent error dialog from being shown |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ource invalidation on Windows theme change" This reverts commit 372f142.
Refactored caret and background brush assignment to avoid sharing mutable instances and ensure proper resource referencing. Added GetNewCaretValue helper for safe caret brush creation. Brushes for backgrounds are now frozen for performance. Improved foreground value retrieval and dynamic resource key extraction. Made some methods static for clarity and consistency. Enhances resource management and reliability in theme handling.
coderabbitai
bot
removed
bug
Jack251970
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Flow.Launcher.Core/Resource/Theme.cs (1)
814-819:⚠️ Potential issue | 🟡 MinorRemove ToString() when looking up DynamicResource keys in ResourceDictionary.
At line 814, converting
dynamicResource.ResourceKeyto string viaToString()violates WPF resource lookup semantics.ResourceDictionaryrequires matching the actual key object (e.g.,ComponentResourceKey), not its string representation. This inconsistency is evident at line 260, where the code correctly preservesResourceKeyobject identity.Proposed fix
- var resourceKey = dynamicResource.ResourceKey.ToString(); + var resourceKey = dynamicResource.ResourceKey; - if (Resources.Contains(resourceKey)) + if (resourceKey != null && Resources.Contains(resourceKey)) { var colorResource = Resources[resourceKey];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher.Core/Resource/Theme.cs` around lines 814 - 819, The code is converting dynamicResource.ResourceKey to a string before lookup which breaks WPF resource key identity; in the Theme.cs block around the dynamicResource handling, stop calling ToString() and use the ResourceKey object directly when checking and retrieving from the ResourceDictionary (i.e., replace the use of var resourceKey = dynamicResource.ResourceKey.ToString() and subsequent Resources.Contains(resourceKey)/Resources[resourceKey] calls with checks that use dynamicResource.ResourceKey itself so ComponentResourceKey and other non-string keys resolve correctly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Flow.Launcher.Core/Resource/Theme.cs`:
- Around line 814-819: The code is converting dynamicResource.ResourceKey to a
string before lookup which breaks WPF resource key identity; in the Theme.cs
block around the dynamicResource handling, stop calling ToString() and use the
ResourceKey object directly when checking and retrieving from the
ResourceDictionary (i.e., replace the use of var resourceKey =
dynamicResource.ResourceKey.ToString() and subsequent
Resources.Contains(resourceKey)/Resources[resourceKey] calls with checks that
use dynamicResource.ResourceKey itself so ComponentResourceKey and other
non-string keys resolve correctly).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Previously, the theme was refreshed on every theme change event. Now, `_theme.RefreshFrameAsync()` is only called if the color scheme setting is set to "System", preventing unnecessary refreshes in other scenarios.
|
🥷 Code experts: jjw24 jjw24, Jack251970 have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
This pull request improves how brush and resource values are handled for theme styling in
Flow.Launcher.Core/Resource/Theme.cs, focusing on safer management of mutable objects and resource references. The main changes enhance the way caret and background brushes are assigned, ensuring that brushes are properly frozen when possible, and that dynamic resources are referenced correctly.Resolve #4298.