Xamarin.forms: [Bug] Page not popped on iOS 13 FormSheet swipe down

[breyed]

Description

On iOS 13, when the user swipes down on a modal page with a FormSheet presentation style to dismiss it, the NavigationPage does not raise a Popped event. Instead, the page is gone, but the NavigationPage's state acts as if the page is still being displayed.

Steps to Reproduce

1. Push a FormSheet style modal page, for example, using an extension method like this:

c# public static NavigationPage PushModal(this Page source, Page modalRoot) { var navigationPage = new NavigationPage(modalRoot); Xamarin.Forms.PlatformConfiguration.iOSSpecific.Page.SetModalPresentationStyle(navigationPage.On<Xamarin.Forms.PlatformConfiguration.iOS>(), Xamarin.Forms.PlatformConfiguration.iOSSpecific.UIModalPresentationStyle.FormSheet); source.Navigation.PushModalAsync(navigationPage); return navigationPage; }

1. Run the app. Don't close the page by pressing a button or other action that causes NavigationController.PopModalAsync to be called. Instead, swipe down on the page.

Expected Behavior

The page is popped from the navigation stack, and NavigationController.Popped is raised.

Actual Behavior

The page remains on the navigation stack, and NavigationController.Popped is not raised.

Basic Information

• Version with issue: XF 4.3.0.851321-pre3
• Last known good version: none
• IDE: VS 16.3.2
• Platform Target Frameworks:
  
  
  
  • iOS: 13.0
  
  
• Affected Devices: iPhone X

[jfversluis]

This is also why #7878 was merged for now I think as a result of #7145. I've created a (draft) PR that I think fixes this, along with the whole dismissal of modals in iOS 13. Since using a modal is pretty common, I do want to have some other people looking at this before we revert back the pinning of the default modal style.

[brminnick]

Workaround

As a workaround, you can call PopModalAsync in OnDisappearing.

This ensures that, when the user dismisses the Page manually, Xamarin.Forms still pops the Page from Navigation.ModalStack.

```csharp
protected override async void OnDisappearing()
{
base.OnDisappearing();

if (Navigation.ModalStack.Any())
    await Navigation.PopModalAsync();

}

[breyed]

@brminnick Are there any situations that OnDisappearing could be called besides the user swiping down the modal page? Switching to another app? Going to the home screen? Pushing another page? Locking the phone? PopModalAsync called by the app? Phone call received?

[brminnick]

@breyed ContentPage.Disappearing doesn't fire on iOS when the app is backgrounded.

To handle other scenarios where we push another page, we can unsubscribe/resubscribe to ContentPage.Disappearing to ensure PopModalAsync doesn't fire unexpectedly.

Here's an example to handle when the user launches a camera view:

public class MyPage : ContentPage
{
    public MyPage()
    {
        Disappearing += HandlePageDisappearing;
    }

    async void HandleTakePhotoButtonClicked(object sender, EventArgs e)
    {
        Disappearing -= HandlePageDisappearing;
        //Display Camera View
        Disappearing += HandlePageDisappearing;
    }

    async void HandlePageDisappearing(object sender, EventArgs e)
    {
        if (Navigation.ModalStack.Count > 0)
            await Navigation.PopModalAsync();
    }
}

This code snippet is taken from my app where I've implemented this work-around:
https://github.com/brminnick/AzureBlobStorageSampleApp/blob/6d8c0f7cb5b86e6cea3ad352f5b6ec761a461962/AzureBlobStorageSampleApp/Pages/AddPhotoPage.cs#L106-L116

[breyed]

I used this extension method as a workaround:

c# public static void PushModal(this Page source, Page modalRoot) { var navigationPage = new NavigationPage(modalRoot); if (Device.RuntimePlatform == Device.iOS) { modalRoot.Disappearing += (sender, e) => { if (source.Navigation.ModalStack.Count > 0) source.Navigation.PopModalAsync(); // Works around a XF bug on iOS 13: https://github.com/xamarin/Xamarin.Forms/issues/7878 }; Xamarin.Forms.PlatformConfiguration.iOSSpecific.Page.SetModalPresentationStyle(navigationPage.On<Xamarin.Forms.PlatformConfiguration.iOS>(), Xamarin.Forms.PlatformConfiguration.iOSSpecific.UIModalPresentationStyle.FormSheet); } source.Navigation.PushModalAsync(navigationPage); }

[brminnick]

@breyed There are two gotchyas in your code:

1. Always unsubscribe Event Handlers
   
   
   
   • Don't subscribe to events using lambdas because they cannot be unsubscribed, leading to memory leaks
     
     
     
     
     
     • For each lambda, the compiler generates a class in which local variables, like Page source and navigationPage in your example, are stored as fields.
     
     
     
     • Because we cannot unsubscribe the compiler-generated class from the event, it will remain subscribed to the event and the garbage collector won't dispose of it
     
     
     
   
   
   • Here is more information: http://alookonthecode.blogspot.com/2011/05/lambda-expressions-anonymous-classes.html
   
   
2. Always await every Task
   
   
   
   • You are calling both PushModalAsync and PopModalAsync without using await
   
   
   • Not awaiting Tasks can hide exceptions
   
   
   • Here is more information: https://www.youtube.com/watch?v=J0mcYVxJEl0
   
   

Here is a solution where we unsubscribe from Disappearing to avoid memory leaks, and we use async/await to ensure .NET rethrows exceptions if they occur:

using System;
using System.Threading.Tasks;
using Xamarin.Forms;
using Xamarin.Forms.PlatformConfiguration;
using Xamarin.Forms.PlatformConfiguration.iOSSpecific;

namespace MyNamespace
{
    public static class NavigationExtensions
    {
        public static async Task PushModal(this Xamarin.Forms.Page source, Xamarin.Forms.Page modalRoot)
        {
            modalRoot.On<iOS>().SetUseSafeArea(true);

            var navigationPage = new Xamarin.Forms.NavigationPage(modalRoot);

            if (Device.RuntimePlatform is Device.iOS)
            {
                modalRoot.Disappearing += HandleModalPageDisappearing;
            }

            Xamarin.Forms.PlatformConfiguration.iOSSpecific.Page.SetModalPresentationStyle(navigationPage.On<iOS>(), UIModalPresentationStyle.FormSheet);

            await source.Navigation.PushModalAsync(navigationPage);
        }

        static async void HandleModalPageDisappearing(object sender, EventArgs e)
        {
            var modalPage = (Xamarin.Forms.Page)sender;

            if (modalPage.Navigation.ModalStack.Count > 0)
                await modalPage.Navigation.PopModalAsync(); // Works around a XF bug on iOS 13: https://github.com/xamarin/Xamarin.Forms/issues/7878

            modalPage.Disappearing -= HandleModalPageDisappearing;
        }
    }
}

[breyed]

@brminnick While those gotchas apply in many cases generally, neither apply in this case:

1. Event handler lifetime: In this case, the lifetime of the event handler matches the lifetime of the modal page. After the modal page is no longer referenced, it will be GCed, at which time, nothing will reference the event handler, which will also be GCed. So no memory leak. If the modal page were longer lived than the event handler, then unsubscription would be necessary.

2. Navigation exceptions: By avoiding the awaits on PushModalAsync and PopModalAsync, you tell the compiler to not generate code to look for and propagate async exceptions. That optimization is a good thing. The generated code would just make the app bigger and slower. Per the interface contract, the returned tasks always complete successfully. Moreover, even if there were a bug in the framework such that they didn't, ignoring the error may be preferable to having to write an exception handler or having an unhandled exception.

[brminnick]

@breyed I'm happy to chat more with you about this offline so that we don't clutter this Issue.

In your example, yes the garbage collector will dispose of the resources properly. I elaborated on your example, using subscribing/unsubscribing event handlers, as best-practices for other devs who would like to implement your sample, but are unaware of the dangers of using lambdas; it ensures that they don't accidentally introduce memory leaks when they modify your example for their specific use-case.

You should be using await for both PopModalAsync and PushModalAsync, because neither are guaranteed to return a successful Task:

• Xamarin.Forms.Platform.iOS.Platform.PopModalAsync utilizes Task UIViewController.PresentViewControllerAsync
• Xamarin.Forms.Platform.iOS.Platform.PushModalAsync (which calls Xamarin.Forms.Platform.iOS.Platform.PresentModal) utilizes Task UIViewController.DismissViewControllerAsync.

If either of these methods throw an exception, and you did not use the await keyword when you called the method, the .NET runtime will not rethrow your exception.

Yes, async/await does add overhead to the app size (each async method increases the app size by ~100 bytes), and yes it adds overhead to the runtime execution due to context switching, however the overhead for this extension method is negligible.

If you are concerned about this overhead, you can avoid increasing the app size and avoid the extra context switching by returning the Task in the PushModal extension method instead of awaiting it:

public static Task PushModal(this Xamarin.Forms.Page source, Xamarin.Forms.Page modalRoot)
{
    modalRoot.On<iOS>().SetUseSafeArea(true);

    var navigationPage = new Xamarin.Forms.NavigationPage(modalRoot);

    if (Device.RuntimePlatform is Device.iOS)
    {
        modalRoot.Disappearing += HandleModalPageDisappearing;
    }

    Xamarin.Forms.PlatformConfiguration.iOSSpecific.Page.SetModalPresentationStyle(navigationPage.On<iOS>(), UIModalPresentationStyle.FormSheet);

    return source.Navigation.PushModalAsync(navigationPage);
}

[breyed]

@brminnick If INaviation.PushModalAsync or INaviation.PopModalAsync return tasks that may not complete successfully, it would be good catch and indicate a need to update their docs accordingly. However, I don't see based on your analysis of the iOS adapter code, how they can fault. PresentViewControllerAsync wraps presentViewController:animated:completion:, which doesn't pass an error parameter to completion; thus it cannot cause the Task to fault. Likewise for dismiss(animated:completion:).

[Thanghand]

It is so dangerous when pop modal on disappearing. For example: Your modal has button. It can navigate to another page and event OnDisappearing will fire. It will remove your current modal page. Therefore, it is not good behavior. My solution that, I make custom NavigationPage. When i push new modal page, I always use navigationPage wrap it because it has a navigation bar. Example:
var newModalPage = new CustomNavigationPage(new TestPage);
await Navigation.PushModalAsync(newModalPage);

I make CustomNavigationPageRenderer and override the ViewDidDisappear
something like this:

It is more safe, when modal page navigate to another page. ViewDidDisappear will not call when your modal page navigate to another page. It just call when you swipe down or Pop

[breyed]

Per @Thanghand's observation that ViewDidDisappear isn't called when navigating forward, below is a custom-renderer-based workaround. A nice aspect is that it involves no bookkeeping. Here's the code, copy-paste friendly, and everything. :-)

``` c#
public class ModalNavigationPage : NavigationPage
{
public ModalNavigationPage(Page root) : base(root) { }
}

[assembly: ExportRenderer(typeof(ModalNavigationPage), typeof(ModalNavigationPageRenderer))]
public class ModalNavigationPageRenderer : NavigationRenderer
{
public override void ViewDidDisappear(bool animated)
{
base.ViewDidDisappear(animated);
if (Element.Navigation.ModalStack.Count > 0)
Element.Navigation.PopModalAsync();
}
}
```

[NoobNovice]

Workaround

As a workaround, you can call PopModalAsync in OnDisappearing.

This ensures that, when the user dismisses the Page manually, Xamarin.Forms still pops the Page from Navigation.ModalStack.

protected override async void OnDisappearing()
{
    base.OnDisappearing();

    if (Navigation.ModalStack.Any())
        await Navigation.PopModalAsync();
}

Dear @brminnick
That work but you can't to push any page stack in this page. Because when go to second page, this page will call OnDisappearing and then this page has remove. It close when I try go to second page.

So how do you have any idea?

[PureWeen]

This will be fixed by this PR

https://github.com/xamarin/Xamarin.Forms/pull/8551

[samhouts]

closed by #8551