Skip to content

Fix for WindowsSettingBehaviour and possible AssociatedObject Nullreference #3146

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 13, 2018
Merged

Fix for WindowsSettingBehaviour and possible AssociatedObject Nullreference #3146

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a4fd6f
Fix possible Nullreference at WindowsSettingBehavior
punker76 Jan 13, 2018
8e9ac65
Fix wrong monitor positioning together with different Dpi usage for S…
punker76 Jan 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 59 additions & 21 deletions src/MahApps.Metro/MahApps.Metro.Shared/Behaviours/WindowsSettingBehaviour.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,20 @@ public class WindowsSettingBehaviour : Behavior<MetroWindow>
protected override void OnAttached()
{
base.OnAttached();

this.AssociatedObject.SourceInitialized += this.AssociatedObject_SourceInitialized;
}

/// <inheritdoc />
protected override void OnDetaching()
{
this.CleanUp();
this.CleanUp("from OnDetaching");
base.OnDetaching();
}

private void AssociatedObject_Closed(object sender, EventArgs e)
{
this.CleanUp();
this.CleanUp("from AssociatedObject closed event");
}

private void AssociatedObject_Closing(object sender, System.ComponentModel.CancelEventArgs e)
Expand All @@ -39,34 +40,58 @@ private void AssociatedObject_SourceInitialized(object sender, EventArgs e)
{
this.LoadWindowState();

this.AssociatedObject.StateChanged += this.AssociatedObject_StateChanged;
this.AssociatedObject.Closing += this.AssociatedObject_Closing;
this.AssociatedObject.Closed += this.AssociatedObject_Closed;
var window = this.AssociatedObject;
if (null == window)
{
// if the associated object is null at this point, then there is really something wrong!
Trace.TraceError($"{this}: Can not attach to nested events, cause the AssociatedObject is null.");
return;
}

window.StateChanged += this.AssociatedObject_StateChanged;
window.Closing += this.AssociatedObject_Closing;
window.Closed += this.AssociatedObject_Closed;
}

private void AssociatedObject_StateChanged(object sender, EventArgs e)
{
// save the settings on this state change, because hidden windows gets no window placements
// all the saving stuff could be so much easier with ReactiveUI :-D
if (this.AssociatedObject.WindowState == WindowState.Minimized)
if (this.AssociatedObject?.WindowState == WindowState.Minimized)
{
this.SaveWindowState();
}
}

private void CleanUp()
private void CleanUp(string fromWhere)
{
this.AssociatedObject.StateChanged -= this.AssociatedObject_StateChanged;
this.AssociatedObject.Closing -= this.AssociatedObject_Closing;
this.AssociatedObject.Closed -= this.AssociatedObject_Closed;
this.AssociatedObject.SourceInitialized -= this.AssociatedObject_SourceInitialized;
var window = this.AssociatedObject;
if (null == window)
{
// it's bad if the associated object is null, so trace this here
Trace.TraceWarning($"{this}: Can not clean up {fromWhere}, cause the AssociatedObject is null. This can maybe happen if this Behavior was already detached.");
return;
}

Trace.TraceInformation($"{this}: Clean up {fromWhere}.");

window.StateChanged -= this.AssociatedObject_StateChanged;
window.Closing -= this.AssociatedObject_Closing;
window.Closed -= this.AssociatedObject_Closed;
window.SourceInitialized -= this.AssociatedObject_SourceInitialized;
}

#pragma warning disable 618
private void LoadWindowState()
{
var settings = this.AssociatedObject.GetWindowPlacementSettings();
if (settings == null || !this.AssociatedObject.SaveWindowPosition)
var window = this.AssociatedObject;
if (null == window)
{
return;
}

var settings = window.GetWindowPlacementSettings();
if (null == settings || !window.SaveWindowPosition)
{
return;
}
Expand All @@ -77,12 +102,12 @@ private void LoadWindowState()
}
catch (Exception e)
{
Trace.TraceError($"The settings could not be reloaded! {e}");
Trace.TraceError($"{this}: The settings could not be reloaded! {e}");
return;
}

// check for existing placement and prevent empty bounds
if (settings.Placement == null || settings.Placement.normalPosition.IsEmpty)
if (null == settings.Placement || settings.Placement.normalPosition.IsEmpty)
{
return;
}
Expand All @@ -92,10 +117,15 @@ private void LoadWindowState()
var wp = settings.Placement;
wp.flags = 0;
wp.showCmd = (wp.showCmd == SW.SHOWMINIMIZED ? SW.SHOWNORMAL : wp.showCmd);
var hwnd = new WindowInteropHelper(this.AssociatedObject).Handle;

// this fixes wrong monitor positioning together with different Dpi usage for SetWindowPlacement
window.Left = wp.normalPosition.Left;
window.Top = wp.normalPosition.Top;

var hwnd = new WindowInteropHelper(window).Handle;
if (!NativeMethods.SetWindowPlacement(hwnd, wp))
{
Trace.TraceWarning($"The WINDOWPLACEMENT {wp} could not be set by SetWindowPlacement.");
Trace.TraceWarning($"{this}: The WINDOWPLACEMENT {wp} could not be set by SetWindowPlacement.");
}
}
catch (Exception ex)
Expand All @@ -106,12 +136,19 @@ private void LoadWindowState()

private void SaveWindowState()
{
var settings = this.AssociatedObject.GetWindowPlacementSettings();
if (settings == null || !this.AssociatedObject.SaveWindowPosition)
var window = this.AssociatedObject;
if (null == window)
{
return;
}
var hwnd = new WindowInteropHelper(this.AssociatedObject).Handle;

var settings = window.GetWindowPlacementSettings();
if (null == settings || !window.SaveWindowPosition)
{
return;
}

var hwnd = new WindowInteropHelper(window).Handle;
var wp = NativeMethods.GetWindowPlacement(hwnd);
// check for saveable values
if (wp.showCmd != SW.HIDE && wp.length > 0)
Expand All @@ -124,6 +161,7 @@ private void SaveWindowState()
wp.normalPosition = rect;
}
}

if (!wp.normalPosition.IsEmpty)
{
settings.Placement = wp;
Expand All @@ -136,7 +174,7 @@ private void SaveWindowState()
}
catch (Exception e)
{
Trace.TraceError($"The settings could not be saved! {e}");
Trace.TraceError($"{this}: The settings could not be saved! {e}");
}
}
#pragma warning restore 618
Expand Down