Glynn> My guess is that the WM makes the window the full size of Glynn> the screen, ignoring the gridding constraints, XEmacs Glynn> recomputes the correct size according to the gridding Glynn> constraints, and the WM then resizes the window to the new Glynn> size. If so, the WMs are broken. The WM should just say no: "you're maximized, I just set you that way, and that's that---shut up and get to work." See the ICCCM, and also the Xt shell spec. Why this gets into a loop I don't understand. There should be a flash, then XEmacs will docilely stay maximized. Re fixing: I guess there must be some protocol for maximizing by now so that XEmacs can find out that it is in fact "maximized", or we can use a heuristic of matching the size the WM specifies to the size of the screen (note that this *will* be heuristic, because XEmacs cannot know if there will be decorations or what size they are in this state; I guess we can query the Shell widget for its size, which might be more accurate.) Glynn> XEmacs probably shouldn't be specifying an absolute size. If the window size is specified in characters and the size of the characters change, it should. I *want* my window to stay 80 columns wide when I change from a 10pt font to a 14pt font, or vice versa. I know that modern text widgets (by Procrustes[tm]) don't care about that, but I do like it that way, and I don't see why XEmacs should change this behavior unless the users *want* their lines wrapped or truncated differently just because they changed font sizes. Glynn> it will be specified at startup to match the geometry Glynn> resource setting. Which is in character cells. Glynn> OTOH, the WM should probably ignore XEmacs' requested size Glynn> when the user has chosen to maximise the window. Precisely. The ICCCM doesn't allow the client to resize its toplevel windows. As far as I can tell, XEmacs does not try to do so. -- School of Systems and Information Engineering http://turnbull.sk.tsukuba.ac.jp University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN Ask not how you can "do" free software business; ask what your business can "do for" free software.

Glynn> I think that means that the client shouldn't use Glynn> XResizeWindow() etc on top-level shells. That's exactly my point. The WM is holding the smoking gun; its maintainers should stop pointing their fingers at XEmacs and saying "but you told us to shoot". I'm looking hard for XEmacs prints on the trigger; I haven't found any yet. I think it's crazy to say "but the app should know better to ask when it's been maximized at the user's request". It doesn't know (as you point out!), and in the X Window System model, it shouldn't need to know. That's the WM's job. There's probably still an XEmacs bug here. My prime candidate is that it's somehow calling XtGeometryRequest from a Resize procedure, which is explicitly forbidden in the Xt documents. If that propagates up to the Shell widget, it could result in the symptoms we hear about. However, in a defensively programmed WM, that should result in CPU consumption and useless ICCCM protocol, with no visible window jitter. But it's possible that there is no bug. The code for the relevant widget class methods doesn't seem to explicitly call XtGeometryRequest that I can find so far. And in fact layout is done in a separate "thread of control", ie, redisplay. So it's quite possible that having finished the management of the X window, we return to the read-eval-redisplay loop. At that point redisplay notices that its idea of window configuration and the actual are out of sync, and computes its idea of an optimal configuration, and makes the request for that "natural size". If the WM grants it, then says "oops, that window is supposed to be maximized" and remaximizes it, bingo! window jitter. While this doesn't seem like the brightest way to do things ex post, I don't think we can call it an actual bug in XEmacs. Anyway, if that hypothesis is correct, changing it will be pretty hard, I think. Am I missing something? N.B. The point is not to make excuses for XEmacs, it's to get the symptoms relieved. I think it should be relatively easy for WMs to do so; my experience so far is that it's quite hard for XEmacs. Glynn> I would assume that any change to the WM_NORMAL_HINTS Glynn> property /might/ result in the WM resizing the window. If Glynn> the resize in turn results in the client changing the Glynn> WM_NORMAL_HINTS property, that seems like it could create Glynn> the risk of a loop. True. And indeed, I believe that's what happened before the "Take XtGeometryNo for an answer" patch. However, in recent XEmacsen, as far as I know once they're told "no", they adjust their internal configuration to that given from outside, and the loop should stop. The immediate cause of the symptom is that the WM isn't saying "no". (Unless XEmacs is calling XConfigureWindow on the toplevel Shell's Window. But I don't think it does.) Glynn> But I suspect that XEmacs probably shouldn't change the Glynn> overall size in response to being resized by the WM. How does it know? Ultimately, *all* resizes are done by the WM. Anyway, the Xt model is simply "ask for what you want, and take what you are given." As far as I can tell, that's what XEmacs does. -- School of Systems and Information Engineering http://turnbull.sk.tsukuba.ac.jp University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN Ask not how you can "do" free software business; ask what your business can "do for" free software.

Glynn> Hehe; I've just written pretty much the same thing, before Glynn> having read this. Yeah, I saw that. I got a chuckle out of it. I sympathize with the users' predicament, but they're wrong. Maybe I should just see if I can fix metacity. If Havoc takes a patch, the rest of 'em are going to come under huge pressure whether he calls it a "bugfix" or a "defensive feature". :-) Nah, first I'm going to work on the patch suggested below. Then I'll have more credibility when I submit a metacity patch. :-) Glynn> I don't /think/ that Xt rules are relevant here. The reason I mention them is that geometry negotiation can go *up* from where it starts. That implies a request to the WM, eventually. If that happens in the middle of a WM configuration change, who knows what the result might be? Glynn> I'm assuming that shell widgets handle resize requests by Glynn> changing WM_NORMAL_HINTS rather than calling Glynn> XResizeWindow() etc. AFAICT, ours do. Glynn> The WM doesn't know what's going on within the client; Glynn> there isn't a right-time/wrong-time for changing property Glynn> settings. Oh, are you that sure that the WM is correctly implemented? ;-) Glynn> There's no endpoint where the client can't request a resize Glynn> from the WM until the processing is "complete", but can do Glynn> so after that point. Assuming that the WM authors understand that is a bit iffy IMAO. X-P Glynn> I suspect it depends upon whether the code which sets the Glynn> hints can figure out why it's setting them. Glynn> If you can distinguish the case where the parameters have Glynn> changed because of a WM-driven resize from the case where Glynn> they have been changed internally, you can avoid changing Glynn> the hints in reponse to WM actions. What *should* happen is that in the case of a WM-driven resize, size changes should propagate unidirectionally downward via widget Resize methods. Parents may XQueryGeometry their children, but then they *command* the resize they think best via Resize. Children must never XtConfigureRequest their parents. That latter is what I'm afraid is happening, in EmacsManagerChangeSize, called from EmacsFrameResize. It is true that EmacsFrameResize always does a x_wm_set_variable_size. However, this *always* ratifies the size demanded (in char cell units, which presumably is what confuses the WMs), and a well-behaved WM should ignore that in this case. There's also a comment in EmacsFrameResize which I interpret to mean that this change is intended to be entirely internal to XEmacs until the "next resize". Ah, but that can't be true. XEmacs doesn't own the WMShell resources, Xt does, and x_wm_set_variable_size does an XtSetValues on those resources. So that does propagate to the WMShell, and it must have a hook which does the WM protocol---how else to implement that? Glynn> That's how xterm behaves. If you resize the window using Glynn> the WM, the PSize doesn't change. If you select a different Glynn> font size from the Ctrl-RMB menu, it changes the PSize. AFAICT, XEmacs _never_ changes the PSize per se. When you say "PSize" do you include the case where one or more of base size, increment, or char cell dimensions change? Glynn> If the WM implements the concept of "maximised" windows, it Glynn> should probably ignore the WM_NORMAL_HINTS property for Glynn> such windows. The WM authors obviously disagree. Glynn> The former should result in the hints being changed, the Glynn> latter shouldn't. The WM has just told you what size you're Glynn> getting; requesting a specific size /in response/ seems Glynn> likely to be the cause of the troubles. Glynn> At the client<->WM level, it seems that it's the other way Glynn> around, i.e. every time it's given something, it replies Glynn> with what it would have rather had instead. OK, let's try simply not doing the x_wm_set_variable_size call in EmacsFrameResize. AFAICT that should have the effect of only changing the WM hints when XEmacs initiates the change. Thanks! I'm not sure I can reproduce (the 21.4.17 or so patch did fix for me on Mac OS X), but at least I can make sure that normal behavior isn't impaired. Then I'll post a patch. -- School of Systems and Information Engineering http://turnbull.sk.tsukuba.ac.jp University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN Ask not how you can "do" free software business; ask what your business can "do for" free software.

APPROVE COMMIT 21.5 RECOMMEND 21.4 Vin, there's a bunch of "future work" cruft here, so I don't necessarily recommend this patch as-is. Also, I haven't actually checked if it applies, and the backwards-compatibility variable might be a little bogus in the stable line. Let me know what, if anything of this you want. Thanks to Glynn Clements, without whose discussion I never would have figured this out, to the good folks at Sun, HP, etc, who funded Real Documentation that Actually Helps, and to the various users who kept reporting the issue even after we thought we (mostly) had it licked. I provided an earlier patch, about which I wrote: sjt> It is possible to inhibit this, with no huge ill-effect on sjt> XEmacs functionality. I'm supplying a patch, with this sjt> action controlled from Lisp by the variable `wedge-metacity', sjt> defaulting to nil. This patch is *superseded* by the following patch, which may not apply to 21.4 (sorry), and if you applied the earlier patch you'll have to revert it (sorry again). If you can apply it and are willing to test it, it should "just work", no need to fiddle with anything. I will work up a 21.4 patch per Vin's comments (if any) within a day or so. And it's being committed to 21.5 immediately. If you run with the patch and have minor problems (but enough that you're willing to avoid maximizing under metacity et al), you can restore the old buggy behavior with (setq wedge-metacity t). I'm pretty sure this patch is "right", although I haven't been able to test with metacity. (1) The Xt conventions say "don't call geometry request functions from the resize callback". This patch moves the request from a resize callback into a function explicitly intended to change size. The theory is right. (2) With the old behavior, the resize callback called itself recursively, at least once. Now it doesn't. (3) The fluxbox window manager now gives "true maximization" where it used to flash to the true maximum, then drop back to the largest cell-oriented window. Index: src/ChangeLog =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/ChangeLog,v retrieving revision 1.954 diff -u -r1.954 ChangeLog --- src/ChangeLog 6 May 2006 08:46:38 -0000 1.954 +++ src/ChangeLog 6 May 2006 17:26:00 -0000 ＠＠ -0,0 +1,26 ＠＠ +2006-04-30 Stephen J. Turnbull <stephen(a)xemacs.org&gt; + + Move geometry management from EmacsFrameResize to x_set_frame_size. + Should fix "metacity maximization" bug. + Provide (deprecated, temporary) backward compatibility option. + + * EmacsFrame.c (EmacsFrameResize): + * frame-x.c (x_set_frame_size): + Move call of ChangeEmacsManagerSize. + Don't bogusly notify WM about size changes the WM asked for. + + * console-x.c (wedge-metacity): New builtin Boolean Lisp variable. + * console-x-impl.h (wedge_metacity): Declare C variable. + * console-x.c (vars_of_console_x): New function to initialize it. + * symsinit.h (vars_of_console_x): declare it. + * emacs.c (main_1): Call vars_of_console_x. + + * EmacsFrameP.h (struct EmacsFrame): + * EmacsManager.c (ChangeEmacsManagerSize): + * EmacsShell-sub.c (SuperClassRootGeometryManager): + * console-x-impl.h (wedge_metacity): + Various comments, some improved documentation, mostly sad comments + on the state of the art of Xt programming. + + * frame-x.c (defi): #undef it. (Random code cleanliness.) + Index: src/EmacsFrame.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/EmacsFrame.c,v retrieving revision 1.28 diff -u -r1.28 EmacsFrame.c --- src/EmacsFrame.c 25 Oct 2005 11:16:20 -0000 1.28 +++ src/EmacsFrame.c 6 May 2006 17:26:00 -0000 ＠＠ -360,19 +360,32 ＠＠ pixel_to_char_size (f, ew->core.width, ew->core.height, &columns, &rows); change_frame_size (f, rows, columns, 0); - /* Now we tell the EmacsShell that we've changed the size of the non-fixed - portion of the frame. Note that, if we the resize occurred as a result - of EmacsFrameSetCharSize(), this information will be stored twice. - This is not a big deal, as storing this information doesn't actually - do anything until the next resize. */ - if (FRAME_X_TOP_LEVEL_FRAME_P (f)) - x_wm_set_variable_size (FRAME_X_SHELL_WIDGET (f), columns, rows); + /* The code below is just plain wrong. If the EmacsShell or EmacsManager + needs to know, they should just ask. If needed information is being + updated here, then we should set a dirty flag and have it updated on an + as-needed basis. + For now, conditionalize so people can get work done if this breaks + something. */ + if (wedge_metacity) /* cf. x_set_frame_size */ + { + /* Now we tell the EmacsShell that we've changed the size of the + non-fixed portion of the frame. Note that, if the resize occurred + as a result of EmacsFrameSetCharSize(), this information will be + stored twice. This is not a big deal, as storing this information + doesn't actually do anything until the next resize. */ + if (FRAME_X_TOP_LEVEL_FRAME_P (f)) + x_wm_set_variable_size (FRAME_X_SHELL_WIDGET (f), columns, rows); - /* Kick the manager so that it knows we've changed size. */ - req.request_mode = 0; - XtQueryGeometry (FRAME_X_CONTAINER_WIDGET (f), &req, &repl); - EmacsManagerChangeSize (FRAME_X_CONTAINER_WIDGET (f), repl.width, - repl.height); + /* Kick the manager so that it knows we've changed size. + #### No, no, no! If this does anything at all, it will involve + changing the manager's size. That's not something that a child + widget should initialize as part of a purely informational + method!! */ + req.request_mode = 0; + XtQueryGeometry (FRAME_X_CONTAINER_WIDGET (f), &req, &repl); + EmacsManagerChangeSize (FRAME_X_CONTAINER_WIDGET (f), + repl.width, repl.height); + } } static Boolean Index: src/EmacsFrameP.h =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/EmacsFrameP.h,v retrieving revision 1.7 diff -u -r1.7 EmacsFrameP.h --- src/EmacsFrameP.h 4 May 2003 02:34:35 -0000 1.7 +++ src/EmacsFrameP.h 6 May 2006 17:26:00 -0000 ＠＠ -65,13 +65,16 ＠＠ Boolean iconic; /* whether this frame is iconic */ /* The rest of this is crap and should be deleted. + #### Comments that start with + are fields that actually get referred + to somewhere aside from the init function. + I guess the "crap" has mostly moved to specifiers? */ Boolean minibuffer; /* 0: normal frames with minibuffers. * 1: frames without minibuffers * 2: minibuffer only. */ Boolean unsplittable; /* frame can only have one window */ - int internal_border_width; /* internal borders */ + int internal_border_width; /* + internal borders */ int scrollbar_width; /* width of frame vertical sb's */ int scrollbar_height; /* height of frame horizontal sb's */ int top_toolbar_height; /* height of top toolbar */ ＠＠ -82,9 +85,9 ＠＠ int bottom_toolbar_border_width; /* ... of bottom toolbar */ int left_toolbar_border_width; /* ... of left toolbar */ int right_toolbar_border_width; /* ... of right toolbar */ - Dimension toolbar_shadow_thickness; + Dimension toolbar_shadow_thickness; /* + of shadows */ unsigned char scrollbar_placement; - int interline; /* skips between lines */ + int interline; /* + skips between lines */ XFontStruct* font; /* font */ Pixel foreground_pixel; /* foreground */ ＠＠ -97,7 +100,7 ＠＠ int bell_volume; /* how loud is beep */ Boolean menubar_p; /* initially show a menubar? */ - Boolean initially_unmapped; /* inhibit initial window mapping */ + Boolean initially_unmapped; /* + inhibit initial window mapping */ Boolean use_backing_store; /* backing store for menubar & ew? */ Dimension preferred_width; /* if non-zero, preferred size for */ Index: src/EmacsManager.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/EmacsManager.c,v retrieving revision 1.7 diff -u -r1.7 EmacsManager.c --- src/EmacsManager.c 22 Dec 2004 10:59:09 -0000 1.7 +++ src/EmacsManager.c 6 May 2006 17:26:00 -0000 ＠＠ -241,6 +241,12 ＠＠ if (height == 0) height = w->core.height; + /* #### AFAICT this gets called in two places. One is in ChangeManaged(), + above. The other is in EmacsFrameResize(). Perhaps ChangeManaged() + should initiate resize requests, but EmacsFrameResize() should not. + Unfortunately, I've tried making this conditional on whether we're + called from EmacsFrameResize() or not, but that results in an infloop + via the callback to x_layout_widgets() in Resize(). Whee! */ /* do nothing if we're already that size */ if (w->core.width != width || w->core.height != height) { Index: src/EmacsShell-sub.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/EmacsShell-sub.c,v retrieving revision 1.7 diff -u -r1.7 EmacsShell-sub.c --- src/EmacsShell-sub.c 24 Jan 2005 23:33:46 -0000 1.7 +++ src/EmacsShell-sub.c 6 May 2006 17:26:00 -0000 ＠＠ -278,7 +278,8 ＠＠ GenericClassExtRec *gcer; /* find the shell extension record that specifies the - root geometry manager method */ + root geometry manager method + #### We could use XtGetClassExtension here. */ for (gcer = (GenericClassExtRec *) swc->shell_class.extension; gcer; gcer = (GenericClassExtRec *) gcer->next_extension) ＠＠ -287,6 +288,9 ＠＠ break; } + /* #### The R11.6.4 Xt specification says if we don't find NULLQUARK here, + we should assume root_geometry_manager = XtInheritRootGeometryManager. + Is that actually callable? */ if (!gcer) ABORT (); Index: src/console-x-impl.h =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/console-x-impl.h,v retrieving revision 1.5 diff -u -r1.5 console-x-impl.h --- src/console-x-impl.h 26 Nov 2005 11:46:07 -0000 1.5 +++ src/console-x-impl.h 6 May 2006 17:26:00 -0000 ＠＠ -40,6 +40,8 ＠＠ DECLARE_CONSOLE_TYPE (x); +extern int wedge_metacity; + struct x_device { #ifdef NEW_GC ＠＠ -234,7 +236,8 ＠＠ /* The maximum number of widgets that can be displayed above the text area at one time. Currently no more than 3 will ever actually be - displayed (menubar, psheet, debugger panel). */ + displayed (menubar, psheet, debugger panel). + #### Are "psheet" and "debugger panel" relevant any more? */ #define MAX_CONCURRENT_TOP_WIDGETS 8 struct x_frame ＠＠ -243,12 +246,17 ＠＠ struct lrecord_header header; #endif /* NEW_GC */ - /* The widget of this frame. This is an EmacsShell or an - ExternalShell. */ + /* The widget of this frame. + This is an EmacsShell or an ExternalShell. + It negotiates with the window manager or containing app on behalf of + the container widget. Should be (but isn't) invisible to Emacs. */ Widget widget; /* The parent of the EmacsFrame, the menubar, and the scrollbars. - This is an EmacsManager. */ + This is an EmacsManager. + It is responsible for managing the geometry of the frame. This is what + Emacs mostly talks to. Anything that affects its geometry will be + reflected in the Shell widget, and thus cause WM interaction. */ Widget container; /* The widget of the menubar, of whatever widget class it happens to be. */ Index: src/console-x.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/console-x.c,v retrieving revision 1.16 diff -u -r1.16 console-x.c --- src/console-x.c 17 Dec 2005 19:47:02 -0000 1.16 +++ src/console-x.c 6 May 2006 17:26:00 -0000 ＠＠ -41,6 +41,8 ＠＠ DEFINE_CONSOLE_TYPE (x); +int wedge_metacity; /* nonzero means update WM_HINTS always */ + extern void x_has_keysym (KeySym, Lisp_Object, int); static int ＠＠ -399,6 +401,22 ＠＠ CONSOLE_HAS_METHOD (x, perhaps_init_unseen_key_defaults); } + +void +vars_of_console_x (void) +{ + DEFVAR_BOOL ("wedge-metacity", &wedge_metacity /* +When non-nil, frame geometry management is backward-compatible. +This is known to create inflooping window jitter in metacity, et al. +It also does not conform to Xt conventions for geometry management. +Specifically, all frame resizes, XEmacs-initiated or not, update WM_HINTS. +Furthermore, geometry changes occur in the widget resize method. + +The default is nil. This probably gives correct behavior regardless of the +window manager used. +This variable is deprecated and will be removed. +*/ ); +} void reinit_console_type_create_x (void) Index: src/emacs.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/emacs.c,v retrieving revision 1.166 diff -u -r1.166 emacs.c --- src/emacs.c 25 Apr 2006 14:02:08 -0000 1.166 +++ src/emacs.c 6 May 2006 17:26:01 -0000 ＠＠ -2186,6 +2186,7 ＠＠ #ifdef HAVE_BALLOON_HELP vars_of_balloon_x (); #endif + vars_of_console_x (); vars_of_device_x (); #ifdef HAVE_X_DIALOGS vars_of_dialog_x (); Index: src/frame-x.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/frame-x.c,v retrieving revision 1.72 diff -u -r1.72 frame-x.c --- src/frame-x.c 26 Nov 2005 11:46:08 -0000 1.72 +++ src/frame-x.c 6 May 2006 17:26:01 -0000 ＠＠ -507,6 +507,7 ＠＠ defi(Qtop, XtNy); #undef def +#undef defi } static Lisp_Object ＠＠ -2307,6 +2308,17 ＠＠ x_set_frame_size (struct frame *f, int cols, int rows) { EmacsFrameSetCharSize (FRAME_X_TEXT_WIDGET (f), cols, rows); + + if (!wedge_metacity) /* cf. EmacsFrameResize */ + { + /* Kick the manager so that it knows we've changed size. */ + XtWidgetGeometry req, repl; + req.request_mode = 0; + XtQueryGeometry (FRAME_X_CONTAINER_WIDGET (f), &req, &repl); + EmacsManagerChangeSize (FRAME_X_CONTAINER_WIDGET (f), + repl.width, repl.height); + } + #if 0 /* this is not correct. x_set_frame_size() is called from Fset_frame_size(), which may or may not have been called Index: src/symsinit.h =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/symsinit.h,v retrieving revision 1.57 diff -u -r1.57 symsinit.h --- src/symsinit.h 25 Apr 2006 14:02:09 -0000 1.57 +++ src/symsinit.h 6 May 2006 17:26:01 -0000 ＠＠ -306,6 +306,7 ＠＠ void vars_of_console_stream (void); void vars_of_console_mswindows (void); void vars_of_console_tty (void); +void vars_of_console_x (void); void vars_of_data (void); void vars_of_database (void); void vars_of_debug (void); -- School of Systems and Information Engineering http://turnbull.sk.tsukuba.ac.jp University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN Ask not how you can "do" free software business; ask what your business can "do for" free software.

On Sun, 2006-05-07 at 02:48 +0900, Stephen J. Turnbull wrote: This patch fixes all window maximization problems here using 21.5.26 in GNOME (metacity 2.14.3) as well as KDE (kdebase 3.5.2). It doesn't apply to 21.4 as-is, so I haven't tested that. Thanks!

vin> "Stephen J. Turnbull" <stephen(a)xemacs.org&gt; writes: vin> Are you interested in generating a 21.4 version of this vin> patch? If not, I'd be happy to sift through it if you told vin> me which parts are relevant to the actual maximization bug. No problem; it's easier to do than to explain. Do you want the backwards compatibility variable, or just the fix? Steve -- School of Systems and Information Engineering http://turnbull.sk.tsukuba.ac.jp University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN Ask not how you can "do" free software business; ask what your business can "do for" free software.