Skip to content

fix(Navigation): Use NullLogger fallback with diagnostic warning and add documentation to prevent crash when Region.Attached is used in ExtendedSplashScreen (backport #2958) - #2968

Merged
kazo0 merged 8 commits into
release/stable/7.0from
mergify/bp/release/stable/7.0/pr-2958
Dec 3, 2025
Merged

fix(Navigation): Use NullLogger fallback with diagnostic warning and add documentation to prevent crash when Region.Attached is used in ExtendedSplashScreen (backport #2958)#2968
kazo0 merged 8 commits into
release/stable/7.0from
mergify/bp/release/stable/7.0/pr-2958

Conversation

@mergify

@mergify mergify Bot commented Dec 1, 2025

Copy link
Copy Markdown

GitHub Issue (If applicable): closes #2957

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Documentation content changes

What is the current behavior?

Using Region.Attached="True" inside an ExtendedSplashScreen content throws NullReferenceException: "Logger needs to be set" because XAML parsing triggers AttachedChanged before NavigationHostedService.StartAsync initializes the logger.

<utu:ExtendedSplashScreen x:Name="Splash">
    <Grid uen:Region.Attached="True"
          uen:Region.Navigator="Visibility">
        
    </Grid>
</utu:ExtendedSplashScreen>

What is the new behavior?

The navigation system already properly defers region initialization until services are available. When Region.Attached="True" is encountered:

  1. A NavigationRegion is created and hooks up ViewLoading/ViewLoaded events
  2. When the view loads, AssignParent is called which looks for the service provider
  3. If services aren't available yet, it returns early and waits
  4. Once services become available, the region properly initializes

The only missing piece was that Region.Logger was accessed before NavigationHostedService.StartAsync ran, causing the crash. This fix:

  • Uses NullLogger<NavigationRegion>.Instance as a fallback when the logger hasn't been set yet
  • Outputs a diagnostic message via Debug.WriteLine explaining the deferred initialization behavior
  • Uses thread-safe Interlocked.CompareExchange to ensure the message only appears once

Code Changes:

// Before
internal static ILogger Logger { get => _logger ?? throw new NullReferenceException("Logger needs to be set"); ... }

// After  
internal static ILogger Logger
{
    get
    {
        if (_logger is null && Interlocked.CompareExchange(ref _loggerWarningIssued, 1, 0) == 0)
        {
            // This warning is expected when Region.Attached="True" is used before the navigation
            // host is fully started (e.g., inside ExtendedSplashScreen content). The region will
            // be properly initialized once the view is loaded and services become available.
            System.Diagnostics.Debug.WriteLine(
                "[Uno.Extensions.Navigation] Region.Attached is being used before the navigation host has fully started. " +
                "This typically happens when Region.Attached=\"True\" is set on content inside an ExtendedSplashScreen. " +
                "The region will defer its full initialization until services are available. " +
                "Navigation logging is temporarily disabled and will be enabled once the host starts.");
        }
        return _logger ?? NullLogger<NavigationRegion>.Instance;
    }
    set => _logger = value;
}

Documentation Changes:

  • Added a new section "Avoid Using Region.Attached in Shell.xaml Content" to doc/Learn/Navigation/HowTo-Regions.md that explains why Region.Attached="True" should not be used inside ExtendedSplashScreen content, shows incorrect usage with a code example marked to avoid, and provides the correct approach with examples showing where to define regions instead.
  • Added a concise IMPORTANT callout to doc/Learn/Navigation/Walkthrough/DefineRegions.md warning developers not to use Region.Attached="True" inside Shell.xaml or ExtendedSplashScreen content.

This documentation will be available to the Uno Docs MCP and LLM agents to prevent them from generating code with this anti-pattern.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Docs have been added/updated which fit documentation template. (for bug fixes / features)
  • Unit Tests and/or UI Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Wasm UI Tests are not showing unexpected any differences. Validate PR Screenshots Compare Test Run results.
  • Contains NO breaking changes
  • Updated the Release Notes
  • Associated with an issue (GitHub or internal)

Other information

The documentation added to both HowTo-Regions.md and Walkthrough/DefineRegions.md serves as guidance for both developers and LLM/AI agents to avoid using Region.Attached in Shell.xaml content, which is an anti-pattern that can cause initialization issues.

Internal Issue (If applicable):

Original prompt

This section details on the original issue you should resolve

<issue_title>[Navigation] Logger exception is thrown when using custom ExtendedSplashScreen content with Region.Attached="True"</issue_title>
<issue_description>Using the Recommended Uno App Template where we have a Shell.xaml that includes an ExtendedSplashScreen will throw an error if we were to add our own custom Content on the ExtendedSplashScreen.

If we were to add a Grid with the Region.Attached Attached Property set to true like this:

<utu:ExtendedSplashScreen x:Name="Splash"
								  HorizontalAlignment="Stretch"
								  VerticalAlignment="Stretch"
								  HorizontalContentAlignment="Stretch"
								  VerticalContentAlignment="Stretch">
	<utu:ExtendedSplashScreen.LoadingContentTemplate>
		<DataTemplate>
			<Grid>
				<Grid.RowDefinitions>
					<RowDefinition Height="2*" />
					<RowDefinition />
				</Grid.RowDefinitions>

				<ProgressRing IsActive="True"
							  Grid.Row="1"
							  VerticalAlignment="Center"
							  HorizontalAlignment="Center"
							  Height="100"
							  Width="100" />
			</Grid>
		</DataTemplate>
	</utu:ExtendedSplashScreen.LoadingContentTemplate>

	<Grid uen:Region.Attached="True"
		  uen:Region.Navigator="Visibility">
		
	</Grid>
</utu:ExtendedSplashScreen>

There is a runtime exception of type NullReferenceException with the Message: "Logger needs to be set"

The expectation is that the Navigation Extension handles the navigation from the Shell to the initial page by itself and so adding Content to the ExtendedSplashScreen is not needed. But we should still not crash if someone were to add this logic and we should provide a better warning/error message about proper usage. We should also document this in the documentation for Navigation
</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.


This is an automatic backport of pull request #2958 done by Mergify.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ kazo0
❌ Copilot
You have signed the CLA already but the status is still pending? Let us recheck it.

@kazo0
kazo0 enabled auto-merge December 1, 2025 23:24
Copilot AI and others added 8 commits December 1, 2025 18:33
…enceException when Region.Attached is used before NavigationHostedService starts

Co-authored-by: kazo0 <4793020+kazo0@users.noreply.github.com>
(cherry picked from commit 53c151c)
…lization

Co-authored-by: vatsashah45 <72754751+vatsashah45@users.noreply.github.com>
(cherry picked from commit b76cfd2)
Co-authored-by: vatsashah45 <72754751+vatsashah45@users.noreply.github.com>
(cherry picked from commit c4d3e26)
…d region initialization

Co-authored-by: kazo0 <4793020+kazo0@users.noreply.github.com>
(cherry picked from commit 37bcca2)
…content

Co-authored-by: kazo0 <4793020+kazo0@users.noreply.github.com>
(cherry picked from commit 55bf394)
Co-authored-by: kazo0 <4793020+kazo0@users.noreply.github.com>
(cherry picked from commit 19e52c9)
(cherry picked from commit 9823831)
(cherry picked from commit f1c7fd4)
@kazo0
kazo0 force-pushed the mergify/bp/release/stable/7.0/pr-2958 branch from 17f0856 to 98b48b5 Compare December 1, 2025 23:34
@kazo0
kazo0 merged commit 958b719 into release/stable/7.0 Dec 3, 2025
13 of 14 checks passed
@kazo0
kazo0 deleted the mergify/bp/release/stable/7.0/pr-2958 branch December 3, 2025 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants