[Android] Overriding back button functionality with OnBackButtonPressed returning false in a modally pushed page causes stack overflow - fix

[kubaflo]

Issues Fixed

Fixes #28811

Before	After
Screen.Recording.2025-04-05.at.02.58.12.mov	Screen.Recording.2025-04-05.at.03.00.04.mov

CC @pictos

[dotnet-policy-service]

Hey there @@kubaflo! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[Copilot]

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Ensure that _modalNavigationManager is always non-null when invoked in HandleOnBackPressed. If there's any possibility for a null value, consider adding appropriate fallback handling or logging to maintain robust modal navigation behavior.

[pictos]

just because this CallBack is a Java.Lang.Object, can we store the ModalNavigationManager as a WeakReference?

[kubaflo]

Good point! Done

[jsuarezruiz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[bhavanesh2001]

Maybe this test should be only for Android?

[kubaflo]

Yea probably, thanks!

[rmarinho]

OverridingBackButtonShouldNotCauseStackOverflow failing on iOS and Android, also please rebase :)

[kubaflo]

Can you please /azp again?

[PureWeen]

/rebase

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

/rebase

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

@kubaflo this one is a little interesting because the original author is returning false but they are opting out of all of our navigation logic

The fix here also feels incorrect because we are only running this test on android.

My first thought with how this would work is that if the user is overriding OnBackButotnPResed on page and they don't call the base class and then they return false

That by default we don't do anything at that point.

If they are opting out of calling our base class and returning false we can't really assume what to do for them at this point.

@pictos do you remember why we opted to call the OnBackPresed from the Dispatcher here?

[pictos]

The Dispatcher is from AndroidX, just for reference. I think this was implemented to enable modal pages to listen for and handle back button presses. We register the ComponentDialog here, and when the callback is triggered, we forward that event into Maui's structure.

[PureWeen]

yea, but this is what causes the infinite loop

Like, if I'm following the code right I feel like the fix here should just be deleting this code

						{
							customComponentDialog.OnBackPressedDispatcher.OnBackPressed();
						}

this just causes HandleBackPressed to get called infinitely when you return false

[PureWeen]

@kubaflo I think the fix here is to just delete this code completely

if (!preventBackPropagation && _modalNavigationManager.TryGetTarget(out var mnm))
						{
							customComponentDialog.OnBackPressedDispatcher.OnBackPressed();
						}

This whole situation is a bit of a mess :-/

I don't think the "OnBackButtonPressed" was fully thought through with Modal or Android really as we moved to MAUI

like, currently with a navigation page if you return false it'll cause a double back navigation and exit the app.

Technically from the API description I think the way you'd use this API would be more like this

		protected override bool OnBackButtonPressed()
		{
			if (mycustomLogic)
			{
				return true;
			}
			else
			{
				return base.OnBackButtonPressed();
			}
		}

I also worry about this change because if users call "base.OnBackButtonPressed" I think that's going to cause a double navigation back since the base class will also navigate back

changs made