Repository: microsoft/terminal (language: cpp)

Issue:
Reordering tabs messes up relation between tabs and panes
### Windows Terminal version

62448969b30325a75f35201427a052de0e4dbb7d

### Windows build number

10.0.25330.0

### Other Software

_No response_

### Steps to reproduce

Open multiple tabs. Reorder them.

### Expected Behavior

_No response_

### Actual Behavior

The relation/index between tabs and panes is now broken.
Selecting a tab will select a pane which does not belong to that tab. Closing panes will close the wrong tab.
See: https://www.youtube.com/watch?v=Qa9jxFnvI8U


Discussion / hints:
More concrete repro steps:

* Open two windows
* In one window, open two tabs. Pwsh and CMD are good, so they're clearly separate. 
* Reorder those two. 
* Drag the inactive one to the other window. 
* Experience having the active term core in both windows!



<hr>

~It's so obvious:~
![](https://user-images.githubusercontent.com/18356694/231828087-7a191811-9248-4ef7-970e-5575801c6b84.png)


Wait but no, `_onTabStripDrop` doesn't ever cause a `_MoveTab`. It triggers a `_RequestReceiveContentHandlers`, which is bubbled to the sender, which does a `SendContentToOther`, which does a `_MoveContent` on the dragged tab. Hmm. 
oh its so bad

Just reordering tabs, no tearout, just breaks the entire relationship of it. I wonder if adding these drag/drop handlers somehow broke the reorder events we get...

It sure did. We don't get `TabView.TabItemsChanged` anymore now that we get `TabDragStarting` & `TabStripDrop`. WELP. Let's see what we can do. 

Fix patch:
diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp
index 457cf34c1ca..51918082420 100644
--- a/src/cascadia/TerminalApp/TabManagement.cpp
+++ b/src/cascadia/TerminalApp/TabManagement.cpp
@@ -890,34 +890,6 @@ namespace winrt::TerminalApp::implementation
             co_await _HandleCloseTabRequested(tab);
         }
     }
-    // Method Description:
-    // - Responds to changes in the TabView's item list by changing the
-    //   tabview's visibility.
-    // - This method is also invoked when tabs are dragged / dropped as part of
-    //   tab reordering and this method hands that case as well in concert with
-    //   TabDragStarting and TabDragCompleted handlers that are set up in
-    //   TerminalPage::Create()
-    // Arguments:
-    // - sender: the control that originated this event
-    // - eventArgs: the event's constituent arguments
-    void TerminalPage::_OnTabItemsChanged(const IInspectable& /*sender*/, const Windows::Foundation::Collections::IVectorChangedEventArgs& eventArgs)
-    {
-        if (_rearranging)
-        {
-            if (eventArgs.CollectionChange() == Windows::Foundation::Collections::CollectionChange::ItemRemoved)
-            {
-                _rearrangeFrom = eventArgs.Index();
-            }
-
-            if (eventArgs.CollectionChange() == Windows::Foundation::Collections::CollectionChange::ItemInserted)
-            {
-                _rearrangeTo = eventArgs.Index();
-            }
-        }
-
-        CommandPalette().Visibility(Visibility::Collapsed);
-        _UpdateTabView();
-    }
 
     // Method Description:
     // - Additional responses to clicking on a TabView's item. Currently, just remove tab with middle click
@@ -1079,16 +1051,37 @@ namespace winrt::TerminalApp::implementation
     }
 
     void TerminalPage::_TabDragStarted(const IInspectable& /*sender*/,
-                                       const IInspectable& /*eventArgs*/)
+                                       const winrt::MUX::Controls::TabViewTabDragStartingEventArgs& eventArgs)
     {
         _rearranging = true;
         _rearrangeFrom = std::nullopt;
         _rearrangeTo = std::nullopt;
+
+        // Start tracking the index of the tab that is being dragged. In
+        // `_TabDragCompleted`, we'll use this to determine how to reorder our
+        // internal tabs list.
+        const auto& draggedTabViewItem{ eventArgs.Tab() };
+        uint32_t tabIndexFromControl{};
+        const auto tabItems{ _tabView.TabItems() };
+        if (tabItems.IndexOf(draggedTabViewItem, tabIndexFromControl))
+        {
+            // If IndexOf returns true, we've actually got an index
+            _rearrangeFrom = tabIndexFromControl;
+        }
     }
 
     void TerminalPage::_TabDragCompleted(const IInspectable& /*sender*/,
-                                         const IInspectable& /*eventArgs*/)
+                                         const winrt::MUX::Controls::TabViewTabDragCompletedEventArgs& eventArgs)
     {
+        const auto& draggedTabViewItem{ eventArgs.Tab() };
+
+        uint32_t tabIndexFromControl{};
+        const auto tabItems{ _tabView.TabItems() };
+        if (tabItems.IndexOf(draggedTabViewItem, tabIndexFromControl))
+        {
+            _rearrangeTo = tabIndexFromControl;
+        }
+
         auto& from{ _rearrangeFrom };
         auto& to{ _rearrangeTo };
 
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index 240ee8eca8b..fa2612c6c24 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -245,7 +245,6 @@ namespace winrt::TerminalApp::implementation
         });
         _tabView.SelectionChanged({ this, &TerminalPage::_OnTabSelectionChanged });
         _tabView.TabCloseRequested({ this, &TerminalPage::_OnTabCloseRequested });
-        _tabView.TabItemsChanged({ this, &TerminalPage::_OnTabItemsChanged });
 
         _tabView.TabDragStarting({ this, &TerminalPage::_onTabDragStarting });
         _tabView.TabStripDragOver({ this, &TerminalPage::_onTabStripDragOver });
@@ -4720,6 +4719,8 @@ namespace winrt::TerminalApp::implementation
         co_await wil::resume_foreground(Dispatcher());
         if (const auto& page{ weakThis.get() })
         {
+            // `this` is safe to use
+            //
             // First we need to get the position in the List to drop to
             auto index = -1;
 
@@ -4739,8 +4740,8 @@ namespace winrt::TerminalApp::implementation
                 }
             }
 
-            // `this` is safe to use
-            const auto request = winrt::make_self<RequestReceiveContentArgs>(src, _WindowProperties.WindowId(), index);
+            const auto myId{ _WindowProperties.WindowId() };
+            const auto request = winrt::make_self<RequestReceiveContentArgs>(src, myId, index);
 
             // This will go up to the monarch, who will then dispatch the request
             // back down to the source TerminalPage, who will then perform a
diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h
index 83b80bb633c..a775902396e 100644
--- a/src/cascadia/TerminalApp/TerminalPage.h
+++ b/src/cascadia/TerminalApp/TerminalPage.h
@@ -412,12 +412,11 @@ namespace winrt::TerminalApp::implementation
 
         fire_and_forget _LaunchSettings(const Microsoft::Terminal::Settings::Model::SettingsTarget target);
 
-        void _TabDragStarted(const IInspectable& sender, const IInspectable& eventArgs);
-        void _TabDragCompleted(const IInspectable& sender, const IInspectable& eventArgs);
+        void _TabDragStarted(const IInspectable& sender, const winrt::Microsoft::UI::Xaml::Controls::TabViewTabDragStartingEventArgs& eventArgs);
+        void _TabDragCompleted(const IInspectable& sender, const winrt::Microsoft::UI::Xaml::Controls::TabViewTabDragCompletedEventArgs& eventArgs);
 
         void _OnTabClick(const IInspectable& sender, const Windows::UI::Xaml::Input::PointerRoutedEventArgs& eventArgs);
         void _OnTabSelectionChanged(const IInspectable& sender, const Windows::UI::Xaml::Controls::SelectionChangedEventArgs& eventArgs);
-        void _OnTabItemsChanged(const IInspectable& sender, const Windows::Foundation::Collections::IVectorChangedEventArgs& eventArgs);
         void _OnTabCloseRequested(const IInspectable& sender, const Microsoft::UI::Xaml::Controls::TabViewTabCloseRequestedEventArgs& eventArgs);
         void _OnFirstLayout(const IInspectable& sender, const IInspectable& eventArgs);
         void _UpdatedSelectedTab(const winrt::TerminalApp::TabBase& tab);


Test patch: