GNOME Bugzilla – Bug 426519
modification of a Motif Shell's XmNx and XmNy fails if the shell has been moved by the user
Last modified: 2008-02-01 17:24:37 UTC
See http://bugs.opensolaris.org/view_bug.do?bug_id=6532909 for more details The problem is that straight after acting on the ConfigureNotify sent by the motif app. metacity receives a property notify on WM_NORMAL_HINTS and while querying window position (using meta_window_get_user_position) it is retrieving the last user set position (window->user_rect) instead of the one advertised by WM_NORMAL_HINTS. This is because the flag user_has_move_resized is never reset. Resetting this flag in meta_window_move_resize_internal fixes the problem. Here is a patch to fix this problem : diff -rup metacity-2.18.0/src/window.c ../metacity-2.18.0/src/window.c --- metacity-2.18.0/src/window.c 2007-04-05 10:17:19.349211000 +0200 +++ ../metacity-2.18.0/src/window.c 2007-04-05 10:16:51.055657000 +0200 @@ -3354,6 +3354,8 @@ meta_window_move_resize_internal (MetaWi &window->user_rect.x, &window->user_rect.y); } + else + window->user_has_move_resized = FALSE; if (need_move_frame || need_resize_frame || need_move_client || need_resize_client) Let me know if I can commit this fix.
The opensolaris bug mentions an attachment with a testcase, but I can't find anywhere on that page a link to the said attachment. Could you upload it here so I can try it out?
Created attachment 85886 [details] test case originally attached to the opensolaris bug Here is the test case originally attached to the opensolaris bug
Resetting the flag contradicts its purpose. See window.h: /* Track whether the user has ever manually modified * the window; if so, we can use the saved user size/pos */ guint user_has_move_resized : 1; I think the core of the problem is in meta_window_move_resize_now(). Note this comment: /* This used to use the user width/height if the user hadn't resized, * but it turns out that breaks things pretty often, because configure * requests from the app or size hints changes from the app frequently * reflect user actions such as changing terminal font size * or expanding a disclosure triangle. The function now confusingly mixes window->user_rect position with window->rect width/height. That combo doesn't work for this case (and seems odd to use in general). I don't see why the function simply doesn't use window->user_rect in both cases, while just making sure that user_rect reflects user actions. In other words, set user_rect after things such as configure requests. Besides, the purpose of user_rect is to allow the window to be moved in reaction to a new strut, and then be moved back if that strut goes away. I would expect the window to move towards a previous ConfigureRequest position if there was one. Am I missing something?
Created attachment 85978 [details] [review] Set user_rect in response to configure request. This patch also does some code cleanup. It seems there are many occurrences of code doing some_rect->width = window->rect.width; some_rect->height = window->rect.height; meta_window_get_user_position (window, &some_rect->x, &some_rect->y); so I made a simple meta_window_get_client_rect() function to do that.
Created attachment 85979 [details] [review] Same patch, more cleanups and comments in meta_window_configure_request
I was going to let this sit for a bit longer to wait for possible feedback and get some more testing, but decided to go ahead and apply to the development branch (i.e. trunk). I figure feedback would have come quickly, or else it won't come until it's actually committed and tested. (I did make a slight change to the new function name, though)
*** Bug 101561 has been marked as a duplicate of this bug. ***
Just tested the patch it works fine. Thanks.
This patch breaks the 'xinerama' patch, that fixes the shakeloose code for unidirectionally windows on multi-head setups. On order to fix this, I need to understand fully what you mean with "user_rect". In fact, it couldn't hurt to also define EXACTLY what "rect" and "saved_rect" mean. My interpretations are: rect: The current (visible / drawn) rectangle of the window. This includes any state; for example, if a window is maximized, then that is reflected in the value of this variable. saved_rect: The position of the window if it were unmaximized. This means that toggling the (unidirectional) maximization state of a window will not alter this variable. However, moving an unidirectional maximized window will cause one coordinate of this variable to be updated. user_rect: No idea what this means.
I don't remember all the rects exactly, but the way I would define window->rect is that it's the same rectangle the X server has stored for the window. So anytime we _know_ the X server size (due to ConfigureNotify, or because we did XConfigureWindow) then window->rect is updated. window->rect should never have any other value, it is a cache of the server state.
Ok, thanks for the explanation. I've always considered rect as a read-only variable in my patches. The "xinerama" patch (which fixes the use of metacity on multi-head setups in regard to the use of unidirectional maximized windows) does change saved_rect and user_rect in the same way now: Both variables can be looked at as "past" values (values that were in the past), but this is in correct: saved_rect NEEDS to be updated when an unidirectional maximized window is moved; it's name is therefore misleading. Instead, it is better to consider both values as "future" values: possible values that will be used in the future; albeit initialized with "past" values. The point were the "xinerama" patch needs to change these values is as follows: If a (partially) maximized window is shaken loose and moved to a different monitor where it is "reattached" (meaning, it maximizes again - but now on the different monitor) then what the user basically tried to do is have that window SWITCH MONITOR. Therefore, also the (future positions) saved_rect and user_rect need to "switch monitor": because we never want an (un)maximze event cause a window to jump monitor. The most logical thing to do in the case that the old and new monitor have the same geometry is to simply use the same coordinates relative to the monitor. It remains a bit of heuristic, but it's not THAT bad when window unmaximizes to a relatively different position as a result of different monitor geometries, or the presence of different structs. The most important part is that the target monitor remains the same.