Skip to content

Fix context menu crash after split pane submenu #20612

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.

Sign up for GitHub

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

Open
OmarMashal (OmarMashal0) wants to merge 2 commits into
base: main
Choose a base branch
from
+51 −37
Open

Fix context menu crash after split pane submenu #20612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33aa726
Fix context menu crash after split pane submenu
OmarMashal0 Aug 23, 2026
96db85f
Fix context menu crash after split pane submenu
OmarMashal0 Sep 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@
<data name="SplitPaneToolTipText" xml:space="preserve">
<value>Right click for split directions - right/down/up/left</value>
</data>
<data name="SplitPaneAutomaticText" xml:space="preserve">
<value>Automatic</value>
<comment>An option in the Split pane context menu that automatically chooses the split direction.</comment>
</data>
<data name="SplitPaneDownText" xml:space="preserve">
<value>Split pane down</value>
</data>
Expand Down
84 changes: 47 additions & 37 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5584,33 +5584,28 @@ namespace winrt::TerminalApp::implementation
targetMenu.SecondaryCommands().Append(button);
};

auto makeContextItem = [&makeCallback](const winrt::hstring& label,
const winrt::hstring& icon,
const winrt::hstring& tooltip,
const auto& action,
const auto& subMenu,
auto& targetMenu) {
AppBarButton button{};
auto makeMenuFlyoutItem = [&makeCallback](const winrt::hstring& label,

@lhecker Leonard Hecker (lhecker) Sep 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Your AI wrote this PR, right? Can you ask it to minimize the diff please? It's a bug fix after all. 🙂

@OmarMashal0 OmarMashal (OmarMashal0) Sep 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the feedback, I agree the diff was a bit large for a bug fix
I had an idea about what was causing the crash and used AI to help me implement it, then reviewed and tested the changes carefully

I tried a few ways to reduce the production changes, but the nested CommandBarFlyouts need to become MenuFlyouts, so some of the code has to change because the APIs are different. I mainly reduced the diff by removing the extra structural test
If you see a smaller or cleaner approach, I’d really appreciate your guidance. I’d be happy to update it again if you have any suggestions

const winrt::hstring& icon,
const auto& action,
auto& targetMenu) {
WUX::Controls::MenuFlyoutItem item{};

if (!icon.empty())
{
auto iconElement = UI::IconPathConverter::IconWUX(icon);
Automation::AutomationProperties::SetAccessibilityView(iconElement, Automation::Peers::AccessibilityView::Raw);
button.Icon(iconElement);
item.Icon(iconElement);
}

button.Label(label);
button.Click(makeCallback(action));
WUX::Controls::ToolTipService::SetToolTip(button, box_value(tooltip));
button.ContextFlyout(subMenu);
targetMenu.SecondaryCommands().Append(button);
item.Text(label);
item.Click(makeCallback(action));
targetMenu.Items().Append(item);
};

const auto focusedProfile = _GetFocusedTabImpl()->GetFocusedProfile();
auto separatorItem = AppBarSeparator{};
auto activeProfiles = _settings.ActiveProfiles();
auto activeProfileCount = gsl::narrow_cast<int>(activeProfiles.Size());
MUX::Controls::CommandBarFlyout splitPaneMenu{};
WUX::Controls::MenuFlyout splitPaneMenu{};

// Wire up each item to the action that should be performed. By actually
// connecting these to actions, we ensure the implementation is
Expand All @@ -5627,20 +5622,41 @@ namespace winrt::TerminalApp::implementation
const auto splitPaneDownText = RS_(L"SplitPaneDownText");
const auto splitPaneUpText = RS_(L"SplitPaneUpText");
const auto splitPaneLeftText = RS_(L"SplitPaneLeftText");
const auto splitPaneToolTipText = RS_(L"SplitPaneToolTipText");
const auto splitPaneAutomaticText = RS_(L"SplitPaneAutomaticText");

auto makeSplitSubMenu = [&](const winrt::hstring& label,
const winrt::hstring& icon,
const SplitType splitType,
const NewTerminalArgs& args) {
WUX::Controls::MenuFlyoutSubItem subMenu{};

if (!icon.empty())
{
auto iconElement = UI::IconPathConverter::IconWUX(icon);
Automation::AutomationProperties::SetAccessibilityView(iconElement, Automation::Peers::AccessibilityView::Raw);
subMenu.Icon(iconElement);
}

MUX::Controls::CommandBarFlyout splitPaneContextMenu{};
makeItem(splitPaneRightText, focusedProfileIcon, ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ SplitType::Duplicate, SplitDirection::Right, .5, nullptr } }, splitPaneContextMenu);
makeItem(splitPaneDownText, focusedProfileIcon, ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ SplitType::Duplicate, SplitDirection::Down, .5, nullptr } }, splitPaneContextMenu);
makeItem(splitPaneUpText, focusedProfileIcon, ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ SplitType::Duplicate, SplitDirection::Up, .5, nullptr } }, splitPaneContextMenu);
makeItem(splitPaneLeftText, focusedProfileIcon, ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ SplitType::Duplicate, SplitDirection::Left, .5, nullptr } }, splitPaneContextMenu);
subMenu.Text(label);

makeContextItem(splitPaneDuplicateText, focusedProfileIcon, splitPaneToolTipText, ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ SplitType::Duplicate, SplitDirection::Automatic, .5, nullptr } }, splitPaneContextMenu, splitPaneMenu);
// A MenuFlyoutSubItem can't be clicked, so the automatic split is an explicit entry.
WUX::Controls::MenuFlyoutItem autoItem{};
autoItem.Text(splitPaneAutomaticText);
autoItem.Click(makeCallback(ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ splitType, SplitDirection::Automatic, .5, args } }));
subMenu.Items().Append(autoItem);
subMenu.Items().Append(WUX::Controls::MenuFlyoutSeparator{});

// add menu separator
const auto separatorAutoItem = AppBarSeparator{};
makeMenuFlyoutItem(splitPaneRightText, L"", ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ splitType, SplitDirection::Right, .5, args } }, subMenu);
makeMenuFlyoutItem(splitPaneDownText, L"", ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ splitType, SplitDirection::Down, .5, args } }, subMenu);
makeMenuFlyoutItem(splitPaneUpText, L"", ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ splitType, SplitDirection::Up, .5, args } }, subMenu);
makeMenuFlyoutItem(splitPaneLeftText, L"", ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ splitType, SplitDirection::Left, .5, args } }, subMenu);

splitPaneMenu.SecondaryCommands().Append(separatorAutoItem);
splitPaneMenu.Items().Append(subMenu);
};

makeSplitSubMenu(splitPaneDuplicateText, focusedProfileIcon, SplitType::Duplicate, nullptr);

splitPaneMenu.Items().Append(WUX::Controls::MenuFlyoutSeparator{});

for (auto profileIndex = 0; profileIndex < activeProfileCount; profileIndex++)
{
Expand All @@ -5651,21 +5667,15 @@ namespace winrt::TerminalApp::implementation
NewTerminalArgs args{};
args.Profile(profileName);

MUX::Controls::CommandBarFlyout splitPaneContextMenu{};
makeItem(splitPaneRightText, profileIcon, ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ SplitType::Manual, SplitDirection::Right, .5, args } }, splitPaneContextMenu);
makeItem(splitPaneDownText, profileIcon, ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ SplitType::Manual, SplitDirection::Down, .5, args } }, splitPaneContextMenu);
makeItem(splitPaneUpText, profileIcon, ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ SplitType::Manual, SplitDirection::Up, .5, args } }, splitPaneContextMenu);
makeItem(splitPaneLeftText, profileIcon, ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ SplitType::Manual, SplitDirection::Left, .5, args } }, splitPaneContextMenu);

makeContextItem(profileName, profileIcon, splitPaneToolTipText, ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ SplitType::Manual, SplitDirection::Automatic, .5, args } }, splitPaneContextMenu, splitPaneMenu);
makeSplitSubMenu(profileName, profileIcon, SplitType::Manual, args);
}

makeMenuItem(RS_(L"SplitPaneText"), L"\xF246", splitPaneMenu, menu);

// Only wire up "Close Pane" if there's multiple panes.
if (_GetFocusedTabImpl()->GetLeafPaneCount() > 1)
{
MUX::Controls::CommandBarFlyout swapPaneMenu{};
WUX::Controls::MenuFlyout swapPaneMenu{};
const auto rootPane = _GetFocusedTabImpl()->GetRootPane();
const auto mruPanes = _GetFocusedTabImpl()->GetMruPanes();
auto activePane = _GetFocusedTabImpl()->GetActivePane();
Expand All @@ -5681,22 +5691,22 @@ namespace winrt::TerminalApp::implementation

if (auto neighbor = rootPane->NavigateDirection(activePane, FocusDirection::Down, mruPanes))
{
makeItem(RS_(L"SwapPaneDownText"), neighbor->GetProfile().Icon().Resolved(), ActionAndArgs{ ShortcutAction::SwapPane, SwapPaneArgs{ FocusDirection::Down } }, swapPaneMenu);
makeMenuFlyoutItem(RS_(L"SwapPaneDownText"), neighbor->GetProfile().Icon().Resolved(), ActionAndArgs{ ShortcutAction::SwapPane, SwapPaneArgs{ FocusDirection::Down } }, swapPaneMenu);
}

if (auto neighbor = rootPane->NavigateDirection(activePane, FocusDirection::Right, mruPanes))
{
makeItem(RS_(L"SwapPaneRightText"), neighbor->GetProfile().Icon().Resolved(), ActionAndArgs{ ShortcutAction::SwapPane, SwapPaneArgs{ FocusDirection::Right } }, swapPaneMenu);
makeMenuFlyoutItem(RS_(L"SwapPaneRightText"), neighbor->GetProfile().Icon().Resolved(), ActionAndArgs{ ShortcutAction::SwapPane, SwapPaneArgs{ FocusDirection::Right } }, swapPaneMenu);
}

if (auto neighbor = rootPane->NavigateDirection(activePane, FocusDirection::Up, mruPanes))
{
makeItem(RS_(L"SwapPaneUpText"), neighbor->GetProfile().Icon().Resolved(), ActionAndArgs{ ShortcutAction::SwapPane, SwapPaneArgs{ FocusDirection::Up } }, swapPaneMenu);
makeMenuFlyoutItem(RS_(L"SwapPaneUpText"), neighbor->GetProfile().Icon().Resolved(), ActionAndArgs{ ShortcutAction::SwapPane, SwapPaneArgs{ FocusDirection::Up } }, swapPaneMenu);
}

if (auto neighbor = rootPane->NavigateDirection(activePane, FocusDirection::Left, mruPanes))
{
makeItem(RS_(L"SwapPaneLeftText"), neighbor->GetProfile().Icon().Resolved(), ActionAndArgs{ ShortcutAction::SwapPane, SwapPaneArgs{ FocusDirection::Left } }, swapPaneMenu);
makeMenuFlyoutItem(RS_(L"SwapPaneLeftText"), neighbor->GetProfile().Icon().Resolved(), ActionAndArgs{ ShortcutAction::SwapPane, SwapPaneArgs{ FocusDirection::Left } }, swapPaneMenu);
}

makeMenuItem(RS_(L"SwapPaneText"), L"\xF1CB", swapPaneMenu, menu);
Expand Down