-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
MudFocusTrap: Prevent JSException on Disposed Component #11586
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
MudFocusTrap: Prevent JSException on Disposed Component #11586
Conversation
## Changes - Catching possible exceptions in MudFocusTrap.OnAfterRenderAsync which are thrown by JSInterop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to catch a more specific exception than Exception
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to see if (_disposed) return; statements than a bunch of try/catch exceptions. Did you try that?
will try thanks |
Any update on this? |
## Changes - Removing the try-catch from 341d6fa - using `disposed` check, not to start JsInterop processes in SaveFocusAsync and InitializeFocusAsync AFTER the component has already been disposed of
@versile2 I adjusted the solution |
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #11586 +/- ##
=======================================
Coverage 91.31% 91.31%
=======================================
Files 466 466
Lines 14707 14708 +1
Branches 2857 2857
=======================================
+ Hits 13430 13431 +1
Misses 635 635
Partials 642 642 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Did this _isDisposed solve the problem like your other solution? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a disposal guard to MudFocusTrap
to prevent JS interop calls after the component has been disposed, avoiding JS exceptions when rapidly opening and closing dialogs.
- Introduces a
_disposed
flag in the component. - Adds an early-exit check in
OnAfterRenderAsync
to skip focus/save operations if disposed. - Sets
_disposed
totrue
inDispose
before callingRestoreFocusAsync
.
Comments suppressed due to low confidence (2)
src/MudBlazor/Components/FocusTrap/MudFocusTrap.razor.cs:21
- [nitpick] Consider renaming the field
_disposed
to_isDisposed
or_hasBeenDisposed
to more explicitly convey its boolean semantics and align with common naming conventions.
private bool _disposed;
src/MudBlazor/Components/FocusTrap/MudFocusTrap.razor.cs:80
- Add unit or integration tests to simulate quick open/close scenarios, ensuring that no JS Interop calls are made after the component has been disposed to verify this new guard logic.
if (_disposed) return;
Yes, it prevents the component from crashing by preventing code being executed after the component is destroyed |
@danielchalmers @ScarletKuro This is ready to merge, super simple now. |
Thank you! |
Issue
Received above issue when opening and closing dialog fast multiple times.
Changes
disposed
check, not to start JsInterop processes in SaveFocusAsync and InitializeFocusAsync AFTER the component has already been disposed ofType of Changes
Checklist
dev
).