13

Closed

Navigation from CustomMessageBox Dismissed event causes NullReferenceException

description

Steps to reproduce:
1) Create a CustomMessageBox object.
2) Add a Dismissed event handler to this object.
3) In the Dismissed event handler, add a Navigation to another page.
4) Run the app and dismiss the CustomMessageBox. The app should throw a NullReferenceException.

What happens (from what I can understand) is the CustomMessageBox starts dismissing, runs the dismiss event as expected, then it tries to navigate, also as expected. It then gets dismissed a second time, at the end of which (the second Dismissed event), a NullReferenceException is thrown. I believe there is some cleanup that is happening twice in this case. I will do some research in the source today and see if I can't isolate it.

For anyone else looking at this, possible fixes may include:
A try/catch in the cleanup function.
Checking for null before accessing the cleanup.
Preventing the dismiss from being called if previously dismissed, possibly by checking for navigation events.
Adding a post-Dismissed event with which to Navigate from.
Closed Jun 11, 2013 at 9:37 PM by shawnoster
Fixed in the June 2013 - Windows Phone Toolkit release. Updated source available on CodePlex, updated binaries available via NuGet.

comments

LoGravNateD wrote Nov 2, 2012 at 5:44 PM

Reformatting for easier reading because it removed my line returns.

1) Create a CustomMessageBox object.

2) Add a Dismissed event handler to this object.

3) In the Dismissed event handler, add a Navigation to another page.

4) Run the app and dismiss the CustomMessageBox. The app should throw a NullReferenceException.

What happens (from what I can understand) is the CustomMessageBox starts dismissing, runs the dismiss event as expected, then it tries to navigate, also as expected. It then gets dismissed a second time, at the end of which (the second Dismissed event), a NullReferenceException is thrown. I believe there is some cleanup that is happening twice in this case. I will do some research in the source today and see if I can't isolate it.



For anyone else looking at this, possible fixes may include:

A try/catch in the cleanup function.

Checking for null before accessing the cleanup.

Preventing the dismiss from being called if previously dismissed, possibly by checking for navigation events.

Adding a post-Dismissed event with which to Navigate from.

bernitorres wrote Nov 12, 2012 at 9:53 PM

A quick hack is to comment line 657 of CustomMessageBox.cs file and compile again
that line says _popup = null;

the real problem is, as LoGravNateD said, that ClosePopup is being called twice, but for now that hack is enough for me.

wjgroup wrote Nov 27, 2012 at 4:54 AM

to bernitorres:

I think the real proper hack should be stop Dismissed been executed twice (by checking _popup == null) otherwise your callback delegate will be called twice and that will probably cause some real damage.

gofightnguyen wrote Feb 18, 2013 at 1:53 AM

One thing that seems to work for me is adding an Unloaded event handler and using that to navigate

activa wrote Feb 24, 2013 at 3:06 PM

This is a hack that fixes the problem (at least in my cases):

In the Dismissed event handler, do this:
((CustomMessageBox)sender).Dismissing += (o, e) => e.Cancel = true;
This will cancel any extra Dismissed event handlers being triggered.

problemcognition1 wrote Mar 26, 2013 at 1:35 PM

I took a peek at the code and it looks like the Dismiss method is called twice because it is called from the Button Click event handlers AND an event handler registered to the OnNavigating event of the page. This means if you do anything that causes page navigation events in the Dismissed handler it will get called again. The problem manifests because ClosePopup is setting _popup to null and second time round we get a null reference exception. However, I think the solution is to unregister the page navigation events once we are past the point of no return in the Dismiss code. I would suggest that after the Dismissing event is raised and the opportunity to cancel is gone but before the Dismissed event is raised should do it.

Incidentally, the same problem occurs if you try to use the SMS or EMAIL tasks in the Dismissed handler since they result in Navigation events.

aceontech wrote Mar 29, 2013 at 11:10 AM

Hi,

To date (March 29, 2013), this is still a problem (I'm using the Nuget-package).
Is there an ETA for the fix, or will I need to fix and recompile it myself..?


Kind regards,
Alex

problemcognition1 wrote Apr 1, 2013 at 10:13 PM

A simple workaround is to set the Tag of the CustomMessageBox to a value (e.g string.Empty) before you show it, then in your Dismissed handler set it to null. Add a Dismissing handler that cancels if the Tag is null. This ensures that Dismissed is only called once, either because Page navigation occurred or because the user clicked a button.
        messageBox.Tag = string.Empty;

        messageBox.Dismissed += (s1, e1) =>
        {
            CustomMessageBox source = (CustomMessageBox)s1;
            source.Tag = null;
            ...
            ...
        }   

       messageBox.Dismissing += (s1, e1) => { e1.Cancel = (((CustomMessageBox)s1).Tag == null); };

       messageBox.Show();

Stephensimonpaul wrote May 19, 2013 at 3:40 PM

I used this filthy work around:

messageBox.Dismissed += (s1, e1) =>
        {
            DispatcherTimer DelayedTimer = new DispatcherTimer()
            {
                Interval = TimeSpan.FromMilliseconds(500)
            };
            DelayedTimer.Tick += (s, ee) =>
            {
                NavigationService.Navigate(new Uri("/Views/Home/Home.xaml", UriKind.Relative));
                DelayedTimer.Stop();
            };
            DelayedTimer.Start();
        };

wrote May 22, 2013 at 8:18 PM

Fixed on changeset 84869