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

[Christophe Fergeau]

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?

>  
>      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.

ACK if my understanding about initial_display_map is correct.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20140924/14fc06ad/attachment.sig>