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 674664 - fullscreen OSD comes up too aggressively
fullscreen OSD comes up too aggressively
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: display
unspecified
Other Linux
: Normal normal
: --
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
ui-design
: 696841 706129 (view as bug list)
Depends on:
Blocks: 696727
 
 
Reported: 2012-04-23 23:12 UTC by William Jon McCann
Modified: 2016-03-31 14:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
display-page: Reveal overlaybar on pointer in center (1.44 KB, patch)
2014-07-01 18:16 UTC, Zeeshan Ali
needs-work Details | Review
display-page: Smaller overlaybar (1.20 KB, patch)
2014-07-01 18:16 UTC, Zeeshan Ali
rejected Details | Review
display-toolbar: Don't show window decorations (1.18 KB, patch)
2014-07-01 19:05 UTC, Zeeshan Ali
accepted-commit_now Details | Review
display-page: Don't reveal topbar on pointer over edges (1.63 KB, patch)
2014-07-02 15:31 UTC, Zeeshan Ali
needs-work Details | Review
display-toolbar: Don't show WM decorations in fullscreen (1.56 KB, patch)
2014-07-02 15:31 UTC, Zeeshan Ali
committed Details | Review
display-page: Don't reveal topbar on pointer over edges (1.63 KB, patch)
2014-07-02 15:56 UTC, Zeeshan Ali
committed Details | Review
display-page: Bump edge width from 80px to 100px (932 bytes, patch)
2014-07-02 18:57 UTC, Zeeshan Ali
committed Details | Review

Description William Jon McCann 2012-04-23 23:12:37 UTC
We show the fullscreen OSD anytime I touch the top of the screen. This is a bit too aggressive and makes it hard to interact with the top of the screen on the guest.

One possible solution is to have a two step reveal. Push against the top of the screen and show a handle affordance for bringing up the overlay.
Comment 1 Alexandre Franke 2013-11-07 14:28:42 UTC
*** Bug 697883 has been marked as a duplicate of this bug. ***
Comment 2 Zeeshan Ali 2014-02-23 18:35:28 UTC
Mockup would help. :)
Comment 3 Zeeshan Ali 2014-02-23 21:23:25 UTC
*** Bug 696841 has been marked as a duplicate of this bug. ***
Comment 4 Zeeshan Ali 2014-02-23 21:23:33 UTC
*** Bug 706129 has been marked as a duplicate of this bug. ***
Comment 5 Lasse Schuirmann 2014-05-16 16:15:37 UTC
A few thoughts on this.


I think we have the following problems with the current design:

* <b>Borders and edges are used by some UIs (Gnome, KDE [when properly configured], even Windows 8).</b>
* You don't have any way to get out of the VM when working on a touchscreen-only device. (Other than shutting down the VM. You can't hit the border.)
* The user doesn't know how to get out of this anyway.
* Have you tried a dualscreen setup with one monitor above the other and Boxes running fullscreen on the lower one? I just did. I can't get out of the fullscreen mode.

jimmac proposed using a Shortcut, e.g. super+esc. I think this is a good solution:
* We can show an overlay "Press Super + ESC to exit fullscreen mode" when entering the fullscreen mode.
* We have no problems with borders
* Who uses Super + ESC assuming you don't want to launch a VM in the VM?
  * Maybe we can somehow use an escape mechanism (like \" in strings). This is probably not so easy but just an idea to make it _somehow_ possible...

Since we want to detect mouses and keyboards anyway (see also Bug #698430 - avoid mouse/keyboard redirection) I would not offer fullscreening boxes when no keyboard is detected. Next step would be to fall back to a gesture or something other in this case (but I think this is way to much work for the improbable case that someone tries to use Boxes on a gnome tablet).

This way no user ends up in a state where he can't exit the fullscreen mode. The only thing we'd have to take care of is the user forgetting the shortcut:
Should we maybe remember the user in case he hits the border twice or so?
Comment 6 Zeeshan Ali 2014-05-17 18:59:07 UTC
(In reply to comment #5)
> A few thoughts on this.
> 
> 
> I think we have the following problems with the current design:
> 
> * <b>Borders and edges are used by some UIs (Gnome, KDE [when properly
> configured], even Windows 8).</b>
> * You don't have any way to get out of the VM when working on a
> touchscreen-only device. (Other than shutting down the VM. You can't hit the
> border.)
>
> * The user doesn't know how to get out of this anyway.

I guess that is not more true that we don't fullscreen by default. I had put code in place so that fullscreen toolbar doesn't hide until user moves the mouse (commit 1edaf23) and therefore harder for them to miss it's existence in there. We need to do the same I think but thats a different bug than this.

> * Have you tried a dualscreen setup with one monitor above the other and Boxes
> running fullscreen on the lower one? I just did. I can't get out of the
> fullscreen mode.
> 
> jimmac proposed using a Shortcut, e.g. super+esc. I think this is a good
> solution:
> * We can show an overlay "Press Super + ESC to exit fullscreen mode" when
> entering the fullscreen mode.

Sure, sounds good but I think it should be 'Super+F11' rather to keep in consistent a bit with general shortcut for fullscreen. We'll also want some other nice shortcuts, e.g one to quickly switch between running VMs. 'Super+Tab' seems appropriate if we go for 'Super+Esc'. However, all these new shortcuts are unrelated to this bug. IIRC, there is at least already a bug for F11 not working.

> * We have no problems with borders

How does adding a keyboard shortcut help with borders? Unless you meant that we shouldn't display overlay toolbar, which IMO is a bad idea. We still want the overlay. What I suggest we do is making the overlay toolbar shorter (like in virt-manager but not as short) so that it doesn't cover the corners.

> * Who uses Super + ESC assuming you don't want to launch a VM in the VM?

For such cases, we'll still need a UI for sending keys to VMs.

> Since we want to detect mouses and keyboards anyway (see also Bug #698430 -
> avoid mouse/keyboard redirection) I would not offer fullscreening boxes when no
> keyboard is detected. Next step would be to fall back to a gesture or something
> other in this case (but I think this is way to much work for the improbable
> case that someone tries to use Boxes on a gnome tablet).

I don't think we need to be exclusive here. We need to provide multiple ways to achieve the same. i-e Make it work for both keyboard-only, touchscreen and mouse cases.
 
> This way no user ends up in a state where he can't exit the fullscreen mode.
> The only thing we'd have to take care of is the user forgetting the shortcut:
> Should we maybe remember the user in case he hits the border twice or so?

This would be one reason it would be a very bad idea to only have kbd shortcuts. :)
Comment 7 Zeeshan Ali 2014-07-01 18:16:29 UTC
Created attachment 279705 [details] [review]
display-page: Reveal overlaybar on pointer in center

Seems all modern operating systems (GNOME, Windows 8, OS X) reserve
'pointer on corners' as gentures for different UIs. Lets try out best
not to interfer with that by only revealing overlaybar when pointer is
around the top center of the display.
Comment 8 Zeeshan Ali 2014-07-01 18:16:40 UTC
Created attachment 279706 [details] [review]
display-page: Smaller overlaybar

Now that we only reveal the overlaybar when pointer is near the center,
it looks rather odd if overlaybar itself is shown across the whole
display width. We now tell Gtk+ to put it in the center for us, taking
only space that it needs to.

Known issue: The default title label of HeaderBar is now ellipsized,
which isn't something we'd want.
Comment 9 Zeeshan Ali 2014-07-01 18:18:17 UTC
I think these patches provide a good solution for the actual issue. I'd provide a screencast but currently have classic-mode on all my apps for some reason. :(
Comment 10 Zeeshan Ali 2014-07-01 19:05:10 UTC
Created attachment 279708 [details] [review]
display-toolbar: Don't show window decorations

Turns out that 'show-close-button' property is about all window
decorations and in classic-mode this means we show maximize and minimize
buttons and we really don't want to show those now that display toolbar
is so small; otherwise it looks really ugly.

Besides we probably don't even want to show 'close' button in fullscreen
either.
Comment 11 Lasse Schuirmann 2014-07-02 07:29:59 UTC
Review of attachment 279705 [details] [review]:

::: src/display-page.vala
@@ +172,3 @@
             var y = event.motion.y;
+            if (x >= (get_allocated_width () / 2 - 100) &&
+                x <= (get_allocated_width () / 2 + 100) &&

Thats a rather small region especially on large screens. As a user I would normally not distinguish between the center of the top border and the top border: Can we take e.g. (x >= 100) && (x <= get_allocated_width () - 100) ? This also solves our problem while the user don't have to find out it's in the center.

Can we also replace the 100 with a const uint INSENSITIVE_EDGE_SIZE or so to get some meaning behind the number?
Comment 12 Lasse Schuirmann 2014-07-02 07:48:18 UTC
Review of attachment 279706 [details] [review]:

I do have some issues with this change:
* The width of the overlay bar may differ from the one of the detection region. As a user I would expect them to be equal especially if it's centered and rather small. I imagine this feeling better if we exclude just the edges from the detection region and have a full-sized bar.
* This places the back button rather in the middle of the screen. In our UI concept we have it usually on the top left. That looks weird for me.
* The close button vanished.
* The known issue described in log.

It's not so odd for me to have a full-sized bar there. I feel more comfortable without this patch. Especially _if_ we extend the detection region to almost the whole screen width.

Maybe jimmac can provide a third opinion?
Comment 13 Lasse Schuirmann 2014-07-02 07:50:14 UTC
Review of attachment 279706 [details] [review]:

Sorry, the third point doesn't apply to this patch. ("* The close button vanished.")
Comment 14 Lasse Schuirmann 2014-07-02 08:10:51 UTC
Review of attachment 279708 [details] [review]:

I was not able to see these minimize and maximize buttons in classic mode without this patch. It should be in the overlay bar you just shrinked a patch ago, right?
Comment 15 Zeeshan Ali 2014-07-02 11:47:02 UTC
Review of attachment 279705 [details] [review]:

::: src/display-page.vala
@@ +172,3 @@
             var y = event.motion.y;
+            if (x >= (get_allocated_width () / 2 - 100) &&
+                x <= (get_allocated_width () / 2 + 100) &&

Hmm.. I'll give another try to make both this virtual region and topbar take same amount of space. I am not sure but maybe it'd look weird if topbar comes up even when pointer is no where near it?
Comment 16 Zeeshan Ali 2014-07-02 11:58:44 UTC
(In reply to comment #12)
> Review of attachment 279706 [details] [review]:
> 
> I do have some issues with this change:
> * The width of the overlay bar may differ from the one of the detection region.
> As a user I would expect them to be equal especially if it's centered and
> rather small. I imagine this feeling better if we exclude just the edges from
> the detection region and have a full-sized bar.

As I said in the previous comment, I'll give another go at making the detection region and topbar coincide with each other. I recall (I cooked these patches a month ago) that I failed to achieve that and so I chose a size (100) for region that is about the same size as topbar.

> * This places the back button rather in the middle of the screen. In our UI
> concept we have it usually on the top left.


The UI concept doesn't deal with this issue.

> That looks weird for me.

Not to me. :) Maybe we should drop this patch and only ensure that detection region is not above the corners or near it even? In which case, your main comment about previous patch is not valid any more.

> * The known issue described in log.

Yeah, gotta look at that again if we want to keep this patch.
 
> It's not so odd for me to have a full-sized bar there. I feel more comfortable
> without this patch. Especially _if_ we extend the detection region to almost
> the whole screen width.

Ok, thats basically what I wrote above so I guess we agree then. :) I'll try that out and see how that feels and looks.
 
> Maybe jimmac can provide a third opinion?

Thats always welcome but I was hoping we don't have to bug him about this.
Comment 17 Lasse Schuirmann 2014-07-02 12:08:22 UTC
Review of attachment 279706 [details] [review]:

By the way: I just noticed that the HeaderBar is also "ellipsized" without this patch. I'll file a bug about that.
Comment 18 Lasse Schuirmann 2014-07-02 12:11:38 UTC
Review of attachment 279708 [details] [review]:

I'd change the log, I don't really get what you want to say. As I said before I don't understand this classic mode thing (tried to try it out) but isn't your last sentence enough reasoning for this patch?
Comment 19 Zeeshan Ali 2014-07-02 12:22:12 UTC
(In reply to comment #18)
> Review of attachment 279708 [details] [review]:
> 
> I'd change the log, I don't really get what you want to say. As I said before I
> don't understand this classic mode thing (tried to try it out)

Well, thats not my fault. :) Figure how to enable classic-mode and you'll see what I mean.


> but isn't your
> last sentence enough reasoning for this patch?

Yes but its nice to have all the relevant info in the log and its not bad to have multiple rationales in there either.
Comment 20 Lasse Schuirmann 2014-07-02 12:23:32 UTC
(In reply to comment #19)
> (In reply to comment #18)
> > Review of attachment 279708 [details] [review] [details]:
> > 
> > I'd change the log, I don't really get what you want to say. As I said before I
> > don't understand this classic mode thing (tried to try it out)
> 
> Well, thats not my fault. :) Figure how to enable classic-mode and you'll see
> what I mean.

I was in classic mode.
Comment 21 Zeeshan Ali 2014-07-02 12:27:56 UTC
(In reply to comment #20)
> (In reply to comment #19)
> > (In reply to comment #18)
> > > Review of attachment 279708 [details] [review] [details] [details]:
> > > 
> > > I'd change the log, I don't really get what you want to say. As I said before I
> > > don't understand this classic mode thing (tried to try it out)
> > 
> > Well, thats not my fault. :) Figure how to enable classic-mode and you'll see
> > what I mean.
> 
> I was in classic mode.

Then have you tried non-classic ever? :) Its pretty simple really. You get maximize, minimize and close buttons if 'show-close-button' is enabled and we definitely don't want maximize and minimize button in this case at least and perhaps its not good to provide close button either in here even in classic mode.
Comment 22 Lasse Schuirmann 2014-07-02 12:38:46 UTC
(In reply to comment #21)
> 
> Then have you tried non-classic ever? :) Its pretty simple really. You get
> maximize, minimize and close buttons if 'show-close-button' is enabled and we
> definitely don't want maximize and minimize button in this case at least and
> perhaps its not good to provide close button either in here even in classic
> mode.

Okay. I have to say I'm not sure that the removal of the close button is such a good thing. Its not that the action is not applicable in this context, is it?

Another thing: right now, at least in non-classic mode, we have the topbar and this overlay. The great thing about them is the layout and the buttons are exactly the same so its easy to use for the user.

All in all:
- I agree that removing the maximize and minimize button for classic mode is a good idea.
- I'd rather leave the close button as it is
Comment 23 Zeeshan Ali 2014-07-02 12:58:46 UTC
(In reply to comment #22)
> (In reply to comment #21)
> > 
> > Then have you tried non-classic ever? :) Its pretty simple really. You get
> > maximize, minimize and close buttons if 'show-close-button' is enabled and we
> > definitely don't want maximize and minimize button in this case at least and
> > perhaps its not good to provide close button either in here even in classic
> > mode.
> 
> Okay. I have to say I'm not sure that the removal of the close button is such a
> good thing. Its not that the action is not applicable in this context, is it?

Its not exactly relevant. I have myself many times clicked it instead of 'back' button in this context. Somehow it feels like a button to close the current VM rather than the whole app. Besides the more different they look, the less user sees them as the same thing.

> Another thing: right now, at least in non-classic mode, we have the topbar and
> this overlay. The great thing about them is the layout and the buttons are
> exactly the same so its easy to use for the user.

Its consistent but they feel very different. In fullscreen, VM and its display need to be the focus and other buttons only clutter the UI.
Comment 24 Lasse Schuirmann 2014-07-02 13:05:31 UTC
Review of attachment 279708 [details] [review]:

ok for me, but you have to commit it :)
Comment 25 Zeeshan Ali 2014-07-02 15:31:20 UTC
Created attachment 279759 [details] [review]
display-page: Don't reveal topbar on pointer over edges

Seems all modern operating systems (GNOME, Windows 8, OS X) reserve
'pointer on edges' as gentures for different UIs. Lets try out best not to
interfer with that by only revealing overlaybar when pointer is around the
top center of the screen.
Comment 26 Zeeshan Ali 2014-07-02 15:31:42 UTC
Created attachment 279760 [details] [review]
display-toolbar: Don't show WM decorations in fullscreen

Turns out that 'show-close-button' property is about all window
decorations and in classic-mode this means we show maximize and minimize
buttons and they seem misplaced in there in that context. We probably
don't even want to show 'close' button in fullscreen either. Hence
removing all window manager decorations.
Comment 27 Lasse Schuirmann 2014-07-02 15:46:27 UTC
Review of attachment 279759 [details] [review]:

::: src/display-page.vala
@@ +5,3 @@
 [GtkTemplate (ui = "/org/gnome/Boxes/ui/display-page.ui")]
 private class Boxes.DisplayPage: Gtk.Box {
+    private const uint8 SCREEN_EDGE_WIDTH = 50;

I think 50 px is a bit too small. I tested out manically hitting the upper left edge again and again and I'd say 80 is a bit better.

@@ +174,3 @@
             var y = event.motion.y;
+            if (x >= SCREEN_EDGE_WIDTH && x <= (get_allocated_width () - SCREEN_EDGE_WIDTH) &&
+                y <= 0 && toolbar_show_id == 0) {

I think I'd do brackets around at least the bigger subconditions here but its probably a matter of taste.
Comment 28 Lasse Schuirmann 2014-07-02 15:47:21 UTC
Review of attachment 279760 [details] [review]:

Looks good for me.
Comment 29 Zeeshan Ali 2014-07-02 15:56:12 UTC
Created attachment 279785 [details] [review]
display-page: Don't reveal topbar on pointer over edges

v2: edge width changed to 80 pixels but don't agree with need for more brackets in if condition.
Comment 30 Lasse Schuirmann 2014-07-02 15:57:27 UTC
Review of attachment 279785 [details] [review]:

okay.
Comment 31 Zeeshan Ali 2014-07-02 15:59:18 UTC
Attachment 279760 [details] pushed as 7697874 - display-toolbar: Don't show WM decorations in fullscreen
Attachment 279785 [details] pushed as 5aaf352 - display-page: Don't reveal topbar on pointer over edges
Comment 32 Stephen 2014-07-02 16:19:38 UTC
Unless I misunderstand, the fullscreen OSD will appear anywhere inwards of 80 px L/R at the top?

In Windows 7 for example, the minimise/maximise/close group extend inwards from the right ~108 px when a window is maximised, and they are also clickable at the very top edge then. Having only an 80 px ignored area would mean the top bar will still interfere with the minimise button, requiring very careful mouse movement to hit it without triggering the top bar.

Can I suggest updating/re-committing the patch to allow slightly more ignored area for this reason (and similarly for windows controls other common OSes/versions)?
Comment 33 Zeeshan Ali 2014-07-02 18:57:30 UTC
Created attachment 279795 [details] [review]
display-page: Bump edge width from 80px to 100px

This is mainly to ensure that user doesn't accidently reveal topbar
while trying to reach for window buttons on the top right (in LTR
locale).
Comment 34 Zeeshan Ali 2014-07-02 18:59:49 UTC
Fortunately I had win7 VM around to test with. The attached patch makes it so that edges (the part that doesn't reveal topbar on hover) completely cover the window buttons in maximized state.
Comment 35 Zeeshan Ali 2014-07-02 19:00:26 UTC
Comment on attachment 279795 [details] [review]
display-page: Bump edge width from 80px to 100px

Attachment 279795 [details] pushed as 62f7ae2 - display-page: Bump edge width from 80px to 100px