Created attachment 89665 [details] [review]
Add _NET_FRAME_EXTENTS support

I quickly test the patch by looking at the windows in the pager, and it seems to work.

Elijah: I don't think I'm missing something, but can you verify?

*** Bug 344379 has been marked as a duplicate of this bug. ***

Vincent, the crazy madman patching machine.  I don't know how you fix so many things so quickly, but it's pretty awesome.  :-)

Most of the patch looks fine to me; I just saw one part that I wanted to comment on:

@@ -1972,9 +1980,9 @@ wnck_window_get_geometry (WnckWindow *wi
   if (yp)
     *yp = window->priv->y;
   if (widthp)
-    *widthp = window->priv->width;
+    *widthp = window->priv->width + window->priv->left_frame + window->priv->right_frame;
   if (heightp)
-    *heightp = window->priv->height;
+    *heightp = window->priv->height + window->priv->top_frame + window->priv->bottom_frame;
 }


Are window->priv->x and window->priv->y the location of the client window's upper left corner, or the frame's upper left corner?  From the documentation of wnck_window_get_geometry, I assume it's the former.  If so, then it would only make sense that modifying the width and height to account for the frame ought to be accompanied by changes to the x and y coordinates to account for it too.

Also, this is effectively an API change; some apps may be depending on the previous meaning of wnck_window_get_geometry.  I'm fine with changing it anyway, but we do need to update the documentation of the function, and we probably want to provide a wnck_window_get_client_window_geometry function (possibly with a better name) that returns what the old function used to.  Those could be useful, for example, in configure requests to the window manager.

(In reply to comment #3)
> Are window->priv->x and window->priv->y the location of the client window's
> upper left corner, or the frame's upper left corner?  From the documentation of
> wnck_window_get_geometry, I assume it's the former.  If so, then it would only
> make sense that modifying the width and height to account for the frame ought
> to be accompanied by changes to the x and y coordinates to account for it too.

Ah, good catch.

> Also, this is effectively an API change; some apps may be depending on the
> previous meaning of wnck_window_get_geometry.  I'm fine with changing it
> anyway, but we do need to update the documentation of the function, and we
> probably want to provide a wnck_window_get_client_window_geometry function
> (possibly with a better name) that returns what the old function used to. 
> Those could be useful, for example, in configure requests to the window
> manager.

Yes, I was wondering what was the best way to handle this. It seems to me that most people would understand wnck_window_set_geometry() and wnck_window_get_geometry() to be about the "same" geometry, so this API meaning change makes sense to me. Forgetting to update the doc is bad!

I was thinking that a new function to get the window geometry could be useful too, but I was just not sure.

Created attachment 89683 [details] [review]
Add _NET_FRAME_EXTENTS support, version 2

Fix x and y, add wnck_window_get_client_window_geometry() and update the docs.

Created attachment 89684 [details] [review]
Add _NET_FRAME_EXTENTS support, version 3

Makes it more robust in case the window manager is being a bad player .

Don't know why I didn't catch this earlier, but doesn't the is_in_viewport function have the same problem, namely that the x & y coordinates should be modified by frame size if the width and height are?

@@ -2128,8 +2185,8 @@ wnck_window_is_in_viewport (WnckWindow  
 
   window_rect.x = window->priv->x + viewport_rect.x;
   window_rect.y = window->priv->y + viewport_rect.y;
-  window_rect.width = window->priv->width;
-  window_rect.height = window->priv->height;
+  window_rect.width = window->priv->width + window->priv->left_frame + window->priv->right_frame;
+  window_rect.height = window->priv->height + window->priv->top_frame + window->priv->bottom_frame;


Also, as far as documentation goes for wnck_window_get_geometry:
  * Gets the size and position of @window, as last received
  * in a ConfigureNotify event (i.e. this call does not round-trip
  * to the server, just gets the last size we were notified of).
  * The X and Y coordinates are relative to the root window.
  *
  * The returned values take into account the frame that is added
  * by the window manager around windows. If you need to know the
  * actual size of @window, use wnck_window_get_client_window_geometry().
contradicts itself.  I know what you mean, but I'm afraid it might cause confusion because the size and position as last received in a ConfigureNotify event is not what we're returning.  We're modifying it.  Perhaps change to something like

  * Gets the size and position of @window, including decorations.  This
  * function uses the information last received in a ConfigureNotify
  * event and adjusts it according to the size of the frame that is
  * added by the window manager (this call does not round-trip to the
  * server, it just gets the last sizes that we were notified of).  The
  * X and Y coordinates are relative to the root window.
  *
  * If you need to know the actual size of @window ignoring the frame
  * added by the window manager, use
  * wnck_window_get_client_window_geometry().

Other than those two nits; it looks good to me.

Fixed the two issues. Thanks!