window move up/down loses windows

[irssibot]

• • • Issue window move up/down loses windows #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);
 	}
 }