Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing scrollbars on too-high menu-button and alt-F1 menus #922

Closed
wants to merge 1 commit into from

Conversation

lukefromdc
Copy link
Member

This fixes part of #920

@raveit65
Copy link
Member

raveit65 commented Feb 4, 2019

I got a build warning.

panel-menu-button.c: In function 'panel_menu_button_popup_menu':
panel-menu-button.c:480:34: warning: passing argument 1 of 'gtk_menu_reposition' from incompatible pointer type [-Wincompatible-pointer-types]
  gtk_menu_reposition(button->priv->menu);
                      ~~~~~~~~~~~~^~~~~~
In file included from /usr/include/gtk-3.0/gtk/gtklabel.h:34,
                 from /usr/include/gtk-3.0/gtk/gtkaccellabel.h:35,
                 from /usr/include/gtk-3.0/gtk/gtk.h:33,
                 from button-widget.h:4,
                 from panel-menu-button.h:28,
                 from panel-menu-button.c:27:
/usr/include/gtk-3.0/gtk/gtkmenu.h:176:49: note: expected 'GtkMenu *' {aka 'struct _GtkMenu *'} but argument is of type 'GtkWidget *' {aka 'struct _GtkWidget *'}
 void    gtk_menu_reposition    (GtkMenu        *menu);
                                 ~~~~~~~~~~~~~~~~^~~~

@raveit65
Copy link
Member

raveit65 commented Feb 4, 2019

This fixes the build issue.

	/*Ensure scrollbars show up if vertical space is limited*/
	gtk_menu_reposition (GTK_MENU (button->priv->menu));

@lukefromdc
Copy link
Member Author

I figured it would, and I am about to update this with both changes

@lukefromdc
Copy link
Member Author

Just applied both changes asked for and force-pushed. Question: do we want to wait to see if GTK gets an upstream fix as per the discussion at and under
#920 (comment)
or merge this as soon as it can be tested? The issue did not appear until after GTK 3.24.3, when three new commits affected menus.
https://gitlab.gnome.org/GNOME/gtk/commit/00486efd51b29e234cfb180b814dd584805ebae5 would be the initial source of the problem,
https://gitlab.gnome.org/GNOME/gtk/commit/3e586a82e6cc563a84f7f16341c63743b0c62762
was no doubt supposed to ensure the menus were resized to fit but at least here didn't do it,
and
https://gitlab.gnome.org/GNOME/gtk/commit/c35878ecf1e9ee960b72b213b73c15f92b64afe5
was to ensure the scroll arrow was correctly rendered but again this is not what we are getting.

I just tested master with today's GTK 3.24.5 and oh yeah, we still have the issue

@lukefromdc
Copy link
Member Author

GTK 3 issue reported at
https://gitlab.gnome.org/GNOME/gtk/issues/1651

Copy link
Member

@raveit65 raveit65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@raveit65
Copy link
Member

raveit65 commented Feb 5, 2019

do we want to wait to see if GTK gets an upstream fix as per the discussion at and under
#920 (comment)
or merge this as soon as it can be tested?

I would wait a bit, let see how gtk upstream react and if they do a new release with a fix.
On the other side we can always revert your fix when it is fixed by upstream.
Keep in mind that it take some time when a fix from our side will find a way in distros and a fix by gtk+ is already there at this time.

@lukefromdc
Copy link
Member Author

Albert just reproduced this on a gnome-terminal menu bar, which ought to get the GNOME devs working on this

@lukefromdc
Copy link
Member Author

lukefromdc commented Feb 7, 2019

We now have a PR for GTK3 that fixes this right:
https://gitlab.gnome.org/GNOME/gtk/merge_requests/565
so unless that gets rejected we won't need this PR here, which will become unneeded code after the GTK fix gets merged. Holding this open in case the GTK3 PR does not get merged for any reason

@lukefromdc
Copy link
Member Author

gitlab.gnome.org/GNOME/gtk/commit/1d4eac211c09624d29b9309e2b92173f7477a9b7
has been merged and fixes this, so closing this PR which was at best a partial workaround and now becomes unneeded code

@lukefromdc lukefromdc closed this Feb 7, 2019
@raveit65 raveit65 deleted the fix-menu-scrollbars branch February 10, 2019 09:00
@raveit65
Copy link
Member

@lukefromdc
Can branch dev-all be deleted?

@lukefromdc
Copy link
Member Author

lukefromdc commented Feb 15, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants