Fix crash when closing last tab #892
Conversation
This commit fixes crashes that happen when the last tab is being closed. Commit bc69d1a adds code that closes the window when the last tab is being closed, but code that should only be executed when closing one of multiple open tabs continues to be executed. This leads to App::_RemoveTabViewItem trying to call TabView::SelectedIndex(1) when there is only 1 tab remaining in the TabView, resulting in an argument exception. Signed-off-by: Fred Miller <fghzxm@outlook.com>
Signed-off-by: Fred Miller <fghzxm@outlook.com>
Signed-off-by: Fred Miller <fghzxm@outlook.com>
Signed-off-by: Fred Miller <fghzxm@outlook.com>
| _tabView.Items().RemoveAt(tabIndexFromControl); | ||
|
|
||
| // ensure tabs and focus is sync | ||
| _tabView.SelectedIndex(tabIndexFromControl > 0 ? tabIndexFromControl - 1 : 0); |
There was a problem hiding this comment.
This got removed. But I'm noticing it's similar to line 863. Should we be removing this line and updating/replacing line 863 with this?
There was a problem hiding this comment.
From my experiments, line 867 is unnecessary. The SelectedIndex property is maintained correctly by the TabView component when a tab is closed.Fixed in f8efdb9. After further investigation I came to believe that issue #708 is not from a bug in Terminal, (which means PR #737 probably did not actually fix it,) but from one in MUX.Controls.TabView, namely it failing to maintain its SelectedIndex property in the case of one or more TabViewItems are being removed from it. I'm undoing f8efdb9 and instead opening an issue at MUX. Meanwhile unresolving.
| { | ||
| _tabView.SelectedIndex(removingTabIndex > 0 ? removingTabIndex - 1 : 1); | ||
| } | ||
| _tabView.Items().RemoveAt(removingTabIndex); |
There was a problem hiding this comment.
Should this be moved down with line 869 like it was before? Or does closing the window (line 855) basically handle the logic of removing the XAML TabView Control?
This commit restores `App::_RemoveTabViewItem`'s behavior of removing the last Tab control from the TabView when the tab being closed is the last remaining, and additionally destructs the TabView control as suggested (microsoft#892 (comment)). Also restructures the function to handle 2 circumstances (whether the tab being closed is the last one or not) explicitly. Signed-off-by: Fred Miller <fghzxm@outlook.com>
Suggested at microsoft#892 (comment). Signed-off-by: Fred Miller <fghzxm@outlook.com>
ghost
added
the
Needs-Author-Feedback
microsoft#892 (review) Signed-off-by: Fred Miller <fghzxm@outlook.com>
ghost
removed
the
Needs-Author-Feedback
microsoft#892 (comment) Signed-off-by: Fred Miller <fghzxm@outlook.com>
|
I'm closing this PR for now since PR #818 seems to obsolete this. However I'm still monitoring microsoft/microsoft-ui-xaml#629 which I believe very likely contains the bug of |
Summary of the Pull Request
This commit fixes crashes that happen when the last tab is being closed.
References
None.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Commit bc69d1a adds code that closes
the window when the last tab is being closed, but code that should only
be executed when closing one of multiple open tabs continues to be
executed. This leads to App::_RemoveTabViewItem trying to call
TabView::SelectedIndex(1) when there is only 1 tab remaining in the
TabView, resulting in an argument exception.