[virt-tools-list] [PATCH virt-viewer] Don't show extra screens in fullscreen mode

[Jonathon Jongsma]

On Wed, 2014-09-24 at 12:07 +0200, Christophe Fergeau wrote:
> Hey,
> 
> On Tue, Sep 23, 2014 at 04:57:41PM -0500, Jonathon Jongsma wrote:
> > When using the fullscreen display mapping configuration file, extra
> > monitors could end up enabled by mistake. This was because
> > virt_viewer_app_get_initial_monitor_for_display would end up returning
> > Nmonitor = Ndisplay when the display map hash lookup failed. In
> > reality, when a display map is specified, but the hash lookup fails,
> > the display should not be enabled. This function now returns -1 to
> > distinguish this case, and the display is not enabled when this value is
> > returned.
> > ---
> > 
> > The implementation was apparently incomplete. This patch attempts to fix an
> > issue that was found during testing:
> > see https://bugzilla.redhat.com/show_bug.cgi?id=1129477#c9
> > 
> >  src/virt-viewer-app.c           | 22 ++++++++++++----------
> >  src/virt-viewer-session-spice.c |  3 +++
> >  2 files changed, 15 insertions(+), 10 deletions(-)
> > 
> > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> > index a890bf6..12d01eb 100644
> > --- a/src/virt-viewer-app.c
> > +++ b/src/virt-viewer-app.c
> > @@ -314,8 +314,11 @@ gint virt_viewer_app_get_initial_monitor_for_display(VirtViewerApp* self, gint d
> >  
> >      if (self->priv->initial_display_map) {
> >          gpointer value = NULL;
> > -        if (g_hash_table_lookup_extended(self->priv->initial_display_map, GINT_TO_POINTER(display), NULL, &value))
> > +        if (g_hash_table_lookup_extended(self->priv->initial_display_map, GINT_TO_POINTER(display), NULL, &value)) {
> >              monitor = GPOINTER_TO_INT(value);
> > +        } else {
> > +            monitor = -1;
> > +        }
> >      }
> 
> My understanding is that self->priv->initial_display_map is only set
> when a display configuration file is in used?

Yes, that's true

> 
> >  
> >      return monitor;
> > @@ -326,13 +329,14 @@ app_window_try_fullscreen(VirtViewerApp *self G_GNUC_UNUSED,
> >                            VirtViewerWindow *win, gint nth)
> >  {
> >      GdkScreen *screen = gdk_screen_get_default();
> > -
> > -    if (nth >= gdk_screen_get_n_monitors(screen)) {
> > -        g_debug("skipping display %d", nth);
> > +    gint monitor = virt_viewer_app_get_initial_monitor_for_display(self, nth);
> > +    if (monitor == -1 || monitor >= gdk_screen_get_n_monitors(screen)) {
> > +        g_debug("skipping fullscreen for display %d", nth);
> > +        virt_viewer_window_hide(win);
> >          return;
> >      }
> >  
> > -    virt_viewer_window_enter_fullscreen(win, nth);
> > +    virt_viewer_window_enter_fullscreen(win, monitor);
> >  }
> >  
> >  
> > @@ -449,8 +453,7 @@ void virt_viewer_app_set_uuid_string(VirtViewerApp *self, const gchar *uuid_stri
> >  
> >          g_hash_table_iter_init(&iter, self->priv->windows);
> >          while (g_hash_table_iter_next(&iter, NULL, &value)) {
> > -            gint monitor = virt_viewer_app_get_initial_monitor_for_display(self, i);
> > -            app_window_try_fullscreen(self, VIRT_VIEWER_WINDOW(value), monitor);
> > +            app_window_try_fullscreen(self, VIRT_VIEWER_WINDOW(value), i);
> >              i++;
> >          }
> 
> For what it's worth, the move of
> virt_viewer_app_get_initial_monitor_for_display() in
> app_window_try_fullscreen() could have been done in a preparatory
> commit.
> 

Good point, I'll pull that out into a separate patch.

Jonathon