Subject: bug#22000: Patch addressing the menu-bar frame-resize interaction

From: martin rudalics <[email-masked]>
Date: Fri, 12 Oct 2018 20:12:21 +0200

>> The patchset still doesn't apply to master since master has evolved
 >> differently.  I get:
 >
 > Where exactly (commit id) are you applying the patch?

I don't recall - somewhen with today's master.

 >>  GtkScrolledWindow *sw;
 >>
 >> has been removed from the former and the latter is now
 >
 > GtkScrolledWindow is introduced by the patch series - I'm confused:
 > Which patch series did you try and where?

OK (I only tried to second guess the cause of the error messages
probably by looking at a not yet patched version).  But since the
patch applies in its entirety against the release version, the problem
must be master-specific.  In either case don't bother.

 > I can make truncation the default behaviour (in fact it is),
 > the problem is the presence of the scrolledwindow which is necessary
 > for the fix

You explained that earlier.

 > introduces extra padding. I'm not sure there's a way to fix that (well, I guess there is but it's a little tricky as it means the
 > scrolledwindow has to appear and disappear entirely from the widget hierarchy).

Why would it have to do that?

 > I might be able to fix it with a style change, if I can defeat the gtk3 docs
 > and figure out if/how to set a style property on a widget.

Let's postpone that for the moment.

 > That's pretty old... even on oldstable I have 3.14. I can try and find a system with that version of GTK, wondering if it's a GTK bug.

We already care for 3.3.6 and even 3.2.0.  Our usual problems with GTK
are the newer versions (since they often deprecate what we wrote).

Anyway, the primary warning I see is the following:

(emacs:4182): Gtk-WARNING **: gtk_scrolled_window_add(): cannot add non scrollable widget use gtk_scrolled_window_add_with_viewport() instead

Now gtk_scrolled_window_add_with_viewport is deprecated since GTK 3.8
so there's no use bothering with this _if_ we make truncation
optional.  But for some reason we don't.

I do not understand the following part of your changes:

#if GTK_CHECK_VERSION (3, 16, 0)
   GtkPolicyType menuscroll_policy = GTK_POLICY_EXTERNAL;
#else
   GtkPolicyType menuscroll_policy = GTK_POLICY_NEVER;
#endif
   ...
   menuscroll = get_frame_param (f, Qmenu_bar_scrollbar);
   if (EQ (menuscroll, Qautomatic))
     menuscroll_policy = GTK_POLICY_AUTOMATIC;
   else if (EQ (menuscroll, Qalways))
     menuscroll_policy = GTK_POLICY_ALWAYS;

Doesn't this mean that when a frame has the 'menu-bar-scrollbar'
parameter set we effectively override the version check above?

Immediately after that you unconditionally do

   /* Put the menu bar inside a scrolled window so that adding items
      to the menu bar (such as when entering dired mode or activating
      a minor more) does not trigger a frame resize:*/
   x->menubar_viewport = gtk_scrolled_window_new(NULL, NULL);
   sw = GTK_SCROLLED_WINDOW (x->menubar_viewport);
   gtk_scrolled_window_set_policy (sw, menuscroll_policy, GTK_POLICY_AUTOMATIC);

so we always put this into a _scrolled_ window regardless of whether
GTK can handle that.  That's the crucial problem here.

So I think we need two things:

(1) Give the user a variable and a frame parameter that controls
whether the menu bar shall be truncated when the frame gets smaller or
allows to resize the frame.  I would call that parameter
'gtk-menu-bar-resize' and the value would be nil by default which
means "truncate" while t would mean "auto-resize the frame".  In
addition we could add a minor mode like 'menu-bar-resize-mode' to
provide a default for users who don't want to set that individually
for each frame.

(2) Give the user a variable and a frame parameter that controls
whether the menu bar shall be scrollable when it can be truncated.  I
would call that parameter 'gtk-menu-bar-scroll' and the value would be
nil by default which means not to scroll while t would mean to scroll.
In addition we could add a minor mode like 'menu-bar-scroll-mode' to
provide a default for users who don't want to set that individually.

In short: (1) would allow users to specify whether they want a
scrollable menu bar window.  (2) would allow them to specify whether
that window should be really scrollable.  And we should make the menu
bar scrollable and thus provide (1) iff GTK supports it (so GTK 3.8 is
probably the minimum version where we can do that).  And for GTK < 3.8
nothing would change at all to what we have now.

Find a list of my current GTK bugs below.

Thanks, martin

The GTK bugs are currently on Emacs master with patches 0001 and 0002
only (this shows the menu bar initially):

(emacs:4182): Gtk-WARNING **: gtk_scrolled_window_add(): cannot add non scrollable widget use gtk_scrolled_window_add_with_viewport() instead

(emacs:4182): Gtk-CRITICAL **: gtk_scrollable_get_vscroll_policy: assertion `GTK_IS_SCROLLABLE (scrollable)' failed

(emacs:4182): Gtk-CRITICAL **: gtk_scrollable_get_hscroll_policy: assertion `GTK_IS_SCROLLABLE (scrollable)' failed

...

On Emacs 26 with the entire patch set (this is the one where the menu
bar is not shown initially).

(emacs:3730): Gtk-WARNING **: gtk_scrolled_window_add(): cannot add non scrollable widget use gtk_scrolled_window_add_with_viewport() instead

(emacs:3730): Gtk-CRITICAL **: gtk_scrollable_get_vscroll_policy: assertion `GTK_IS_SCROLLABLE (scrollable)' failed

(emacs:3730): Gtk-CRITICAL **: gtk_widget_get_preferred_width_for_height: assertion `height >= 0' failed

(emacs:3730): Gtk-CRITICAL **: gtk_widget_get_preferred_width_for_height: assertion `height >= 0' failed

(emacs:3730): Gtk-CRITICAL **: gtk_widget_get_preferred_width_for_height: assertion `height >= 0' failed

(emacs:3730): Gtk-CRITICAL **: gtk_widget_get_preferred_width_for_height: assertion `height >= 0' failed

(emacs:3730): Gtk-CRITICAL **: gtk_widget_get_preferred_width_for_height: assertion `height >= 0' failed

(emacs:3730): Gtk-CRITICAL **: gtk_widget_get_preferred_width_for_height: assertion `height >= 0' failed

(emacs:3730): Gtk-CRITICAL **: gtk_widget_get_preferred_width_for_height: assertion `height >= 0' failed

(emacs:3730): Gtk-WARNING **: gtk_widget_size_allocate(): attempt to allocate widget with width 32632 and height -1

(emacs:3730): Gtk-CRITICAL **: gtk_widget_get_preferred_width_for_height: assertion `height >= 0' failed

(emacs:3730): Gtk-WARNING **: gtk_widget_size_allocate(): attempt to allocate widget with width 32622 and height -3

(emacs:3730): Gtk-CRITICAL **: gtk_widget_get_preferred_width_for_height: assertion `height >= 0' failed

(emacs:3730): Gtk-WARNING **: gtk_widget_size_allocate(): attempt to allocate widget with width 32632 and height -1

(emacs:3730): Gtk-CRITICAL **: gtk_widget_get_preferred_width_for_height: assertion `height >= 0' failed

(emacs:3730): Gtk-WARNING **: gtk_widget_size_allocate(): attempt to allocate widget with width 32622 and height -3

...