Skip to content
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

add null check to notifications center location change handler. #13520

Merged
merged 1 commit into from
Nov 16, 2022

Conversation

mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Nov 15, 2022

Purpose

Looking at common CER crashes for 2.16 I notice multiple crashes for a null reference exception when the dynamo window is moved.
It appears that in rare circumstances you can move the Dynamo window before the NotificationsCenter constructor is finished executing, this will kick off the location changed handler, which will try to access the UI popup - but it has not been created yet.

Just add a null check.

Object reference not set to an instance of an object.

   at Dynamo.Notifications.NotificationCenterController.DynamoView_LocationChanged(Object sender, EventArgs e)
   at System.EventHandler.Invoke(Object sender, EventArgs e)
   at System.Windows.Window.OnLocationChanged(EventArgs e)
   at System.Windows.Window.WmMoveChangedHelper()
   at System.Windows.Window.WmMoveChanged()
   at System.Windows.Window.WindowFilterMessage(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at System.Windows.Interop.HwndSource.PublicHooksFilterMessage(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
   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

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

Release Notes

Fix crash when moving Dynamo Window.

Reviewers

(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)

(FILL ME IN, optional) Any additional notes to reviewers or testers.

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

@mjkkirschner mjkkirschner changed the title null check add null check to notifications center location change handler. Nov 15, 2022
@mjkkirschner
Copy link
Member Author

The failure is just DynamoCoreWpfTests.CoreUserInterfaceTests.WorkspaceContextMenu_TestIfInCanvasSearchHidesOnOpeningContext again

Copy link
Contributor

@RobertGlobant20 RobertGlobant20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@QilongTang QilongTang added this to the 2.17.0 milestone Nov 16, 2022
@mjkkirschner mjkkirschner merged commit 4e7c024 into DynamoDS:master Nov 16, 2022
@mjkkirschner mjkkirschner deleted the notifcrash branch November 16, 2022 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants