Created attachment 133538 [details]
Screenshot of incorrect fullscreen vinagre

This shows it even worse with the fullscreen display also shifted from the left of the screen.

Still present in 2.26.2

Created attachment 137336 [details]
Vinagre in fullscreen mode

Actually, for me I see that it only properly enters full screen mode 1 time in 10 or 12, the rest of the time it's messed up as shown in the screenshot.

My guess is that the code to do the fullscreening checks the window's state, and sees whether the window is fullscreen or not. That's broken, as the window state changes async.

Instead you should be using gdk_window_fullscreen/unfullscreen on its own, based on the previous state.

The actual hiding/showing of elements should be done in the callback for the "window-state-event" signal (which is where you would record the previous state as well).

See window_state_event_cb() in totem/src/totem-object.c for example.

As far as I can see, vinagre does use gtk_window_[un]fullscreen - see:

http://svn.gnome.org/viewvc/vinagre/trunk/src/vinagre-window.c?revision=534&view=markup

In particular around line 1078. So I think that's not the problem, but I may have misunderstood.

Oh, I just re-read comment #5 which refers to gdk_window_fullscreen - is there anything wrong with using gtk_window_fullscreen?

Created attachment 250625 [details] [review]
proposed patch

this patch fix this issue.

david, if approved, may I push this to 3.8 as well?

Created attachment 250639 [details] [review]
proposed patch, rdp

this patch fixes the rdp plugin

Review of attachment 250639 [details] [review]:

Hi Jonh, is there any reason why this should not be in the default_get_dimensions() virtual function? Taking the dimensions from the GdkWindow for the tab widget seems like a reasonable default.

Review of attachment 250625 [details] [review]:

The patch seems fine, and should be fine for the stable branch. Please drop the "vnc:" prefix from the first line of the commit message, and instead add it inline, for example "Implement get_dimensions() for VNC" (keeping in mind a limit of 50 characters). For the remainder of the commit message, please make the first letter a capital, and use up to 72 characters per line. There is also a small typo: please add "to" between "whether" and "show".

(In reply to comment #9)
> Review of attachment 250639 [details] [review]:
> 
> Hi Jonh, is there any reason why this should not be in the
> default_get_dimensions() virtual function? Taking the dimensions from the
> GdkWindow for the tab widget seems like a reasonable default.

because this wouldn't get the real size of the widget. the widget is put inside a scrolledwindow because it can be potentially larger than window size.

so, it's up to the implementation (who have access to the internal gdkwindow) to return the correct size.

just for reference, I tried to do that in vinagre-tab.c:

static void
default_get_dimensions (VinagreTab *tab, int *w, int *h)
{
  GdkWindow *window = gtk_widget_get_window (GTK_WIDGET (tab));

  if (window)
    {
      *w = gdk_window_get_width (window);
      *h = gdk_window_get_height (window);
      g_print ("have window!!   w = %d, h = %d\n", *w, *h);
    }
  else
    {
      *w = -1;
      *h = -1;
      g_print ("don't have window...\n");
    }
}

and it didn't work...

Review of attachment 250639 [details] [review]:

Ugh, then it seems it has to be done in each plugin. The same comments apply as for the VNC patch. Thanks!

pushed with those changes in commit message to master and 3.8