window move up/down loses windows

[irssibot]

• • • Issue #725 opened by @Elladan

Perform the following operation:

1. /window new
2. Alt-down or otherwise move to the original window
3. /join #foo
4. /window move up

The bottom (original) window is destroyed, and existing channel/window mappings get lost.

This has been driving me nuts for years, so I finally broke down and fixed it.

Please see attached patch to fix this bug.

[irssibot]

• • • Attachment 207 by @Elladan

irssi-window.diff

Index: src/fe-text/gui-windows.c
===================================================================
--- src/fe-text/gui-windows.c   (revision 5102)
+++ src/fe-text/gui-windows.c   (working copy)
@@ -103,31 +103,31 @@ static void gui_window_destroyed(WINDOW_
    GUI_WINDOW_REC *gui;

    g_return_if_fail(window != NULL);

    gui = WINDOW_GUI(window);
    parent = gui->parent;

    gui_window_set_unsticky(window);

    signal_emit("gui window destroyed", 1, window);

    gui_window_deinit(gui);
    window->gui_data = NULL;

    if (parent->active == window)
-       mainwindow_change_active(parent, window);
+       mainwindow_deactivate_win(parent, window);
 }

 void gui_window_resize(WINDOW_REC *window, int width, int height)
 {
    GUI_WINDOW_REC *gui;

    if (window->width == width && window->height == height)
                 return;

    gui = WINDOW_GUI(window);

    irssi_set_dirty();
         WINDOW_MAIN(window)->dirty = TRUE;

         window->width = width;
Index: src/fe-text/mainwindows.c
===================================================================
--- src/fe-text/mainwindows.c   (revision 5102)
+++ src/fe-text/mainwindows.c   (working copy)
@@ -114,65 +114,65 @@ static GSList *get_sticky_windows_sorted
    GSList *tmp, *list;

         list = NULL;
    for (tmp = windows; tmp != NULL; tmp = tmp->next) {
        WINDOW_REC *rec = tmp->data;

        if (WINDOW_GUI(rec)->sticky && WINDOW_MAIN(rec) == mainwin) {
            list = g_slist_insert_sorted(list, rec, (GCompareFunc)
                             window_refnum_cmp);
        }
    }

         return list;
 }

-void mainwindow_change_active(MAIN_WINDOW_REC *mainwin,
-                 WINDOW_REC *skip_window)
+void mainwindow_deactivate_win(MAIN_WINDOW_REC *mainwin,
+                  WINDOW_REC *deactivate)
 {
         WINDOW_REC *window, *other;
    GSList *tmp;

         mainwin->active = NULL;
    if (mainwin->sticky_windows) {
        /* sticky window */
                 tmp = get_sticky_windows_sorted(mainwin);
                 window = tmp->data;
-       if (window == skip_window) {
+       if (window == deactivate) {
            window = tmp->next == NULL ? NULL :
                tmp->next->data;
        }
                 g_slist_free(tmp);

        if (window != NULL) {
            window_set_active(window);
            return;
        }
    }

         other = NULL;
    for (tmp = windows; tmp != NULL; tmp = tmp->next) {
        WINDOW_REC *rec = tmp->data;

-       if (rec != skip_window) {
+       if (rec != deactivate && WINDOW_MAIN(rec) == mainwin) {
            other = rec;
            break;
        }
    }

    window_set_active(other);
-   if (mainwindows->next != NULL)
+   if (other == NULL && mainwindows->next != NULL)
        mainwindow_destroy(mainwin);
 }

 void mainwindows_recreate(void)
 {
    GSList *tmp;

    for (tmp = mainwindows; tmp != NULL; tmp = tmp->next) {
        MAIN_WINDOW_REC *rec = tmp->data;

        rec->screen_win = mainwindow_create_screen(rec);
                 rec->dirty = TRUE;
        textbuffer_view_set_window(WINDOW_GUI(rec->active)->view,
                       rec->screen_win);
    }
@@ -908,31 +908,31 @@ static void cmd_window_right(void)
    refnum = window_refnum_right(active_win->refnum, TRUE);
    if (refnum != -1)
        window_set_active(window_find_refnum(refnum));
 }

 static void window_reparent(WINDOW_REC *win, MAIN_WINDOW_REC *mainwin)
 {
    MAIN_WINDOW_REC *old_mainwin;

    old_mainwin = WINDOW_MAIN(win);

    if (old_mainwin != mainwin) {
        gui_window_set_unsticky(win);

        if (old_mainwin->active == win) {
-           mainwindow_change_active(old_mainwin, win);
+           mainwindow_deactivate_win(old_mainwin, win);
            if (active_mainwin == NULL) {
                active_mainwin = mainwin;
                window_set_active(mainwin->active);
            }
        }

        gui_window_reparent(win, mainwin);
        window_set_active(win);
    }
 }

 /* SYNTAX: WINDOW STICK [<ref#>] [ON|OFF] */
 static void cmd_window_stick(const char *data)
 {
         MAIN_WINDOW_REC *mainwin;
Index: src/fe-text/mainwindows.h
===================================================================
--- src/fe-text/mainwindows.h   (revision 5102)
+++ src/fe-text/mainwindows.h   (working copy)
@@ -35,26 +35,26 @@ void mainwindows_init(void);
 void mainwindows_deinit(void);

 MAIN_WINDOW_REC *mainwindow_create(void);
 void mainwindow_destroy(MAIN_WINDOW_REC *window);

 void mainwindows_redraw(void);
 void mainwindows_recreate(void);

 /* Change the window height - the height includes the lines needed for
    statusbars. If resize_lower is TRUE, the lower window is first tried
    to be resized instead of upper window. */
 void mainwindow_set_size(MAIN_WINDOW_REC *window, int height,
             int resize_lower);
 void mainwindows_resize(int width, int height);

-void mainwindow_change_active(MAIN_WINDOW_REC *mainwin,
-                 WINDOW_REC *skip_window);
+void mainwindow_deactivate_win(MAIN_WINDOW_REC *mainwin,
+                  WINDOW_REC *deactivate);

 int mainwindows_reserve_lines(int top, int bottom);
 int mainwindow_set_statusbar_lines(MAIN_WINDOW_REC *window,
                   int top, int bottom);
 void mainwindows_redraw_dirty(void);

 GSList *mainwindows_get_sorted(int reverse);

 #endif

[irssibot]

• • • Comment 1132 by @Elladan

That patch wasn't quite safe. You could get it to segfault with the following sequence:

1. Create some windows
2. /window kill in the top window
3. alt-up/down

The up/down would emit a signal, but the active_window would be NULL.

The attached (ugly) patch seems to fix it. I didn't see any crashes, and /window move up/down worked.

[irssibot]

• • • Attachment 208 by @Elladan

irssi-window2.diff

Index: mainwindows.c
===================================================================
--- mainwindows.c   (revision 5102)
+++ mainwindows.c   (working copy)
@@ -137,43 +137,60 @@ void mainwindow_change_active(MAIN_WINDO
        /* sticky window */
                 tmp = get_sticky_windows_sorted(mainwin);
                 window = tmp->data;
        if (window == skip_window) {
            window = tmp->next == NULL ? NULL :
                tmp->next->data;
        }
                 g_slist_free(tmp);

        if (window != NULL) {
            window_set_active(window);
            return;
        }
    }

+   /* Look for a window in this mainwin. */
         other = NULL;
    for (tmp = windows; tmp != NULL; tmp = tmp->next) {
        WINDOW_REC *rec = tmp->data;

-       if (rec != skip_window) {
+       if (rec != skip_window && WINDOW_MAIN(rec) == mainwin) {
            other = rec;
            break;
        }
    }

-   window_set_active(other);
-   if (mainwindows->next != NULL)
-       mainwindow_destroy(mainwin);
+   if (other != NULL) {
+       window_set_active(other);
+       if (other == NULL && mainwindows->next != NULL)
+           mainwindow_destroy(mainwin);
+   } else {
+       /* If that didn't work, look for any window. */
+       for (tmp = windows; tmp != NULL; tmp = tmp->next) {
+           WINDOW_REC *rec = tmp->data;
+
+           if (rec != skip_window) {
+               other = rec;
+               break;
+           }
+       }
+
+       window_set_active(other);
+       if (mainwindows->next != NULL)
+           mainwindow_destroy(mainwin);
+   }
 }

 void mainwindows_recreate(void)
 {
    GSList *tmp;

    for (tmp = mainwindows; tmp != NULL; tmp = tmp->next) {
        MAIN_WINDOW_REC *rec = tmp->data;

        rec->screen_win = mainwindow_create_screen(rec);
                 rec->dirty = TRUE;
        textbuffer_view_set_window(WINDOW_GUI(rec->active)->view,
                       rec->screen_win);
    }
 }