mirror of
https://github.com/microsoft/terminal.git
synced 2025-12-19 18:11:39 -05:00
Bounds check some tab GetAt()s (#16016)
`GetAt` can throw if the index is out of range. We don't check that in
some places. This fixes some of those.
I don't think this will take care of #15689, but it might help?
(cherry picked from commit 5aadddaea9)
Service-Card-Id: 90731980
Service-Version: 1.18
This commit is contained in:
committed by
Dustin L. Howett
parent
daf03bae02
commit
32b6220703
@@ -882,7 +882,11 @@
|
|||||||
</data>
|
</data>
|
||||||
<data name="TerminalPage_PaneMovedAnnouncement_ExistingWindow" xml:space="preserve">
|
<data name="TerminalPage_PaneMovedAnnouncement_ExistingWindow" xml:space="preserve">
|
||||||
<value>Active pane moved to "{0}" tab in "{1}" window</value>
|
<value>Active pane moved to "{0}" tab in "{1}" window</value>
|
||||||
<comment>{Locked="{0}"}{Locked="{1}"}This text is read out by screen readers upon a successful pane movement. {0} is the name of the tab the pane was moved to. {1} is the name of the window the pane was moved to.</comment>
|
<comment>{Locked="{0}"}{Locked="{1}"}This text is read out by screen readers upon a successful pane movement. {0} is the name of the tab the pane was moved to. {1} is the name of the window the pane was moved to. Replaced in 1.19 by TerminalPage_PaneMovedAnnouncement_ExistingWindow2</comment>
|
||||||
|
</data>
|
||||||
|
<data name="TerminalPage_PaneMovedAnnouncement_ExistingWindow2" xml:space="preserve">
|
||||||
|
<value>Active pane moved to "{0}" window</value>
|
||||||
|
<comment>{Locked="{0}"}This text is read out by screen readers upon a successful pane movement. {0} is the name of the window the pane was moved to.</comment>
|
||||||
</data>
|
</data>
|
||||||
<data name="TerminalPage_PaneMovedAnnouncement_NewWindow" xml:space="preserve">
|
<data name="TerminalPage_PaneMovedAnnouncement_NewWindow" xml:space="preserve">
|
||||||
<value>Active pane moved to new window</value>
|
<value>Active pane moved to new window</value>
|
||||||
|
|||||||
@@ -498,7 +498,7 @@ namespace winrt::TerminalApp::implementation
|
|||||||
// * B (tabIndex=1): We'll want to focus tab A (now in index 0)
|
// * B (tabIndex=1): We'll want to focus tab A (now in index 0)
|
||||||
// * C (tabIndex=2): We'll want to focus tab B (now in index 1)
|
// * C (tabIndex=2): We'll want to focus tab B (now in index 1)
|
||||||
// * D (tabIndex=3): We'll want to focus tab C (now in index 2)
|
// * D (tabIndex=3): We'll want to focus tab C (now in index 2)
|
||||||
const auto newSelectedIndex = std::clamp<int32_t>(tabIndex - 1, 0, _tabs.Size());
|
const auto newSelectedIndex = std::clamp<int32_t>(tabIndex - 1, 0, _tabs.Size() - 1);
|
||||||
// _UpdatedSelectedTab will do the work of setting up the new tab as
|
// _UpdatedSelectedTab will do the work of setting up the new tab as
|
||||||
// the focused one, and unfocusing all the others.
|
// the focused one, and unfocusing all the others.
|
||||||
auto newSelectedTab{ _tabs.GetAt(newSelectedIndex) };
|
auto newSelectedTab{ _tabs.GetAt(newSelectedIndex) };
|
||||||
@@ -665,7 +665,7 @@ namespace winrt::TerminalApp::implementation
|
|||||||
{
|
{
|
||||||
uint32_t tabIndexFromControl{};
|
uint32_t tabIndexFromControl{};
|
||||||
const auto items{ _tabView.TabItems() };
|
const auto items{ _tabView.TabItems() };
|
||||||
if (items.IndexOf(tabViewItem, tabIndexFromControl))
|
if (items.IndexOf(tabViewItem, tabIndexFromControl) && tabIndexFromControl < _tabs.Size())
|
||||||
{
|
{
|
||||||
// If IndexOf returns true, we've actually got an index
|
// If IndexOf returns true, we've actually got an index
|
||||||
return _tabs.GetAt(tabIndexFromControl);
|
return _tabs.GetAt(tabIndexFromControl);
|
||||||
@@ -1022,7 +1022,8 @@ namespace winrt::TerminalApp::implementation
|
|||||||
// - suggestedNewTabIndex: the new index of the tab, might get clamped to fit int the tabs row boundaries
|
// - suggestedNewTabIndex: the new index of the tab, might get clamped to fit int the tabs row boundaries
|
||||||
// Return Value:
|
// Return Value:
|
||||||
// - <none>
|
// - <none>
|
||||||
void TerminalPage::_TryMoveTab(const uint32_t currentTabIndex, const int32_t suggestedNewTabIndex)
|
void TerminalPage::_TryMoveTab(const uint32_t currentTabIndex,
|
||||||
|
const int32_t suggestedNewTabIndex)
|
||||||
{
|
{
|
||||||
auto newTabIndex = gsl::narrow_cast<uint32_t>(std::clamp<int32_t>(suggestedNewTabIndex, 0, _tabs.Size() - 1));
|
auto newTabIndex = gsl::narrow_cast<uint32_t>(std::clamp<int32_t>(suggestedNewTabIndex, 0, _tabs.Size() - 1));
|
||||||
if (currentTabIndex != newTabIndex)
|
if (currentTabIndex != newTabIndex)
|
||||||
@@ -1064,16 +1065,21 @@ namespace winrt::TerminalApp::implementation
|
|||||||
|
|
||||||
if (from.has_value() && to.has_value() && to != from)
|
if (from.has_value() && to.has_value() && to != from)
|
||||||
{
|
{
|
||||||
auto& tabs{ _tabs };
|
try
|
||||||
auto tab = tabs.GetAt(from.value());
|
{
|
||||||
tabs.RemoveAt(from.value());
|
auto& tabs{ _tabs };
|
||||||
tabs.InsertAt(to.value(), tab);
|
auto tab = tabs.GetAt(from.value());
|
||||||
_UpdateTabIndices();
|
tabs.RemoveAt(from.value());
|
||||||
|
tabs.InsertAt(to.value(), tab);
|
||||||
|
_UpdateTabIndices();
|
||||||
|
}
|
||||||
|
CATCH_LOG();
|
||||||
}
|
}
|
||||||
|
|
||||||
_rearranging = false;
|
_rearranging = false;
|
||||||
|
|
||||||
if (to.has_value())
|
if (to.has_value() &&
|
||||||
|
*to < gsl::narrow_cast<int32_t>(TabRow().TabView().TabItems().Size()))
|
||||||
{
|
{
|
||||||
// Selecting the dropped tab
|
// Selecting the dropped tab
|
||||||
TabRow().TabView().SelectedIndex(to.value());
|
TabRow().TabView().SelectedIndex(to.value());
|
||||||
|
|||||||
@@ -2026,9 +2026,6 @@ namespace winrt::TerminalApp::implementation
|
|||||||
{
|
{
|
||||||
if (const auto pane{ terminalTab->GetActivePane() })
|
if (const auto pane{ terminalTab->GetActivePane() })
|
||||||
{
|
{
|
||||||
// Get the tab title _before_ moving things around in case the tabIdx doesn't point to the right one after the move
|
|
||||||
const auto tabTitle = _tabs.GetAt(tabIdx).Title();
|
|
||||||
|
|
||||||
auto startupActions = pane->BuildStartupActions(0, 1, true, true);
|
auto startupActions = pane->BuildStartupActions(0, 1, true, true);
|
||||||
_DetachPaneFromWindow(pane);
|
_DetachPaneFromWindow(pane);
|
||||||
_MoveContent(std::move(startupActions.args), windowId, tabIdx);
|
_MoveContent(std::move(startupActions.args), windowId, tabIdx);
|
||||||
@@ -2047,7 +2044,7 @@ namespace winrt::TerminalApp::implementation
|
|||||||
{
|
{
|
||||||
autoPeer.RaiseNotificationEvent(Automation::Peers::AutomationNotificationKind::ActionCompleted,
|
autoPeer.RaiseNotificationEvent(Automation::Peers::AutomationNotificationKind::ActionCompleted,
|
||||||
Automation::Peers::AutomationNotificationProcessing::ImportantMostRecent,
|
Automation::Peers::AutomationNotificationProcessing::ImportantMostRecent,
|
||||||
fmt::format(std::wstring_view{ RS_(L"TerminalPage_PaneMovedAnnouncement_ExistingWindow") }, tabTitle, windowId),
|
fmt::format(std::wstring_view{ RS_(L"TerminalPage_PaneMovedAnnouncement_ExistingWindow2") }, windowId),
|
||||||
L"TerminalPageMovePaneToExistingWindow" /* unique name for this notification category */);
|
L"TerminalPageMovePaneToExistingWindow" /* unique name for this notification category */);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -2064,7 +2061,7 @@ namespace winrt::TerminalApp::implementation
|
|||||||
|
|
||||||
// Moving the pane from the current tab might close it, so get the next
|
// Moving the pane from the current tab might close it, so get the next
|
||||||
// tab before its index changes.
|
// tab before its index changes.
|
||||||
if (_tabs.Size() > tabIdx)
|
if (tabIdx < _tabs.Size())
|
||||||
{
|
{
|
||||||
auto targetTab = _GetTerminalTabImpl(_tabs.GetAt(tabIdx));
|
auto targetTab = _GetTerminalTabImpl(_tabs.GetAt(tabIdx));
|
||||||
// if the selected tab is not a host of terminals (e.g. settings)
|
// if the selected tab is not a host of terminals (e.g. settings)
|
||||||
|
|||||||
Reference in New Issue
Block a user