Skip to content

Fixed NullReferenceException when using x:Name with SolidColorBrush#28805

Closed
Vignesh-SF3580 wants to merge 10 commits intodotnet:mainfrom
Vignesh-SF3580:fix-28711
Closed

Fixed NullReferenceException when using x:Name with SolidColorBrush#28805
Vignesh-SF3580 wants to merge 10 commits intodotnet:mainfrom
Vignesh-SF3580:fix-28711

Conversation

@Vignesh-SF3580
Copy link
Contributor

Issue Detail

Null exception occurs when using x:Name for SolidColorBrush.

Root Cause:

When x:Name is assigned to SolidColorBrush, it triggers the RegisterName method in NameScope.cs, adding the value to a dictionary. It calls GetHashCode when setting x:Name. In that time, color was null and it throws null exception.

Description of change:

Added a null check for Color resolves the issue.

Reference:

https://github.com/dotnet/maui/blob/main/src/Controls/src/Core/GradientStop.cs#L53
https://github.com/dotnet/maui/blob/main/src/Controls/src/Core/ImageBrush.cs#L32

Issues Fixed

Fixes #28711

Screenshots

Before Issue Fix After Issue Fix
28711Before.mov
28711After.mov

@dotnet-policy-service dotnet-policy-service bot added community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration labels Apr 4, 2025
@Vignesh-SF3580 Vignesh-SF3580 marked this pull request as ready for review April 7, 2025 04:59
Copilot AI review requested due to automatic review settings April 7, 2025 04:59
@Vignesh-SF3580 Vignesh-SF3580 requested a review from a team as a code owner April 7, 2025 04:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • src/Controls/tests/TestCases.HostApp/Issues/Issue28711.xaml: Language not supported
Comments suppressed due to low confidence (1)

src/Controls/src/Core/SolidColorBrush.cs:56

  • [nitpick] Consider adding a dedicated unit test for the GetHashCode method to explicitly validate its behavior when Color is null, ensuring that the fix works as expected in isolation from the UI tests.
return -1234567890 + (Color?.GetHashCode() ?? 0);

@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz jsuarezruiz added the area-xaml XAML, CSS, Triggers, Behaviors label Apr 7, 2025
@rmarinho rmarinho added this to the .NET 9 SR7 milestone Apr 9, 2025
@PureWeen PureWeen modified the milestones: .NET 9 SR7, .NET 9 SR8 May 8, 2025
@PureWeen PureWeen moved this from Todo to Ready To Review in MAUI SDK Ongoing May 8, 2025
rmarinho
rmarinho previously approved these changes May 9, 2025
@github-project-automation github-project-automation bot moved this from Ready To Review to Approved in MAUI SDK Ongoing May 9, 2025
@rmarinho
Copy link
Member

rmarinho commented May 9, 2025

/rebase

@rmarinho
Copy link
Member

rmarinho commented May 9, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@Vignesh-SF3580
Copy link
Contributor Author

  • the expectations of the bug report aren't clear, I don't know what a good fix for his could be, beyond 'not crash'
  • the implementation of GetHashCode is wrong (not your fault), but fix anyway
  • the tests should be moved to Xaml.UnitTests so it tests for all present and future inflators

I’ve made the changes based on your suggestions. Let me know if you have any further feedback.

@StephaneDelcroix
Copy link
Contributor

I’ve made the changes based on your suggestions. Let me know if you have any further feedback.

not really, but that'll do provided you fix the GetHashCode implementation as suggested

I'll look at what is expected of this code later on

@Vignesh-SF3580
Copy link
Contributor Author

I’ve made the changes based on your suggestions. Let me know if you have any further feedback.

not really, but that'll do provided you fix the GetHashCode implementation as suggested

I'll look at what is expected of this code later on

I have made the changes as per your suggestion regarding the GetHashCode implementation. Please let me know if you have any further feedback.

@PureWeen PureWeen modified the milestones: .NET 9 SR8, .NET 9 SR9 Jun 9, 2025
}

[Fact]
public void TestGetSolidColorBrushHashCode()
Copy link
Contributor

Choose a reason for hiding this comment

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

Could include more tests for the added methods to GradientStip and imageBrush?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jsuarezruiz Tests for GradientStop already exist. I’ve now added tests for ImageBrush as well.

@dartasen
Copy link
Contributor

dartasen commented Jul 3, 2025

@StephaneDelcroix Any news on this one ?

@PureWeen PureWeen modified the milestones: .NET 9 SR9, .NET 9 SR10 Jul 3, 2025
@PureWeen PureWeen modified the milestones: .NET 9 SR10, .NET 9 SR12 Aug 4, 2025
@PureWeen PureWeen modified the milestones: .NET 9 SR12, .NET 10 SR1 Sep 10, 2025
@PureWeen PureWeen modified the milestones: .NET 10 SR1, .NET 10.0 SR2 Nov 4, 2025
StephaneDelcroix added a commit that referenced this pull request Nov 27, 2025
Fixes #28711

When using x:Name on a SolidColorBrush in ContentPage.Resources, a
NullReferenceException was thrown because GetHashCode was called
(during NameScope registration) before Color was set.

Changes:
- SolidColorBrush.GetHashCode(): Add null check for Color
- GradientStop.GetHashCode(): Use HashCode.Combine for proper hashing
- ImageBrush.GetHashCode(): Use HashCode.Combine for proper hashing
- Add XAML unit test for x:Name on resources

Close #28805
StephaneDelcroix added a commit that referenced this pull request Nov 27, 2025
Fixes #28711

When using x:Name on a SolidColorBrush in ContentPage.Resources, a
NullReferenceException was thrown because GetHashCode was called
(during NameScope registration) before Color was set.

The fix uses base.GetHashCode() (identity-based) instead of hashing
the mutable Color/ImageSource properties. This is correct because:
1. These are mutable reference types - their hash shouldn't change
2. The original implementation was incorrect anyway (hash based on
   mutable state breaks dictionary/hashset semantics)

Changes:
- SolidColorBrush.GetHashCode(): Use base.GetHashCode()
- GradientStop.GetHashCode(): Use base.GetHashCode()
- ImageBrush.GetHashCode(): Use base.GetHashCode()
- Add XAML unit test for x:Name on resources

Close #28805
jfversluis pushed a commit that referenced this pull request Nov 28, 2025
Fixes #28711

When using x:Name on a SolidColorBrush in ContentPage.Resources, a
NullReferenceException was thrown because GetHashCode was called
(during NameScope registration) before Color was set.

The fix uses base.GetHashCode() (identity-based) instead of hashing
the mutable Color/ImageSource properties. This is correct because:
1. These are mutable reference types - their hash shouldn't change
2. The original implementation was incorrect anyway (hash based on
   mutable state breaks dictionary/hashset semantics)

Changes:
- SolidColorBrush.GetHashCode(): Use base.GetHashCode()
- GradientStop.GetHashCode(): Use base.GetHashCode()
- ImageBrush.GetHashCode(): Use base.GetHashCode()
- Add XAML unit test for x:Name on resources

Close #28805
jfversluis added a commit that referenced this pull request Nov 28, 2025
<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting
artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from
this PR and let us know in a comment if this change resolves your issue.
Thank you!

## Description

Fixes #28711
Close #28805

When using `x:Name` on a `SolidColorBrush` (or other brush types) in
`ContentPage.Resources`, a `NullReferenceException` was thrown. This
occurred because `GetHashCode()` was called during NameScope
registration before the `Color` property was set.

## Root Cause

When XAML elements with `x:Name` are registered in the NameScope, the
registration calls `GetHashCode()` on the object (for dictionary
storage). The original `SolidColorBrush.GetHashCode()` implementation
directly called `Color.GetHashCode()` without null-checking, causing a
crash when `Color` was still `null`.

## Changes

- **`SolidColorBrush.GetHashCode()`**: Added null-safe operator
(`Color?.GetHashCode() ?? 0`) and use `HashCode.Combine()` for proper
hashing
- **`GradientStop.GetHashCode()`**: Updated to use `HashCode.Combine()`
for proper hashing (fixes same potential issue)
- **`ImageBrush.GetHashCode()`**: Updated to use `HashCode.Combine()`
for proper hashing (fixes same potential issue)

The changes use `HashCode.Combine()` for non-NETSTANDARD targets and a
properly implemented hash formula for NETSTANDARD compatibility.

## Test

Added XAML unit test `Maui28711.xaml`/`.xaml.cs` that:
- Creates a page with `x:Name="namedBrush"` on a `SolidColorBrush` in
Resources
- Verifies the page loads without crashing across all 3 XAML inflators
(Runtime, XamlC, SourceGen)
- Validates the named field exists and the Color is set correctly

## Comparison with PR #28805

This PR supersedes #28805 with the following improvements:
- Test is properly placed in `Xaml.UnitTests` with `XamlInflator`
parameter (as requested by reviewer @StephaneDelcroix)
- Added `unchecked` to the NETSTANDARD path to properly handle potential
overflow
- Cleaner implementation following existing patterns in the codebase
PureWeen pushed a commit that referenced this pull request Dec 3, 2025
Fixes #28711

When using x:Name on a SolidColorBrush in ContentPage.Resources, a
NullReferenceException was thrown because GetHashCode was called
(during NameScope registration) before Color was set.

The fix uses base.GetHashCode() (identity-based) instead of hashing
the mutable Color/ImageSource properties. This is correct because:
1. These are mutable reference types - their hash shouldn't change
2. The original implementation was incorrect anyway (hash based on
   mutable state breaks dictionary/hashset semantics)

Changes:
- SolidColorBrush.GetHashCode(): Use base.GetHashCode()
- GradientStop.GetHashCode(): Use base.GetHashCode()
- ImageBrush.GetHashCode(): Use base.GetHashCode()
- Add XAML unit test for x:Name on resources

Close #28805
PureWeen pushed a commit that referenced this pull request Dec 3, 2025
Fixes #28711

When using x:Name on a SolidColorBrush in ContentPage.Resources, a
NullReferenceException was thrown because GetHashCode was called
(during NameScope registration) before Color was set.

The fix uses base.GetHashCode() (identity-based) instead of hashing
the mutable Color/ImageSource properties. This is correct because:
1. These are mutable reference types - their hash shouldn't change
2. The original implementation was incorrect anyway (hash based on
   mutable state breaks dictionary/hashset semantics)

Changes:
- SolidColorBrush.GetHashCode(): Use base.GetHashCode()
- GradientStop.GetHashCode(): Use base.GetHashCode()
- ImageBrush.GetHashCode(): Use base.GetHashCode()
- Add XAML unit test for x:Name on resources

Close #28805
PureWeen pushed a commit that referenced this pull request Dec 3, 2025
Fixes #28711

When using x:Name on a SolidColorBrush in ContentPage.Resources, a
NullReferenceException was thrown because GetHashCode was called
(during NameScope registration) before Color was set.

The fix uses base.GetHashCode() (identity-based) instead of hashing
the mutable Color/ImageSource properties. This is correct because:
1. These are mutable reference types - their hash shouldn't change
2. The original implementation was incorrect anyway (hash based on
   mutable state breaks dictionary/hashset semantics)

Changes:
- SolidColorBrush.GetHashCode(): Use base.GetHashCode()
- GradientStop.GetHashCode(): Use base.GetHashCode()
- ImageBrush.GetHashCode(): Use base.GetHashCode()
- Add XAML unit test for x:Name on resources

Close #28805
github-actions bot pushed a commit that referenced this pull request Dec 5, 2025
Fixes #28711

When using x:Name on a SolidColorBrush in ContentPage.Resources, a
NullReferenceException was thrown because GetHashCode was called
(during NameScope registration) before Color was set.

The fix uses base.GetHashCode() (identity-based) instead of hashing
the mutable Color/ImageSource properties. This is correct because:
1. These are mutable reference types - their hash shouldn't change
2. The original implementation was incorrect anyway (hash based on
   mutable state breaks dictionary/hashset semantics)

Changes:
- SolidColorBrush.GetHashCode(): Use base.GetHashCode()
- GradientStop.GetHashCode(): Use base.GetHashCode()
- ImageBrush.GetHashCode(): Use base.GetHashCode()
- Add XAML unit test for x:Name on resources

Close #28805
@PureWeen PureWeen closed this in e13ef48 Dec 8, 2025
@github-project-automation github-project-automation bot moved this from Changes Requested to Done in MAUI SDK Ongoing Dec 8, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jan 8, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-xaml XAML, CSS, Triggers, Behaviors community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration platform/android platform/ios platform/windows

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Specifying x:Name of SolidColorBrush in ContentPage.Resources causes System.NullReferenceException

8 participants