After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 686935 - Dragging the toolbar down should leave full screen
Dragging the toolbar down should leave full screen
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: general
3.6.x
Other Linux
: Normal normal
: --
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks: 686781
 
 
Reported: 2012-10-26 11:46 UTC by Allan Day
Modified: 2016-03-31 13:57 UTC
See Also:
GNOME target: ---
GNOME version: 3.5/3.6


Attachments
Allow drag to unfullscreen (2.91 KB, patch)
2013-02-20 11:24 UTC, Alexander Larsson
needs-work Details | Review
Allow drag to unfullscreen (3.29 KB, patch)
2013-02-21 09:56 UTC, Alexander Larsson
committed Details | Review

Description Allan Day 2012-10-26 11:46:35 UTC
Dragging the toolbar down when the window is maximized unmaximizes. I expected the same thing to happen in fullscreen mode, but it didn't have any effect.

One alternative would be to make the fullscreen toolbar look different from the normal one: something that is overlaid and semi-transparent probably wouldn't have this problem.
Comment 1 Jakub Steiner 2012-10-26 12:23:01 UTC
The fullscreen toolbar _is_ supposed to be styled in the similar way to the overlay toolbar (which Boxes is lacking too).

https://raw.github.com/gnome-design-team/gnome-mockups/master/boxes/boxes-fullscreen.png
Comment 2 Alexander Larsson 2013-01-25 14:09:55 UTC
This affects both the overlay in a VM display and the toolbar in the collection view. Right now if you start boxes in fullscreen (-f) there is no way to get out of it other than starting a VM and unfullscreening from there.
Comment 3 Alexander Larsson 2013-02-20 11:24:53 UTC
Created attachment 236912 [details] [review]
Allow drag to unfullscreen
Comment 4 Zeeshan Ali 2013-02-20 14:15:11 UTC
Review of attachment 236912 [details] [review]:

::: src/display-page.vala
@@ +63,3 @@
+    private uint button_down_button;
+
+    public override bool button_press_event (Gdk.EventButton ev) {

More descriptive names please: ev -> event

@@ +66,3 @@
+        var res = base.button_press_event (ev);
+
+        Gdk.Event *eev = (Gdk.Event *)(&ev);

Why do we need pointer? We are not modifying the event struct even.

@@ +87,3 @@
+            double dy = ev.y - button_down_y;
+
+            if (dx * dx + dy * dy > 40 * 40) {

whats this calculating?

@@ +91,3 @@
+                App.app.fullscreen = false;
+
+                var window = get_toplevel () as Gtk.Window?;

Don't think we need the '?' here. Looks slightly odd.

@@ +92,3 @@
+
+                var window = get_toplevel () as Gtk.Window?;
+                int old_w;

old_w -> old_width

@@ +97,3 @@
+                ulong id = 0;
+                id = App.app.notify["fullscreen"].connect ( () => {
+                    int root_x, root_y, w;

w -> width
Comment 5 Alexander Larsson 2013-02-21 09:10:25 UTC
Review of attachment 236912 [details] [review]:

::: src/display-page.vala
@@ +66,3 @@
+        var res = base.button_press_event (ev);
+
+        Gdk.Event *eev = (Gdk.Event *)(&ev);

This is due to problems with the current bindings of GdkEvent in vala. There is no other way to upcast from Gdk.EventButton to GdkEvent which is what we need to get at GdkEvent.triggers_context_menu.

I'm chatting with the vala people about redoing the GdkEvent binding, but this is it for now.

@@ +87,3 @@
+            double dy = ev.y - button_down_y;
+
+            if (dx * dx + dy * dy > 40 * 40) {

Its the normal euclidian distance calculation.

@@ +91,3 @@
+                App.app.fullscreen = false;
+
+                var window = get_toplevel () as Gtk.Window?;

I guess not.
Comment 6 Alexander Larsson 2013-02-21 09:56:36 UTC
Created attachment 237021 [details] [review]
Allow drag to unfullscreen
Comment 7 Alexander Larsson 2013-02-21 09:58:40 UTC
I added comments to the upcast workaround, but if ever a better mapping for GdkEvent lands we need to drop that as it will stop working. Juerg *may* land it this cycle, but maybe not. I've asked him to keep us in the loop on the decision.
Comment 8 Alexander Larsson 2013-02-21 10:29:57 UTC
This is a tricky one.

During size_allocate() on the spice widget the GtkDrawingArea sends a configure event, which is caught by spice-widget.c:configure_event(). This then tries to get a mouse grab via try_mouse_grab(). If this succeeds we'll send a SPICE_DISPLAY_MOUSE_GRAB signal which eventually will reach the boxes code that adds the label about unbreaking.

The label change causes a queue_resize on the label widgets and up the widget tree to the toplevel, but this is ignored/overridden by gtk+ since you're not allowed to request size changes during size allocation (that leads to loops). So, the labels are not correctly relayouted.
Comment 9 Alexander Larsson 2013-02-21 10:30:23 UTC
eh, wrong bug.
should be bug 692465
Comment 10 Zeeshan Ali 2013-02-22 10:41:56 UTC
Review of attachment 236912 [details] [review]:

::: src/display-page.vala
@@ +87,3 @@
+            double dy = ev.y - button_down_y;
+
+            if (dx * dx + dy * dy > 40 * 40) {

Forgive me, I've started to forget my school education. :(
Comment 11 Zeeshan Ali 2013-02-22 10:42:06 UTC
Review of attachment 237021 [details] [review]:

Look good now, ack
Comment 12 Alexander Larsson 2013-02-22 11:43:27 UTC
Attachment 236912 [details] pushed as 715d4f1 - Allow drag to unfullscreen
Attachment 237021 [details] pushed as 715d4f1 - Allow drag to unfullscreen