New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Null Reference Exception in BorderlessWindowBehavior.TopMostChangeNotifierOnValueChanged #2674

Closed
mikeasage opened this Issue Sep 28, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@mikeasage
Contributor

mikeasage commented Sep 28, 2016

What steps will reproduce this issue?

I got a Null Reference exception when trying to open my Metro app that contains several metro windows. I really don't have a particular way to re-create this as I typically don't have this error. I'm guessing this is dependent on the execution order of the underlying threads somehow. The fix looks simple I guess, but I'm not sure how to create a test for it (assuming that's required). Here's my stack trace.


System.NullReferenceException: Object reference not set to an instance of an object.
at MahApps.Metro.Behaviours.BorderlessWindowBehavior.TopMostChangeNotifierOnValueChanged(Object sender, EventArgs e)
at MahApps.Metro.Controls.PropertyChangeNotifier.OnPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
at System.Windows.DependencyObject.OnPropertyChanged(DependencyPropertyChangedEventArgs e)
at System.Windows.DependencyObject.NotifyPropertyChange(DependencyPropertyChangedEventArgs args)
at System.Windows.DependencyObject.UpdateEffectiveValue(EntryIndex entryIndex, DependencyProperty dp, PropertyMetadata metadata, EffectiveValueEntry oldEntry, EffectiveValueEntry& newEntry, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType)
at System.Windows.DependencyObject.InvalidateProperty(DependencyProperty dp, Boolean preserveCurrentValue)
at System.Windows.Data.BindingExpressionBase.Invalidate(Boolean isASubPropertyChange)
at System.Windows.Data.BindingExpression.TransferValue(Object newValue, Boolean isASubPropertyChange)
at MS.Internal.Data.ClrBindingWorker.NewValueAvailable(Boolean dependencySourcesChanged, Boolean initialValue, Boolean isASubPropertyChange)
at MS.Internal.Data.PropertyPathWorker.UpdateSourceValueState(Int32 k, ICollectionView collectionView, Object newValue, Boolean isASubPropertyChange)
at MS.Internal.Data.PropertyPathWorker.OnDependencyPropertyChanged(DependencyObject d, DependencyProperty dp, Boolean isASubPropertyChange)
at System.Windows.Data.BindingExpression.HandlePropertyInvalidation(DependencyObject d, DependencyPropertyChangedEventArgs args)
at System.Windows.Data.BindingExpressionBase.OnPropertyInvalidation(DependencyObject d, DependencyPropertyChangedEventArgs args)
at System.Windows.Data.BindingExpression.OnPropertyInvalidation(DependencyObject d, DependencyPropertyChangedEventArgs args)
at System.Windows.DependentList.InvalidateDependents(DependencyObject source, DependencyPropertyChangedEventArgs sourceArgs)
at System.Windows.DependencyObject.NotifyPropertyChange(DependencyPropertyChangedEventArgs args)
at System.Windows.DependencyObject.UpdateEffectiveValue(EntryIndex entryIndex, DependencyProperty dp, PropertyMetadata metadata, EffectiveValueEntry oldEntry, EffectiveValueEntry& newEntry, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType)
at System.Windows.DependencyObject.SetValueCommon(DependencyProperty dp, Object value, PropertyMetadata metadata, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType, Boolean isInternal)
at System.Windows.Window.set_Topmost(Boolean value)
at System.Windows.Window.OnContentRendered(EventArgs e)
at System.Windows.Window.b__193_0(Object unused)
at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)


Expected outcome

Um, it doesn't crash?

Suggested Code Change

Here's the existing code. The only thing that could be null is the AssociatedObject.

    private void TopMostChangeNotifierOnValueChanged(object sender, EventArgs e)
    {
        this.savedTopMost = this.AssociatedObject.Topmost;
    }

So, I think the obvious change should be this, but beyond that, I'm not sure what should happen.

    private void TopMostChangeNotifierOnValueChanged(object sender, EventArgs e)
    {
        if (this.AssociatedObject != null)
            this.savedTopMost = this.AssociatedObject.Topmost;
    }

Environment

  • MahApps.Metro v1.3.0
  • Windows 7.1
  • Visual Studio 2015
  • .NET Framework 4.5
@punker76

This comment has been minimized.

Show comment
Hide comment
@punker76

punker76 Sep 29, 2016

Member

@mikeasage That's in my eyes little bt strange and I want to figure out why this can happen. So I see in your stack trace (thx for this) that the TopMost property is changed. Maybe one of your windows do this? Thx.

Member

punker76 commented Sep 29, 2016

@mikeasage That's in my eyes little bt strange and I want to figure out why this can happen. So I see in your stack trace (thx for this) that the TopMost property is changed. Maybe one of your windows do this? Thx.

@mikeasage

This comment has been minimized.

Show comment
Hide comment
@mikeasage

mikeasage Sep 29, 2016

Contributor

Yes, TopMost is being called in these event handlers for the Metro Window. Perhaps this not the best practice for ensuring that the window is on top?

    ContentRendered="MyWindow_ContentRendered"

    Initialized="MyWindow_Initialized



    private void MyWindow_ContentRendered(object sender, EventArgs e)

    {

        this.Topmost = false;

    }



    private void MyWindow_Initialized(object sender, EventArgs e)

    {

        this.Topmost = true;

    }

From: Jan Karger [mailto:notifications@github.com]
Sent: Thursday, September 29, 2016 2:20 AM
To: MahApps/MahApps.Metro MahApps.Metro@noreply.github.com
Cc: Michael A Sage mikeasage@sbcglobal.net; Mention mention@noreply.github.com
Subject: Re: [MahApps/MahApps.Metro] Null Reference Exception in BorderlessWindowBehavior.TopMostChangeNotifierOnValueChanged (#2674)

@mikeasage https://github.com/mikeasage That's in my eyes little bi strange and I want to figure out why this can happen. So I see in your stack trace (thx for this) that the TopMost property is changed. Maybe one of your windows do this? Thx.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #2674 (comment) , or mute the thread https://github.com/notifications/unsubscribe-auth/AAtv_6DMZKz3Jt8KeymVqzjwVN8Q4khSks5qu2ajgaJpZM4KJT-s . https://github.com/notifications/beacon/AAtv__rSO0SlGUuvVLFBBaAWjnpD0haFks5qu2ajgaJpZM4KJT-s.gif

Contributor

mikeasage commented Sep 29, 2016

Yes, TopMost is being called in these event handlers for the Metro Window. Perhaps this not the best practice for ensuring that the window is on top?

    ContentRendered="MyWindow_ContentRendered"

    Initialized="MyWindow_Initialized



    private void MyWindow_ContentRendered(object sender, EventArgs e)

    {

        this.Topmost = false;

    }



    private void MyWindow_Initialized(object sender, EventArgs e)

    {

        this.Topmost = true;

    }

From: Jan Karger [mailto:notifications@github.com]
Sent: Thursday, September 29, 2016 2:20 AM
To: MahApps/MahApps.Metro MahApps.Metro@noreply.github.com
Cc: Michael A Sage mikeasage@sbcglobal.net; Mention mention@noreply.github.com
Subject: Re: [MahApps/MahApps.Metro] Null Reference Exception in BorderlessWindowBehavior.TopMostChangeNotifierOnValueChanged (#2674)

@mikeasage https://github.com/mikeasage That's in my eyes little bi strange and I want to figure out why this can happen. So I see in your stack trace (thx for this) that the TopMost property is changed. Maybe one of your windows do this? Thx.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #2674 (comment) , or mute the thread https://github.com/notifications/unsubscribe-auth/AAtv_6DMZKz3Jt8KeymVqzjwVN8Q4khSks5qu2ajgaJpZM4KJT-s . https://github.com/notifications/beacon/AAtv__rSO0SlGUuvVLFBBaAWjnpD0haFks5qu2ajgaJpZM4KJT-s.gif

@punker76 punker76 added this to the 1.4.0 milestone Sep 30, 2016

@punker76 punker76 added the Bug label Sep 30, 2016

@punker76

This comment has been minimized.

Show comment
Hide comment
@punker76

punker76 Sep 30, 2016

Member

@mikeasage I merged your changes with some additional changes from me. Thx. But a minor note, you don't need this ContentRendered/Initialized stuff to set the Topmost. Set it directly in Xaml Topmost="True" or in code behind yourWindow.Topmost = true;.

Member

punker76 commented Sep 30, 2016

@mikeasage I merged your changes with some additional changes from me. Thx. But a minor note, you don't need this ContentRendered/Initialized stuff to set the Topmost. Set it directly in Xaml Topmost="True" or in code behind yourWindow.Topmost = true;.

@punker76 punker76 closed this Sep 30, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment