Re: assertion error "PRI(owl) >= priority" otp.c line 248

=== modified file 'event_handlers.c'
--- event_handlers.c	2018-11-19 06:15:52 +0000
+++ event_handlers.c	2019-07-11 02:13:12 +0000
@@ -367,35 +367,36 @@
 static void
 HandleFocusIn(void)
 {
-#ifdef EWMH
-	TwmWindow *old_focus = Scr->Focus;
-#endif
-
 	if(! Tmp_win->wmhints->input) {
 		return;
 	}
 	if(Scr->Focus == Tmp_win) {
 		return;
 	}
+
+#ifdef EWMH
+	// Handle focus-dependent re-stacking of what we're moving out of.
+	if(Scr->Focus && OtpIsFocusDependent(Scr->Focus)) {
+		OtpUnfocusWindow(Scr->Focus);
+		// NULL's Scr->Focus
+	}
+#endif
+
 	if(Tmp_win->AutoSqueeze && Tmp_win->squeezed) {
 		AutoSqueeze(Tmp_win);
 	}
 	SetFocusVisualAttributes(Tmp_win, true);
 
+#ifdef EWMH
+	// Handle focus-dependent re-stacking of what we're moving in to.
+	if(Tmp_win && OtpIsFocusDependent(Tmp_win)) {
+		OtpFocusWindow(Tmp_win);
+		// Sets Scr->Focus
+	}
+#endif
+
+	// Redundant in EWMH case
 	Scr->Focus = Tmp_win;
-
-#ifdef EWMH
-	/*
-	 * Some EWMH flags may affect stacking, so after we change
-	 * Scr->Focus...
-	 */
-	if(old_focus && OtpIsFocusDependent(old_focus)) {
-		OtpRestackWindow(old_focus);
-	}
-	if(Tmp_win && OtpIsFocusDependent(Tmp_win)) {
-		OtpRestackWindow(Tmp_win);
-	}
-#endif
 }
 
 
@@ -413,18 +414,20 @@
 	}
 	SetFocusVisualAttributes(Tmp_win, false);
 
-	Scr->Focus = NULL;
-
 #ifdef EWMH
 	/*
 	 * X-ref HandleFocusIn() comment.  FocusOut is only leaving a window,
 	 * not entering a new one, so there's only one we may need to
 	 * restack.
 	 */
-	if(Tmp_win && OtpIsFocusDependent(Tmp_win)) {
-		OtpRestackWindow(Tmp_win);
+	if(Scr->Focus && OtpIsFocusDependent(Scr->Focus)) {
+		OtpUnfocusWindow(Scr->Focus);
+		// NULL's Scr->Focus
 	}
 #endif
+
+	// Redundant in EWMH case
+	Scr->Focus = NULL;
 }
 
 

=== modified file 'otp.c'
--- otp.c	2018-08-19 22:50:13 +0000
+++ otp.c	2019-07-07 04:44:45 +0000
@@ -1579,6 +1579,99 @@
 }
 
 
+
+/**
+ * Focus/unfocus backend.  This is used on windows whose stacking is
+ * focus-dependent (e.g., EWMH fullscreen), to move them and their
+ * transients around.  For these windows, getting/losing focus is
+ * practically the same as a f.setpriority, except it's on the calculated
+ * rather than the base parts.  And it's hard to re-use our existing
+ * functions to do it because we have to move Scr->Focus before the main
+ * window changes, but then it's too late to see where all the transients
+ * were.
+ *
+ * There are a number of unpleasant assumptions in here relating to where
+ * the transients are, and IWBNI we could be smarter and quicker about
+ * dealing with them.  But this gets us past the simple to cause
+ * assertion failures, anyway...
+ */
+static void
+OtpFocusWindowBE(TwmWindow *twm_win, int oldprio)
+{
+	OtpWinList *owl = twm_win->otp;
+
+	// This one comes off the list, and goes back in its new place.
+	RemoveOwl(owl);
+	InsertOwl(owl, Above);
+
+	// Now root around for any transients of it, and
+	// nudge them into the new location.  The whole Above/Below thing is
+	// kinda a heavy-handed guess, but...
+	//
+	// This is nearly a reimplementation of TryToMoveTransientsOfTo(),
+	// but the assumption that we can find the transients by starting
+	// from where the old priority was in the list turns out to be deeply
+	// broken.  So just walk the whole thing.  Which isn't ideal, but...
+	//
+	// XXX It should not be this freakin' hard to find a window's
+	// transients.  We should fix that more globally.
+	OtpWinList *trans = Scr->bottomOwl;
+	while((trans != NULL)) {
+		// Gotta pre-stash, since we're sometimes about to move trans.
+		OtpWinList *next = trans->above;
+
+		if((trans->type == WinWin)
+				&& isTransientOf(trans->twm_win, twm_win)) {
+			// Bounce this one
+			RemoveOwl(trans);
+			InsertOwl(trans, Above);
+		}
+
+		trans = next;
+	}
+
+	OtpCheckConsistency();
+}
+
+/**
+ * Unfocus a window.  This needs to know internals of OTP because of
+ * focus-dependent stacking of it and its transients.
+ */
+void
+OtpUnfocusWindow(TwmWindow *twm_win)
+{
+	// Stash where it currently appears to be.  We assume all its
+	// transients currently have the same effective priority.  See also
+	// TryToMoveTransientsOfTo() which makes the same assumption.  I'm
+	// not sure that's entirely warranted...
+	int oldprio = PRI(twm_win->otp);
+
+	// Now tell ourselves it's unfocused
+	assert(Scr->Focus == twm_win);
+	Scr->Focus = NULL;
+
+	// And do the work
+	OtpFocusWindowBE(twm_win, oldprio);
+}
+
+/**
+ * Focus a window.  This needs to know internals of OTP because of
+ * focus-dependent stacking of it and its transients.
+ */
+void
+OtpFocusWindow(TwmWindow *twm_win)
+{
+	// X-ref OtoUnfocusWindow() comments.
+	int oldprio = PRI(twm_win->otp);
+
+	assert(Scr->Focus != twm_win);
+	Scr->Focus = twm_win;
+
+	OtpFocusWindowBE(twm_win, oldprio);
+}
+
+
+
 /*
  * Calculating effective priority.  Take the base priority (what gets
  * set/altered by various OTP config and functions), and then tack on

=== modified file 'otp.h'
--- otp.h	2017-06-01 15:51:31 +0000
+++ otp.h	2019-07-07 04:42:48 +0000
@@ -61,6 +61,9 @@
 void OtpStashAflagsFirstTime(TwmWindow *twm_win);
 void OtpRestackWindow(TwmWindow *twm_win);
 
+void OtpUnfocusWindow(TwmWindow *twm_win);
+void OtpFocusWindow(TwmWindow *twm_win);
+
 /* functions to manage the preferences. The second arg specifies icon prefs */
 void OtpScrInitData(ScreenInfo *);
 name_list **OtpScrSwitchingL(ScreenInfo *, WinType);

=== modified file 'win_ops.c'
--- win_ops.c	2018-11-18 21:09:02 +0000
+++ win_ops.c	2019-07-07 04:42:48 +0000
@@ -129,9 +129,6 @@
 {
 	Window w = (tmp_win ? tmp_win->w : PointerRoot);
 	bool f_iconmgr = false;
-#ifdef EWMH
-	TwmWindow *old_focus = Scr->Focus;
-#endif
 
 	if(Scr->Focus && (Scr->Focus->isiconmgr)) {
 		f_iconmgr = true;
@@ -153,24 +150,34 @@
 			AutoSqueeze(Scr->Focus);
 		}
 		SetFocusVisualAttributes(Scr->Focus, false);
+#ifdef EWMH
+		// Priority may change when focus does
+		if(OtpIsFocusDependent(Scr->Focus)) {
+			OtpUnfocusWindow(Scr->Focus);
+			// That Scr->Focus = NULL's internally for us, but we don't
+			// care, since we're about to reset it if we need to.
+		}
+#endif
 	}
-	if(tmp_win)    {
+
+	if(tmp_win) {
 		if(tmp_win->AutoSqueeze && tmp_win->squeezed) {
 			AutoSqueeze(tmp_win);
 		}
 		SetFocusVisualAttributes(tmp_win, true);
+#ifdef EWMH
+		// Priority may change when focus does
+		if(OtpIsFocusDependent(tmp_win)) {
+			OtpFocusWindow(tmp_win);
+			// Pre-sets Scr->Focus
+		}
+#endif
 	}
+
+	// in the EWMH cases, this was already done.
 	Scr->Focus = tmp_win;
 
-#ifdef EWMH
-	/* Priority may change when focus does */
-	if(old_focus && OtpIsFocusDependent(old_focus)) {
-		OtpRestackWindow(old_focus);
-	}
-	if(tmp_win && OtpIsFocusDependent(tmp_win)) {
-		OtpRestackWindow(tmp_win);
-	}
-#endif
+	return;
 }