Fix handling of visibility on tty frames (Bug#76031)

* src/frame.h (FRAME_REDISPLAY_P): Remove.  Use the new function
frame_redisplay_p instead.  Extern frame_redisplay_p.
* src/frame.c (frame_redisplay_p): New function to replace
FRAME_REDISPLAY_P macro.
(make_terminal_frame): Don't tinker with frame visibility and
don't make the new frame the terminal's top frame.
(do_switch_frame): Make sure frame switched to and any of its
ancestors are visible.  Don't reset the visibility of other
frames.
(other_frames): Do not assume tty frames are by default visible.
(Fmake_frame_invisible): When making the selected tty frame
invisible, explicitly select the next visible frame.
* src/dispnew.c (Fredraw_display): Use frame_redisplay_p instead
of FRAME_REDISPLAY_P.
* src/xdisp.c (clear_garbaged_frames, echo_area_display)
(prepare_menu_bars, redisplay_internal, display_and_set_cursor)
(gui_clear_cursor): Use frame_redisplay_p instead of
FRAME_REDISPLAY_P.
* src/keyboard.c (tty_read_avail_input): When storing an event
and the selected frame is a child frame whose root is its
terminal's top frame, set the frame_or_window slot to the child
frame since otherwise the next switch frame event will select
the top frame instead.

src/dispnew.c

@@ -3240,7 +3240,7 @@ DEFUN ("redraw-display", Fredraw_display, Sredraw_display, 0, 0, "",
   Lisp_Object tail, frame;
 
   FOR_EACH_FRAME (tail, frame)
-    if (FRAME_REDISPLAY_P (XFRAME (frame)))
+    if (frame_redisplay_p (XFRAME (frame)))
       redraw_frame (XFRAME (frame));
 
   return Qnil;

src/frame.c

@@ -338,6 +338,51 @@ predicates which report frame's specific UI-related capabilities.  */)
     return type;
 }
 
+/** Return true if F can be redisplayed, that is if F is visible and, if
+    F is a tty frame, all its ancestors are visible too.  */
+bool
+frame_redisplay_p (struct frame *f)
+{
+  if (is_tty_frame (f))
+    {
+      struct frame *p = FRAME_PARENT_FRAME (f);
+      struct frame *q = NULL;
+
+      while (p)
+	{
+	  if (!p->visible)
+	    /* A tty child frame cannot be redisplayed if one of its
+	       ancestors is invisible.  */
+	    return false;
+	  else
+	    {
+	      q = p;
+	      p = FRAME_PARENT_FRAME (p);
+	    }
+	}
+
+      struct tty_display_info *tty = FRAME_TTY (f);
+      struct frame *r = XFRAME (tty->top_frame);
+
+      /* A tty child frame can be redisplayed iff its root is the top
+	 frame of its terminal.  Any other tty frame can be redisplayed
+	 iff it is the top frame of its terminal itself which must be
+	 always visible.  */
+      return (q ? q == r : f == r);
+    }
+  else
+#ifndef HAVE_X_WINDOWS
+    return FRAME_VISIBLE_P (f);
+#else
+  /* Under X, frames can continue to be displayed to the user by the
+     compositing manager even if they are invisible, so this also
+     checks whether or not the frame is reported visible by the X
+     server.  */
+  return (FRAME_VISIBLE_P (f)
+	  || (FRAME_X_P (f) && FRAME_X_VISIBLE (f)));
+#endif
+}
+
 /* Placeholder used by temacs -nw before window.el is loaded.  */
 DEFUN ("frame-windows-min-size", Fframe_windows_min_size,
        Sframe_windows_min_size, 4, 4, 0,
@@ -1407,18 +1452,6 @@ make_terminal_frame (struct terminal *terminal, Lisp_Object parent,
   FRAME_TEXT_HEIGHT (f) = FRAME_TEXT_HEIGHT (f) - FRAME_MENU_BAR_HEIGHT (f)
     - FRAME_TAB_BAR_HEIGHT (f);
 
-  /* Mark current topmost frame obscured if we make a new root frame.
-     Child frames don't completely obscure other frames.  */
-  if (NILP (parent) && FRAMEP (FRAME_TTY (f)->top_frame))
-    {
-      struct frame *top = XFRAME (FRAME_TTY (f)->top_frame);
-      struct frame *root = root_frame (top);
-      if (FRAME_LIVE_P (root))
-	SET_FRAME_VISIBLE (root, false);
-    }
-
-  /* Set the top frame to the newly created frame.  */
-  FRAME_TTY (f)->top_frame = frame;
   return f;
 }
 
@@ -1772,28 +1805,27 @@ do_switch_frame (Lisp_Object frame, int track, int for_deletion, Lisp_Object nor
       struct tty_display_info *tty = FRAME_TTY (f);
       Lisp_Object top_frame = tty->top_frame;
 
-      /* Switching to a frame on a different root frame is special.  The
+      /* When FRAME's root frame is not its terminal's top frame, make
-	 old root frame has to be marked invisible, and the new root
+	 that root frame the new top frame of FRAME's terminal.  */
-	 frame has to be made visible.  */
+      if (root_frame (f) != XFRAME (top_frame))
-      if (!EQ (frame, top_frame)
-	  && (!FRAMEP (top_frame)
-	      || root_frame (f) != root_frame (XFRAME (top_frame))))
 	{
-	  struct frame *new_root = root_frame (f);
+	  struct frame *p = FRAME_PARENT_FRAME (f);
-	  SET_FRAME_VISIBLE (new_root, true);
-	  SET_FRAME_VISIBLE (f, true);
 
-	  /* Mark previously displayed root frame as no longer
+	  XSETFRAME (top_frame, root_frame (f));
-	     visible.  */
+	  tty->top_frame = top_frame;
-	  if (FRAMEP (top_frame))
+
+	  while (p)
 	    {
-	      struct frame *top = XFRAME (top_frame);
+	      /* If FRAME is a child frame, make its ancsetors visible
-	      struct frame *old_root = root_frame (top);
+		 and garbage them ...  */
-	      if (old_root != new_root)
+	      SET_FRAME_VISIBLE (p, true);
-		SET_FRAME_VISIBLE (old_root, false);
+	      SET_FRAME_GARBAGED (p);
+	      p = FRAME_PARENT_FRAME (p);
 	    }
 
-	  tty->top_frame = frame;
+	  /* ... and FRAME itself too.  */
+	  SET_FRAME_VISIBLE (f, true);
+	  SET_FRAME_GARBAGED (f);
 
 	  /* FIXME: Why is it correct to set FrameCols/Rows here?  */
 	  if (!FRAME_PARENT_FRAME (f))
@@ -1808,10 +1840,8 @@ do_switch_frame (Lisp_Object frame, int track, int for_deletion, Lisp_Object nor
 	    }
 	}
       else
-	{
+	/* Should be covered by the condition above.  */
-	  SET_FRAME_VISIBLE (f, true);
+	SET_FRAME_VISIBLE (f, true);
-	  tty->top_frame = frame;
-	}
     }
 
   sf->select_mini_window_flag = MINI_WINDOW_P (XWINDOW (sf->selected_window));
@@ -2229,8 +2259,8 @@ DEFUN ("last-nonminibuffer-frame", Flast_nonminibuf_frame,
  * other_frames:
  *
  * Return true if there exists at least one visible or iconified frame
- * but F.  Tooltip frames do not qualify as candidates.  Return false
+ * but F.  Tooltip and child frames do not qualify as candidates.
- * if no such frame exists.
+ * Return false if no such frame exists.
  *
  * INVISIBLE true means we are called from make_frame_invisible where
  * such a frame must be visible or iconified.  INVISIBLE nil means we
@@ -2322,7 +2352,6 @@ other_frames (struct frame *f, bool invisible, bool force)
 	      /* For invisibility and normal deletions, at least one
 		 visible or iconified frame must remain (Bug#26682).  */
 	      && (FRAME_VISIBLE_P (f1)
-		  || is_tty_frame (f1)
 		  || FRAME_ICONIFIED_P (f1)
 		  || (!invisible
 		      && (force
@@ -3234,11 +3263,18 @@ displayed in the terminal.  */)
   if (FRAME_WINDOW_P (f) && FRAME_TERMINAL (f)->frame_visible_invisible_hook)
     FRAME_TERMINAL (f)->frame_visible_invisible_hook (f, false);
 
-  /* The ELisp manual says that this "usually" makes child frames
+  if (is_tty_frame (f) && EQ (frame, selected_frame))
-     invisible, too, but without saying when not.  Since users can't
+  /* On a tty if FRAME is the selected frame, we have to select another
-     rely on this, it's not implemented.  */
+    frame instead.  If FRAME is a child frame, use the first visible
-  if (is_tty_frame (f))
+    ancestor as returned by 'mru_rooted_frame'.  If FRAME is a root
-    SET_FRAME_VISIBLE (f, false);
+    frame, use the frame returned by 'next-frame' which must exist since
+    otherwise other_frames above would have lied.  */
+    Fselect_frame (FRAME_PARENT_FRAME (f)
+		   ? mru_rooted_frame (f)
+		   : next_frame (frame, make_fixnum (0)),
+		   Qnil);
+
+  SET_FRAME_VISIBLE (f, false);
 
   /* Make menu bar update for the Buffers and Frames menus.  */
   windows_or_buffers_changed = 16;

src/frame.h

@@ -1152,20 +1152,6 @@ default_pixels_per_inch_y (void)
 /* True if frame F is currently visible.  */
 #define FRAME_VISIBLE_P(f) (f)->visible
 
-/* True if frame F should be redisplayed.  This is normally the same
-   as FRAME_VISIBLE_P (f).  Under X, frames can continue to be
-   displayed to the user by the compositing manager even if they are
-   invisible, so this also checks whether or not the frame is reported
-   visible by the X server.  */
-
-#ifndef HAVE_X_WINDOWS
-#define FRAME_REDISPLAY_P(f) FRAME_VISIBLE_P (f)
-#else
-#define FRAME_REDISPLAY_P(f) (FRAME_VISIBLE_P (f)		\
-			      || (FRAME_X_P (f)			\
-				  && FRAME_X_VISIBLE (f)))
-#endif
-
 /* True if frame F is currently iconified.  */
 #define FRAME_ICONIFIED_P(f) (f)->iconified
 
@@ -1473,6 +1459,7 @@ extern struct frame *decode_live_frame (Lisp_Object);
 extern struct frame *decode_any_frame (Lisp_Object);
 extern struct frame *make_initial_frame (void);
 extern struct frame *make_frame (bool);
+extern bool frame_redisplay_p (struct frame *);
 extern int tty_child_pos_param (struct frame *, Lisp_Object,
 				Lisp_Object, int);
 extern int tty_child_size_param (struct frame *, Lisp_Object,

src/keyboard.c

@@ -8133,8 +8133,15 @@ tty_read_avail_input (struct terminal *terminal,
       buf.code = cbuf[i];
       /* Set the frame corresponding to the active tty.  Note that the
          value of selected_frame is not reliable here, redisplay tends
-         to temporarily change it.  */
+         to temporarily change it.  However, if the selected frame is a
-      buf.frame_or_window = tty->top_frame;
+         child frame, don't do that since it will cause switch frame
+         events to switch to the root frame instead.  */
+      if (FRAME_PARENT_FRAME (XFRAME (selected_frame))
+	  && (root_frame (XFRAME (selected_frame))
+	      == XFRAME (tty->top_frame)))
+	buf.frame_or_window = selected_frame;
+      else
+	buf.frame_or_window = tty->top_frame;
       buf.arg = Qnil;
 
       kbd_buffer_store_event (&buf);

src/xdisp.c

@@ -13453,7 +13453,7 @@ clear_garbaged_frames (void)
 	{
 	  struct frame *f = XFRAME (frame);
 
-	  if (FRAME_REDISPLAY_P (f) && FRAME_GARBAGED_P (f))
+	  if (frame_redisplay_p (f) && FRAME_GARBAGED_P (f))
 	    {
 	      if (f->resized_p
 		  /* It makes no sense to redraw a non-selected TTY
@@ -13507,7 +13507,7 @@ echo_area_display (bool update_frame_p)
 
   /* Don't display if frame is invisible or not yet initialized or
      if redisplay is inhibited.  */
-  if (!FRAME_REDISPLAY_P (f) || !f->glyphs_initialized_p
+  if (!frame_redisplay_p (f) || !f->glyphs_initialized_p
       || !NILP (Vinhibit_redisplay))
     return;
 
@@ -14048,7 +14048,7 @@ prepare_menu_bars (void)
 		     TTY frames to be completely redrawn, when there
 		     are more than one of them, even though nothing
 		     should be changed on display.  */
-		  || (FRAME_REDISPLAY_P (f) && FRAME_WINDOW_P (f))))
+		  || (frame_redisplay_p (f) && FRAME_WINDOW_P (f))))
 	    gui_consider_frame_title (frame);
 	}
     }
@@ -17062,8 +17062,8 @@ redisplay_internal (void)
     {
       struct frame *f = XFRAME (frame);
 
-      /* FRAME_REDISPLAY_P true basically means the frame is visible. */
+      /* frame_redisplay_p true basically means the frame is visible. */
-      if (FRAME_REDISPLAY_P (f))
+      if (frame_redisplay_p (f))
 	{
 	  ++number_of_visible_frames;
 	  /* Adjust matrices for visible frames only.  */
@@ -17206,7 +17206,7 @@ redisplay_internal (void)
       && !w->update_mode_line
       && !current_buffer->clip_changed
       && !current_buffer->prevent_redisplay_optimizations_p
-      && FRAME_REDISPLAY_P (XFRAME (w->frame))
+      && frame_redisplay_p (XFRAME (w->frame))
       && !XFRAME (w->frame)->cursor_type_changed
       && !XFRAME (w->frame)->face_change
       /* Make sure recorded data applies to current buffer, etc.  */
@@ -17467,7 +17467,7 @@ redisplay_internal (void)
 	  if (is_tty_frame (f))
 	    {
 	      /* Ignore all invisble tty frames, children or root.  */
-	      if (!FRAME_VISIBLE_P (root_frame (f)))
+	      if (!frame_redisplay_p (f))
 		continue;
 
 	      /* Remember tty root frames which we've seen.  */
@@ -17498,7 +17498,7 @@ redisplay_internal (void)
 	      if (gcscrollbars && FRAME_TERMINAL (f)->condemn_scroll_bars_hook)
 		FRAME_TERMINAL (f)->condemn_scroll_bars_hook (f);
 
-	      if (FRAME_REDISPLAY_P (f))
+	      if (frame_redisplay_p (f))
 		{
 		  /* Don't allow freeing images and faces for this
 		     frame as long as the frame's update wasn't
@@ -17524,7 +17524,7 @@ redisplay_internal (void)
 	      if (gcscrollbars && FRAME_TERMINAL (f)->judge_scroll_bars_hook)
 		FRAME_TERMINAL (f)->judge_scroll_bars_hook (f);
 
-	      if (FRAME_REDISPLAY_P (f))
+	      if (frame_redisplay_p (f))
 		{
 		  /* If fonts changed on visible frame, display again.  */
 		  if (f->fonts_changed)
@@ -17630,7 +17630,7 @@ redisplay_internal (void)
             }
 	}
     }
-  else if (FRAME_REDISPLAY_P (sf))
+  else if (frame_redisplay_p (sf))
     {
       sf->inhibit_clear_image_cache = true;
       displayed_buffer = XBUFFER (XWINDOW (selected_window)->contents);
@@ -17681,7 +17681,7 @@ redisplay_internal (void)
 	unrequest_sigio ();
       STOP_POLLING;
 
-      if (FRAME_REDISPLAY_P (sf))
+      if (frame_redisplay_p (sf))
 	{
           if (hscroll_retries <= MAX_HSCROLL_RETRIES
               && hscroll_windows (selected_window))
@@ -17763,7 +17763,7 @@ redisplay_internal (void)
 
   FOR_EACH_FRAME (tail, frame)
     {
-      if (FRAME_REDISPLAY_P (XFRAME (frame)))
+      if (frame_redisplay_p (XFRAME (frame)))
 	new_count++;
     }
 
@@ -34268,7 +34268,7 @@ display_and_set_cursor (struct window *w, bool on,
      windows and frames; in the latter case, the frame or window may
      be in the midst of changing its size, and x and y may be off the
      window.  */
-  if (! FRAME_REDISPLAY_P (f)
+  if (! frame_redisplay_p (f)
       || vpos >= w->current_matrix->nrows
       || hpos >= w->current_matrix->matrix_w)
     return;
@@ -34436,7 +34436,7 @@ gui_update_cursor (struct frame *f, bool on_p)
 void
 gui_clear_cursor (struct window *w)
 {
-  if (FRAME_REDISPLAY_P (XFRAME (w->frame)) && w->phys_cursor_on_p)
+  if (frame_redisplay_p (XFRAME (w->frame)) && w->phys_cursor_on_p)
     update_window_cursor (w, false);
 }
 

src/xterm.h

@@ -1494,7 +1494,7 @@ extern void x_mark_frame_dirty (struct frame *f);
 #define FRAME_X_VISUAL_INFO(f) (&FRAME_DISPLAY_INFO (f)->visual_info)
 
 /* Whether or not the frame is visible.  Do not test this alone.
-   Instead, use FRAME_REDISPLAY_P.  */
+   Instead, use frame_redisplay_p.  */
 #define FRAME_X_VISIBLE(f) (FRAME_X_OUTPUT (f)->visibility_state	\
 			    != VisibilityFullyObscured)