gtk: menubar visible in fullscreen mode with gtk3

Bug #1294898 reported by Cole Robinson
22
This bug affects 3 people
Affects Status Importance Assigned to Milestone
QEMU
Fix Released
Undecided
Unassigned
ubuntu-gnome3-desktop (Ubuntu)
Confirmed
Undecided
Unassigned

Bug Description

Using the gtk UI, compiled with gtk3, the menu bar is fully visible in full screen mode. On gtk2 it's hidden. The set_size_request call isn't abided on gtk3 it seems.

Simple fix is:

diff --git a/ui/gtk.c b/ui/gtk.c
index 66e886f..7b3bd3d 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -805,7 +805,7 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)

     if (!s->full_screen) {
         gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
- gtk_widget_set_size_request(s->menu_bar, 0, 0);
+ gtk_widget_hide(s->menu_bar);
         gtk_widget_set_size_request(s->drawing_area, -1, -1);
         gtk_window_fullscreen(GTK_WINDOW(s->window));
         if (gd_on_vga(s)) {
@@ -815,7 +815,7 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
     } else {
         gtk_window_unfullscreen(GTK_WINDOW(s->window));
         gd_menu_show_tabs(GTK_MENU_ITEM(s->show_tabs_item), s);
- gtk_widget_set_size_request(s->menu_bar, -1, -1);
+ gtk_widget_show(s->menu_bar);
         gtk_widget_set_size_request(s->drawing_area,
                                     surface_width(s->ds),
                                     surface_height(s->ds));

The problem with that is that hiding the menu bar means all its associated accelerators are no longer usable, so there's way to exit fullscreen mode. That's kind of a problem :)

We can install the accelerators on the window, but make sure the menu item still shows the accelerator short cut. Example with the fullscreen accelerator:

diff --git a/ui/gtk.c b/ui/gtk.c
index 66e886f..fbce2b0 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -799,7 +799,7 @@ static void gd_menu_show_tabs(GtkMenuItem *item, void *opaque)
     }
 }

-static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
+static void gd_do_full_screen(void *opaque)
 {
     GtkDisplayState *s = opaque;

@@ -828,6 +828,11 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
     gd_update_cursor(s, FALSE);
 }

+static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
+{
+ gd_do_full_screen(opaque);
+}
+
 static void gd_menu_zoom_in(GtkMenuItem *item, void *opaque)
 {
     GtkDisplayState *s = opaque;
@@ -1304,10 +1309,11 @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, GtkAccelGroup *accel_g
     gtk_menu_set_accel_group(GTK_MENU(view_menu), accel_group);

     s->full_screen_item = gtk_menu_item_new_with_mnemonic(_("_Fullscreen"));
- gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->full_screen_item),
- "<QEMU>/View/Full Screen");
- gtk_accel_map_add_entry("<QEMU>/View/Full Screen", GDK_KEY_f,
- HOTKEY_MODIFIERS);
+ gtk_accel_group_connect(accel_group, GDK_KEY_f, HOTKEY_MODIFIERS, 0,
+ g_cclosure_new_swap(G_CALLBACK(gd_do_full_screen), s, NULL));
+ gtk_accel_label_set_accel(
+ GTK_ACCEL_LABEL(gtk_bin_get_child(GTK_BIN(s->full_screen_item))),
+ GDK_KEY_f, HOTKEY_MODIFIERS);
     gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->full_screen_item);

     separator = gtk_separator_menu_item_new();

However gtk_accel_label_set_accel, which shows the accel key sequence in the menu, is gtk 3.8+ :/ So older versions wouldn't have any visual indication of the shortcuts. Maybe that's not a problem, SDL didn't have any indication of shortcuts either.

Revision history for this message
ManDay (manday) wrote :

For what it's worth, Gerd replied on the list that

> Patch trades one bug for another. Hiding the menu bar also kills the
> accelerator keys, which is especially problematic for the fullscreen
> hotkey as there is no way out of fullscreen mode then.

Revision history for this message
Cole Robinson (crobinso) wrote :

Mailing list discussion:

https://lists.gnu.org/archive/html/qemu-devel/2014-10/msg00510.html

When Gerd made the comment quoted above, he hadn't read the full report. If someone wants to flesh out my patch bits and send it to qemu-devel it will be easier to follow up there.

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in ubuntu-gnome3-desktop (Ubuntu):
status: New → Confirmed
Revision history for this message
Thomas Huth (th-huth) wrote :
Changed in qemu:
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.