Subject: bug#72986: Disabling menu-bar-mode changes size of new frames

From: martin rudalics <[email-masked]>
Date: Thu, 3 Oct 2024 15:54:53 +0200

> First frame with menu bar:
...
 > ConfigureNotify, PS=1328x1260, XS=1328x1258, DS=1328x1260
 > xg_frame_resized, changed, PS=1328x1260, XS=1328x1258

Now we get the 1328x1258 notification and do not reject it because it's
within the tolerance.

 > change_frame_size (5), TS=1280x1260~>1280x1258, TC=80x36~>80x35,

And here we end up with a frame of 35 lines, one less than we want
(rounding due to the 2 missing pixels).  But that's what you already got
with commit 24161683102 reverted.  Some users strongly object such
behavior so we probably will have to abolish the tolerance trick.  You
should "see" the two missing pixels on the last line of the frame's
normal window just above the minibuffer window, if there's text on that
last line.

With the earlier "strong" rejection the last change of text columns was

gui_figure_window_size (5), TS=1280x875~>1280x1260, TC=80x25~>80x36,

which is the expected value.

One obvious cause are these lines in adjust_frame_size

	  && (f->new_width >= 0 || f->new_height >= 0))
	/* For implied resizes with inhibit 2 (external menu and tool
	   bar) pick up any new sizes the display engine has not
	   processed yet.  Otherwise, we would request the old sizes
	   which will make this request appear as a request to set new
	   sizes and have the WM react accordingly which is not TRT.

	   We don't that for the external menu bar on Motif.
	   Otherwise, switching off the menu bar will shrink the frame
	   and switching it on will not enlarge it.  */
	{
	  if (f->new_width >= 0)
	    new_native_width = f->new_width;
	  if (f->new_height >= 0)
	    new_native_height = f->new_height;
	}

Here we use the (unrejected) 1258 value (which was stored by a delayed
change_frame_size_1) in f->new_height and use it as something god-given
in future calls.  But on GTK all this doesn't make sense.  Whatever the
size of the menu or tool bar is, it won't affect the height of the
native frame.

 > First frame without menu bar:
...
 > ConfigureNotify, PS=1328x1260, XS=1328x1258, DS=1328x1260
 > xg_frame_resized, changed, PS=1328x1260, XS=1328x1258

Same here.  I never asked you but IIUC this means that creating the
first frame without menubar proceeds just as "smoothly" as with commit
24161683102 reverted.  Right?  Does it proceed just as smoothly with the
earlier gtkutil-reject-2024-10-01.diff used instead?  Or can you observe
the correction by two pixels somehow?

 > Second frame with menu bar:
...
 > ConfigureNotify, PS=1328x1260, XS=400x340, DS=1328x1260
 > xg_frame_resized, rejected, PS=1328x1260, XS=400x340, DS=664x655

Here we reject the event and ask again for 1260 pixels ...

 > tool-bar-lines (2), MS=160x175
 > xg_frame_set_char_size, visible, PS=1328x1260, XS=1328x1260, DS=1328x1260
 >   +values, PS=200x195, XS=664x696
 > ConfigureNotify, PS=1328x1260, XS=1328x1258, DS=1328x1260
 > xg_frame_resized, changed, PS=1328x1260, XS=1328x1258

... in response we are offered 1258 pixels and accept them ...

 > change_frame_size_1, delayed, PS=1328x1260, XS=1328x1258, DS=1328x1260
 > change_frame_size (5), TS=1280x1260~>1280x1258, TC=80x36~>80x35,
 > NS=1328x1260~>1328x1258, IS=1328x1260~>1328x1258, MS=32x70 IH IV

... and are left with 1258 pixels.  The frame has 35 lines, one less
than we wanted.

 > Second frame without menu bar:
...
 > ConfigureNotify, PS=1328x1260, XS=400x374, DS=1328x1260
 > xg_frame_resized, rejected, PS=1328x1260, XS=400x374, DS=664x630

Same here ...

 > ConfigureNotify, PS=1328x1260, XS=1328x1258, DS=1328x1260
 > xg_frame_resized, changed, PS=1328x1260, XS=1328x1258

... here ...

 > change_frame_size_1, delayed, PS=1328x1260, XS=1328x1258, DS=1328x1260
 > change_frame_size (5), TS=1280x1260~>1280x1258, TC=80x36~>80x35,
 > NS=1328x1260~>1328x1258, IS=1328x1260~>1328x1258, MS=32x70 IH IV

... and here.  I suppose you still see here the small frame first and
the resized one immediately afterwards.  Right?

Now about the 25 lines scenario which is quite interesting:

 > First frame with menu bar:
 >
 > Frame size history of #<frame  *Minibuf-1* - GNU Emacs at dwks
 > 0x5e3d15f35b10>
 > x_create_frame_1 (5), TS=80x25~>1280x875, NS=80x25~>1296x875,

The 875 come from (* 25 35) where 25 is the initial frame height in
lines Emacs wants (for historical reasons I can't explain).  In the
other scenarios we changed that to 36 later, now we don't.

 > IS=80x25~>1296x875, MS=32x70 IH IV
 > gui_figure_window_size (5), MS=32x70 IH IV
 > scroll-bar-width (3), NS=1296x875~>1328x875, IS=1296x875~>1328x875,
 > MS=160x175
 > scroll-bar-height (3), MS=160x175
 > menu-bar-lines (2), MS=160x175
 > x_create_frame_2 (0), MS=160x175
 > xg_frame_set_char_size, invisible, PS=1328x875, XS=1328x875, DS=1328x875

The 875 is what we ask for here ...

 > xg_frame_set_char_size (5), MS=32x70 IH IV
 > x_make_frame_visible
 > MapNotify, not hidden & not iconified, PS=1328x875, DS=1328x875
 > ConfigureNotify, PS=1328x875, XS=1328x874, DS=1328x875
 > xg_frame_resized, changed, PS=1328x875, XS=1328x874

... and what we get are 874 pixels (one less so we stay within the
height tolerance and accept it) ...

 > change_frame_size_1, delayed, PS=1328x875, XS=1328x874, DS=1328x875
 > change_frame_size (5), TS=1280x875~>1280x874, TC=80x25~>80x24,
 > NS=1328x875~>1328x874, IS=1328x875~>1328x874, MS=32x70 IH IV

... but here we get a second ConfigureNotify which allots us 850 pixels
only.  We don't reject it because the last time xg_frame_set_char_size
was called the frame was still invisible and there are no last values
available.  We reduce the number of lines from 25 to 24.

 > ConfigureNotify, PS=1328x874, XS=1328x850
 > xg_frame_resized, changed, PS=1328x874, XS=1328x850
 > change_frame_size_1, delayed, PS=1328x874, XS=1328x850
 > tool-bar-lines (2), NS=1328x874~>1328x850, MS=160x175
 > xg_frame_set_char_size, visible, PS=1328x874, XS=1328x850, DS=1328x850
 >   +values, PS=664x450, XS=664x491
 > ConfigureNotify, PS=1328x874, XS=1328x850, DS=1328x850
 > xg_frame_resized, changed, PS=1328x874, XS=1328x850, DS=1328x850
 > change_frame_size_1, delayed, PS=1328x874, XS=1328x850, DS=1328x850
 > change_frame_size (5), TS=1280x874~>1280x850, NS=1328x874~>1328x850,
 > IS=1328x874~>1328x850, MS=32x70 IH IV
 > set_window_configuration (4), MS=160x175 IH IV

Eventually, the frame ends up with 24 lines, one less than we want.

If you still have enough patience, I'd ask you to try next with the
attached patch gtkutil-frame.diff.  It does not reject anything but also
does not pick up any sizes from delayed size changes.

martin

diff --git a/src/frame.c b/src/frame.c
index 7f4bf274ad9..af95cb54701 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -734,6 +734,7 @@ adjust_frame_size (struct frame *f, int new_text_width, int new_text_height,
 	      && (new_native_height != old_native_height
 		  || inhibit == 0 || inhibit == 2))))
     {
+#ifndef USE_GTK
       if (inhibit == 2
 #ifdef USE_MOTIF
 	  && !EQ (parameter, Qmenu_bar_lines)
@@ -754,7 +755,7 @@ adjust_frame_size (struct frame *f, int new_text_width, int new_text_height,
 	  if (f->new_height >= 0)
 	    new_native_height = f->new_height;
 	}
-
+#endif
       if (CONSP (frame_size_history))
 	frame_size_history_adjust (f, inhibit, parameter,
 				   old_text_width, old_text_height,
@@ -6769,7 +6770,7 @@ focus (where a frame immediately loses focus when it's left by the mouse
 
 The function `frame--size-history' displays the value of this variable
 in a more readable form.  */);
-    frame_size_history = Qnil;
+  frame_size_history = Fcons (make_fixnum (100), Qnil);
 
   DEFVAR_BOOL ("tooltip-reuse-hidden-frame", tooltip_reuse_hidden_frame,
 	       doc: /* Non-nil means reuse hidden tooltip frames.
diff --git a/src/gtkutil.c b/src/gtkutil.c
index d57627f152f..2be84ae60b9 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -1327,10 +1327,17 @@ xg_frame_set_char_size (struct frame *f, int width, int height)
   if (FRAME_VISIBLE_P (f) && !was_visible)
     {
       if (CONSP (frame_size_history))
-	frame_size_history_extra
-	  (f, build_string ("xg_frame_set_char_size, visible"),
-	   FRAME_PIXEL_WIDTH (f), FRAME_PIXEL_HEIGHT (f), width, height,
-	   f->new_width, f->new_height);
+	{
+	  frame_size_history_extra
+	    (f, build_string ("xg_frame_set_char_size, visible"),
+	     FRAME_PIXEL_WIDTH (f), FRAME_PIXEL_HEIGHT (f), width, height,
+	     f->new_width, f->new_height);
+
+	  if (gwidth > 0 || gheight > 0)
+	    frame_size_history_extra
+	      (f, build_string (" +values"), gwidth, gheight,
+	       outer_width, outer_height, -1, -1);
+	}
 
       /* Must call this to flush out events */
       (void)gtk_events_pending ();