-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Fix crash when closing scratchpad pane #20195
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -814,6 +814,23 @@ namespace winrt::TerminalApp::implementation | |
|
|
||
| if (const auto pane{ activeTab->GetActivePane() }) | ||
| { | ||
| // GH#20187: When the user closes a pane with a keyboard | ||
| // shortcut, focus moves off the closed pane while keys | ||
| // are still depressed. The corresponding KeyUp events | ||
| // arrive at TabRowControl, whose KeyUp is wired to | ||
| // TerminalPage::_KeyDownHandler (see TerminalPage.xaml). | ||
| // _KeyDownHandler can't tell it was invoked from KeyUp | ||
| // and dispatches the close action a second time. By | ||
| // then the parent's close animation in | ||
| // _CloseChildRoutine is still running, so | ||
| // Tab::_activePane still points at the now-closed leaf | ||
| // ("corpse"). Detect that here and bail before we | ||
| // dereference its null content in GetTerminalArgsForPane. | ||
|
Comment on lines
+818
to
+828
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. way too much detail |
||
| if (pane->IsClosed()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it should be sufficient to fix In general, we should not need to interrogate objects to say "are you half-dead?" because in the best case scenario they're HALF DEAD and should not be answering questions... |
||
| { | ||
| co_return; | ||
| } | ||
|
|
||
| const auto weak = get_weak(); | ||
|
|
||
| // Check if we should warn before closing a single pane | ||
|
|
||
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.
It's OK for us to leave briefer comments and not refer to github issues.
This whole thing could be...
or just no comment
(because the current comment encodes implementation details from everywhere else in the project and will be incorrect the moment somebody fixes the double-close bug)