bug#18196: 24.4.50; crash when setting face background in terminal frame

[martin rudalics <rudalics@gmx.at>]

[-- Attachment #1: Type: text/plain, Size: 2513 bytes --]

 > Everything looks good. Your patch still applies fine and fixes the face
 > background crash, and your commit fixes the terminal emacsclient display
 > issue.

OK.  This was part one of our endeavour.  We now have to discuss with
Eli on how to proceed.  The current patch has three advantages:

(1) It doesn't violate the uniform interface where all calls to
     change_frame_size and adjust_frame_size use the window's text sizes
     as arguments.

(2) It doesn't require any change in window.c.

(3) It has been tested.

It has the following disadvantages:

(1) Code changes in term.c and w32console.c from FRAME_LINES to
     FRAME_TOTAL_LINES.

(2) change_frame_size and adjust_frame_size must be explicitly called
     with FRAME_MENU_BAR_LINES subtracted after calling get_tty_size.

Alternatively, I could try to do what I mentioned earlier: Change the
definition of FRAME_TOP_MARGIN and FRAME_TOP_MARGIN_HEIGHT so that these
do not count the menubar for TTY frames.  This means that I would have
to check every use of these macros for whether the underlying system is
a TTY or not.  This solution would have the following advantages:

(1) Code in term.c and w32console.c would remain unchanged.

(2) The calls of change_frame_size and adjust_frame_size after a
     get_tty_size would remain unchanged.

Disadvantages are:

(1) Code in window.c would have to be special cased for TTY: Affected
     are Fdelete_other_windows_internal, Fwindow_resize_apply_total and
     resize_frame_windows.

(2) I don't yet know how the two uses of FRAME_TOP_MARGIN in dispnew.c
     would be affected.  I suspect that something there might be fishy
     anyway without any visible consequences but am too silly to
     understand it.

(3) The semantics of the size arguments in change_frame_size and
     adjust_frame_size would become inconsistent wrt GUI builds.  We'd
     include the menubar for TTYs and exclude them for GUIs.

(4) We'd have to specially mention TTYs in all documentations of frame
     size parameters and functions setting or querying the size of
     frames.

Personally, I think that the real show-stopper here is (4).  One major
aim of adjust_frame_size was to provide a unified concept where
functions like `frame-text-height' and `set-frame-height' conceptually
do not count menu and tool bars throughout all builds, that is, always
refer to the "same object".

In any case I once more attach the patch I posted earlier.  Mat could
you try whether it solves Bug#18161 for you?

martin

[-- Attachment #2: tty-frame-size.diff --]
[-- Type: text/plain, Size: 9594 bytes --]

=== modified file 'src/dispnew.c'
--- src/dispnew.c	2014-08-03 23:16:39 +0000
+++ src/dispnew.c	2014-08-05 09:36:54 +0000
@@ -5444,7 +5444,9 @@
           /* Record the new sizes, but don't reallocate the data
              structures now.  Let that be done later outside of the
              signal handler.  */
-          change_frame_size (XFRAME (frame), width, height, 0, 1, 0, 0);
+          change_frame_size (XFRAME (frame), width,
+			     height - FRAME_MENU_BAR_LINES (XFRAME (frame)),
+			     0, 1, 0, 0);
     }
   }
 }

=== modified file 'src/frame.c'
--- src/frame.c	2014-08-04 16:47:27 +0000
+++ src/frame.c	2014-08-06 08:45:15 +0000
@@ -215,7 +215,8 @@
 {

   return (frame_inhibit_implied_resize
-	  || !NILP (get_frame_param (f, Qfullscreen)));
+	  || !NILP (get_frame_param (f, Qfullscreen))
+	  || FRAME_TERMCAP_P (f) || FRAME_MSDOS_P (f));
 }

 #if 0
@@ -563,7 +564,7 @@
       /* MSDOS frames cannot PRETEND, as they change frame size by
 	 manipulating video hardware.  */
       if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
-	FrameRows (FRAME_TTY (f)) = new_lines;
+	FrameRows (FRAME_TTY (f)) = new_lines + FRAME_TOP_MARGIN (f);
     }

   /* Assign new sizes.  */
@@ -917,7 +918,9 @@
 #endif

   FRAME_MENU_BAR_LINES (f) = NILP (Vmenu_bar_mode) ? 0 : 1;
+  FRAME_LINES (f) = FRAME_LINES (f) - FRAME_MENU_BAR_LINES (f);
   FRAME_MENU_BAR_HEIGHT (f) = FRAME_MENU_BAR_LINES (f) * FRAME_LINE_HEIGHT (f);
+  FRAME_TEXT_HEIGHT (f) = FRAME_TEXT_HEIGHT (f) - FRAME_MENU_BAR_HEIGHT (f);

   /* Set the top frame to the newly created frame.  */
   if (FRAMEP (FRAME_TTY (f)->top_frame)
@@ -1037,7 +1040,7 @@
   {
     int width, height;
     get_tty_size (fileno (FRAME_TTY (f)->input), &width, &height);
-    adjust_frame_size (f, width, height, 5, 0);
+    adjust_frame_size (f, width, height - FRAME_MENU_BAR_LINES (f), 5, 0);
   }

   adjust_frame_glyphs (f);
@@ -1168,8 +1171,8 @@
 	     frame's data.  */
 	  if (FRAME_COLS (f) != FrameCols (tty))
 	    FrameCols (tty) = FRAME_COLS (f);
-	  if (FRAME_LINES (f) != FrameRows (tty))
-	    FrameRows (tty) = FRAME_LINES (f);
+	  if (FRAME_TOTAL_LINES (f) != FrameRows (tty))
+	    FrameRows (tty) = FRAME_TOTAL_LINES (f);
 	}
       tty->top_frame = frame;
     }
@@ -2733,7 +2736,7 @@
     return make_number (FRAME_PIXEL_HEIGHT (f));
   else
 #endif
-    return make_number (FRAME_LINES (f));
+    return make_number (FRAME_TOTAL_LINES (f));
 }

 DEFUN ("frame-pixel-width", Fframe_pixel_width,
@@ -2750,7 +2753,7 @@
     return make_number (FRAME_PIXEL_WIDTH (f));
   else
 #endif
-    return make_number (FRAME_COLS (f));
+    return make_number (FRAME_TOTAL_COLS (f));
 }

 DEFUN ("tool-bar-pixel-width", Ftool_bar_pixel_width,

=== modified file 'src/keyboard.c'
--- src/keyboard.c	2014-08-05 05:43:35 +0000
+++ src/keyboard.c	2014-08-05 09:37:28 +0000
@@ -1868,7 +1868,7 @@
 safe_run_hooks_error (Lisp_Object error, ptrdiff_t nargs, Lisp_Object *args)
 {
   Lisp_Object hook, fun, msgargs[4];
-  
+
   eassert (nargs == 2);
   hook = args[0];
   fun = args[1];
@@ -10221,8 +10221,9 @@
      with a window system; but suspend should be disabled in that case.  */
   get_tty_size (fileno (CURTTY ()->input), &width, &height);
   if (width != old_width || height != old_height)
-    change_frame_size (SELECTED_FRAME (), width, height
-		       - FRAME_MENU_BAR_LINES (SELECTED_FRAME ()), 0, 0, 0, 0);
+    change_frame_size (SELECTED_FRAME (), width,
+		       height - FRAME_MENU_BAR_LINES (SELECTED_FRAME ()),
+		       0, 0, 0, 0);

   /* Run suspend-resume-hook.  */
   hook = intern ("suspend-resume-hook");

=== modified file 'src/menu.c'
--- src/menu.c	2014-07-03 06:00:53 +0000
+++ src/menu.c	2014-08-05 09:29:29 +0000
@@ -1455,7 +1455,7 @@
 	 their upper-left corner at the given position.)  */
       if (STRINGP (prompt))
 	x_coord -= SCHARS (prompt);
-      y_coord = FRAME_LINES (f);
+      y_coord = FRAME_TOTAL_LINES (f);
     }

   XSETFRAME (frame, f);

=== modified file 'src/term.c'
--- src/term.c	2014-07-27 13:21:30 +0000
+++ src/term.c	2014-08-05 09:37:43 +0000
@@ -91,7 +91,7 @@

 #define OUTPUT(tty, a)                                          \
   emacs_tputs ((tty), a,                                        \
-               FRAME_LINES (XFRAME (selected_frame)) - curY (tty),	\
+               FRAME_TOTAL_LINES (XFRAME (selected_frame)) - curY (tty),	\
                cmputc)

 #define OUTPUT1(tty, a) emacs_tputs ((tty), a, 1, cmputc)
@@ -201,7 +201,7 @@
              off the screen, so it won't be overwritten and lost.  */
           int i;
           current_tty = tty;
-          for (i = 0; i < FRAME_LINES (XFRAME (selected_frame)); i++)
+          for (i = 0; i < FRAME_TOTAL_LINES (XFRAME (selected_frame)); i++)
             cmputc ('\n');
         }

@@ -257,7 +257,7 @@
 {
   struct tty_display_info *tty = FRAME_TTY (f);

-  tty->specified_window = size ? size : FRAME_LINES (f);
+  tty->specified_window = size ? size : FRAME_TOTAL_LINES (f);
   if (FRAME_SCROLL_REGION_OK (f))
     tty_set_scroll_region (f, 0, tty->specified_window);
 }
@@ -272,9 +272,9 @@
     buf = tparam (tty->TS_set_scroll_region, 0, 0, start, stop - 1, 0, 0);
   else if (tty->TS_set_scroll_region_1)
     buf = tparam (tty->TS_set_scroll_region_1, 0, 0,
-		  FRAME_LINES (f), start,
-		  FRAME_LINES (f) - stop,
-		  FRAME_LINES (f));
+		  FRAME_TOTAL_LINES (f), start,
+		  FRAME_TOTAL_LINES (f) - stop,
+		  FRAME_TOTAL_LINES (f));
   else
     buf = tparam (tty->TS_set_window, 0, 0, start, 0, stop, FRAME_COLS (f));

@@ -446,7 +446,7 @@
     }
   else
     {
-      for (i = curY (tty); i < FRAME_LINES (f); i++)
+      for (i = curY (tty); i < FRAME_TOTAL_LINES (f); i++)
 	{
 	  cursor_to (f, i, 0);
 	  clear_end_of_line (f, FRAME_COLS (f));
@@ -748,7 +748,7 @@
      since that would scroll the whole frame on some terminals.  */

   if (AutoWrap (tty)
-      && curY (tty) + 1 == FRAME_LINES (f)
+      && curY (tty) + 1 == FRAME_TOTAL_LINES (f)
       && (curX (tty) + len) == FRAME_COLS (f))
     len --;
   if (len <= 0)
@@ -820,7 +820,7 @@
      since that would scroll the whole frame on some terminals.  */

   if (AutoWrap (tty)
-      && curY (tty) + 1 == FRAME_LINES (f)
+      && curY (tty) + 1 == FRAME_TOTAL_LINES (f)
       && (curX (tty) + len) == FRAME_COLS (f))
     len --;
   if (len <= 0)
@@ -1009,7 +1009,7 @@
       && vpos + i >= tty->specified_window)
     return;
   if (!FRAME_MEMORY_BELOW_FRAME (f)
-      && vpos + i >= FRAME_LINES (f))
+      && vpos + i >= FRAME_TOTAL_LINES (f))
     return;

   if (multi)
@@ -1046,7 +1046,7 @@
       && FRAME_MEMORY_BELOW_FRAME (f)
       && n < 0)
     {
-      cursor_to (f, FRAME_LINES (f) + n, 0);
+      cursor_to (f, FRAME_TOTAL_LINES (f) + n, 0);
       clear_to_end (f);
     }
 }
@@ -2405,13 +2405,14 @@
 	  struct frame *f = XFRAME (t->display_info.tty->top_frame);
 	  int width, height;
 	  int old_height = FRAME_COLS (f);
-	  int old_width = FRAME_LINES (f);
+	  int old_width = FRAME_TOTAL_LINES (f);

 	  /* Check if terminal/window size has changed while the frame
 	     was suspended.  */
 	  get_tty_size (fileno (t->display_info.tty->input), &width, &height);
 	  if (width != old_width || height != old_height)
-	    change_frame_size (f, width, height - FRAME_MENU_BAR_LINES (f), 0, 0, 0, 0);
+	    change_frame_size (f, width, height - FRAME_MENU_BAR_LINES (f),
+			       0, 0, 0, 0);
 	  SET_FRAME_VISIBLE (XFRAME (t->display_info.tty->top_frame), 1);
 	}

@@ -2894,7 +2895,7 @@
   /* Don't try to display more menu items than the console can display
      using the available screen lines.  Exclude the echo area line, as
      it will be overwritten by the help-echo anyway.  */
-  int max_items = min (menu->count - first_item, FRAME_LINES (sf) - 1 - y);
+  int max_items = min (menu->count - first_item, FRAME_TOTAL_LINES (sf) - 1 - y);

   menu_help_message = NULL;

@@ -3274,7 +3275,7 @@
     {
       mi_result input_status;
       int min_y = state[0].y;
-      int max_y = min (min_y + state[0].menu->count, FRAME_LINES (sf) - 1) - 1;
+      int max_y = min (min_y + state[0].menu->count, FRAME_TOTAL_LINES (sf) - 1) - 1;

       input_status = read_menu_input (sf, &x, &y, min_y, max_y, &first_time);
       if (input_status)

=== modified file 'src/w32console.c'
--- src/w32console.c	2014-07-27 13:21:30 +0000
+++ src/w32console.c	2014-08-05 09:30:47 +0000
@@ -118,7 +118,7 @@
 w32con_clear_to_end (struct frame *f)
 {
   w32con_clear_end_of_line (f, FRAME_COLS (f) - 1);
-  w32con_ins_del_lines (f, cursor_coords.Y, FRAME_LINES (f) - cursor_coords.Y - 1);
+  w32con_ins_del_lines (f, cursor_coords.Y, FRAME_TOTAL_LINES (f) - cursor_coords.Y - 1);
 }

 /* Clear the frame.  */
@@ -133,7 +133,7 @@
   GetConsoleScreenBufferInfo (GetStdHandle (STD_OUTPUT_HANDLE), &info);

   /* Remember that the screen buffer might be wider than the window.  */
-  n = FRAME_LINES (f) * info.dwSize.X;
+  n = FRAME_TOTAL_LINES (f) * info.dwSize.X;
   dest.X = dest.Y = 0;

   FillConsoleOutputAttribute (cur_screen, char_attr_normal, n, dest, &r);
@@ -175,18 +175,18 @@
   if (n < 0)
     {
       scroll.Top = vpos - n;
-      scroll.Bottom = FRAME_LINES (f);
+      scroll.Bottom = FRAME_TOTAL_LINES (f);
       dest.Y = vpos;
     }
   else
     {
       scroll.Top = vpos;
-      scroll.Bottom = FRAME_LINES (f) - n;
+      scroll.Bottom = FRAME_TOTAL_LINES (f) - n;
       dest.Y = vpos + n;
     }
   clip.Top = clip.Left = scroll.Left = 0;
   clip.Right = scroll.Right = FRAME_COLS (f);
-  clip.Bottom = FRAME_LINES (f);
+  clip.Bottom = FRAME_TOTAL_LINES (f);

   dest.X = 0;