Fix for #1240248, Segfaults for :MoveTabLeft/Right

wrong code in Container.cc

4 files changed, 30 insertions, 33 deletions

diff --git a/ChangeLog b/ChangeLog
index 2119c98..2669fd3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 (Format: Year/Month/Day)
 Changes for 0.9.14:
+*05/07/25:
+    Fix for #1240248, Segfaults for :MoveTabLeft/Right (Mathias)
+    Container.cc FbWinFrame.cc Window.cc
 *05/07/20:
     Changed some *Focus options (Mathias)
     removed SloppyFocus, SemiSloppyFocus and ClickToFocus options
diff --git a/src/Container.cc b/src/Container.cc
index 3a53a4b..0c2ddd5 100644
--- a/src/Container.cc
+++ b/src/Container.cc
@@ -28,6 +28,8 @@
 #include "FbTk/EventManager.hh"
 #include "CompareWindow.hh"
 
+#include <algorithm>
+
 Container::Container(const FbTk::FbWindow &parent):
     FbTk::FbWindow(parent, 0, 0, 1, 1, ExposureMask), 
     m_align(RELATIVE),
@@ -113,35 +115,31 @@ void Container::insertItem(Item item, int pos) {
 }
 
 void Container::moveItem(Item item, int movement) {
+
     int index = find(item);
-    if (index < 0)
+    const int size = m_item_list.size();
+
+    if (index < 0 || (movement % size) == 0) {
         return;
+    }
 
-    int size = m_item_list.size();
     int newindex = (index + movement) % size;
     if (newindex < 0) // neg wrap
         newindex += size;
 
-    if (newindex > index) // one smaller now
-        --newindex;
-
-    ItemList::iterator it = m_item_list.begin();
-    for (; newindex > 0 && index > 0; ++it, --newindex, --index) {
+    ItemList::iterator it = std::find(m_item_list.begin(), 
+                                      m_item_list.end(), 
+                                      item);
+    m_item_list.erase(it);
+    
+    for (it = m_item_list.begin(); newindex >= 0; ++it, --newindex) {
         if (newindex == 0) {
-            m_item_list.insert(it, item);
-            ++newindex;
-        }
-
-        if (index == 0) {
-            m_item_list.erase(it);
-            --index;
+            break;
         }
     }
 
-            m_item_list.insert(it, item);
-    insertItem(item, newindex);
+    m_item_list.insert(it, item);
     repositionItems();
-
 }
 
 // returns true if something was done
diff --git a/src/FbWinFrame.cc b/src/FbWinFrame.cc
index 9ac2935..53d1d2c 100644
--- a/src/FbWinFrame.cc
+++ b/src/FbWinFrame.cc
@@ -372,7 +372,7 @@ void FbWinFrame::moveLabelButtonLeft(FbTk::TextButton &btn) {
 }
 
 void FbWinFrame::moveLabelButtonRight(FbTk::TextButton &btn) {
-    m_tab_container.moveItem(&btn, -1);
+    m_tab_container.moveItem(&btn, +1);
 }
 
 void FbWinFrame::moveLabelButtonTo(FbTk::TextButton &btn, int x, int y) {
@@ -394,7 +394,7 @@ void FbWinFrame::moveLabelButtonRightOf(FbTk::TextButton &btn, const FbTk::TextB
     if (pos < 0)
         return;
 
-    m_tab_container.moveItem(&btn, pos-1);
+    m_tab_container.moveItem(&btn, pos+1);
 }
 
 void FbWinFrame::setLabelButtonFocus(FbTk::TextButton &btn) {
diff --git a/src/Window.cc b/src/Window.cc
index 88690ce..db430f9 100644
--- a/src/Window.cc
+++ b/src/Window.cc
@@ -864,14 +864,12 @@ void FluxboxWindow::moveClientLeft() {
     if (m_clientlist.size() == 1 ||
         *m_clientlist.begin() == &winClient())
         return;
-    // move label button to the left
-    frame().moveLabelButtonLeft(*m_labelbuttons[&winClient()]);
+    
     // move client in clientlist to the left
-    ClientList::iterator it = find(m_clientlist.begin(), m_clientlist.end(), &winClient());
-    ClientList::iterator new_pos = it;
-    new_pos--;
-    m_clientlist.erase(it);
-    m_clientlist.insert(new_pos, &winClient());
+    ClientList::iterator oldpos = find(m_clientlist.begin(), m_clientlist.end(), &winClient());
+    ClientList::iterator newpos = oldpos; newpos--;
+    std::swap(*newpos, *oldpos);
+    frame().moveLabelButtonLeft(*m_labelbuttons[&winClient()]);
 
     updateClientLeftWindow();
 
@@ -879,15 +877,13 @@ void FluxboxWindow::moveClientLeft() {
 
 void FluxboxWindow::moveClientRight() {
     if (m_clientlist.size() == 1 ||
-            *m_clientlist.rbegin() == &winClient())
+        *m_clientlist.rbegin() == &winClient())
         return;
-    // move label button to the right
+    
+    ClientList::iterator oldpos = find(m_clientlist.begin(), m_clientlist.end(), &winClient());
+    ClientList::iterator newpos = oldpos; newpos++;
+    std::swap(*newpos, *oldpos);
     frame().moveLabelButtonRight(*m_labelbuttons[&winClient()]);
-    // move client in clientlist to the right
-    ClientList::iterator it = find(m_clientlist.begin(), m_clientlist.end(), &winClient());
-    ClientList::iterator new_pos = m_clientlist.erase(it);
-    new_pos++;
-    m_clientlist.insert(new_pos, &winClient());
 
     updateClientLeftWindow();
 }