After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 91481 - Window history saving
Window history saving
Status: RESOLVED OBSOLETE
Product: metacity
Classification: Other
Component: general
unspecified
Other Linux
: Normal enhancement
: future
Assigned To: Metacity maintainers list
Metacity maintainers list
: 81802 114373 142645 343857 478721 (view as bug list)
Depends on:
Blocks: 81802
 
 
Reported: 2002-08-23 04:10 UTC by Havoc Pennington
Modified: 2007-09-21 23:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
This patch adds window history to placement. Doesn't yet handle multiple windows of the same type. (30.32 KB, patch)
2002-11-11 22:01 UTC, Benjamin Kahn
none Details | Review
New window history patch. This patch auto-numbers similar windows (36.37 KB, patch)
2002-11-20 04:04 UTC, Benjamin Kahn
none Details | Review
Third patch. This adds saving of sticky/maximized/fullscreen state. It also remmbers window size. (56.44 KB, patch)
2002-12-18 22:43 UTC, Benjamin Kahn
none Details | Review
Update patch to work against 2.4.8 (54.72 KB, patch)
2002-12-19 20:57 UTC, Benjamin Kahn
none Details | Review
Window History Patch with _NET_WM_GEOMETRY_SAVE_ID support (61.27 KB, patch)
2003-01-02 23:39 UTC, Benjamin Kahn
none Details | Review
Patch that implements the "visually centering" first-fit algorithm proposed (4.01 KB, patch)
2003-01-04 16:21 UTC, Rob Adams
none Details | Review
History patch with SAVE_ID which actually works. (61.62 KB, patch)
2003-01-07 22:58 UTC, Benjamin Kahn
none Details | Review
Correct bug saving geometry information on some windows to disk. (98.03 KB, patch)
2003-01-09 21:40 UTC, Benjamin Kahn
none Details | Review
Window History Patch (part 1): create the new atom (4.51 KB, patch)
2003-03-06 22:33 UTC, Benjamin Kahn
needs-work Details | Review
Window History Patch (part 2): Adding History Saving (56.48 KB, patch)
2003-03-06 22:34 UTC, Benjamin Kahn
needs-work Details | Review
Create the atom (second patch) (4.35 KB, patch)
2004-09-12 03:09 UTC, Benjamin Kahn
none Details | Review
Updated Window History Patch against metacity 2.8.1 (545.31 KB, patch)
2004-09-12 03:11 UTC, Benjamin Kahn
none Details | Review
New version of the atom patch. (4.85 KB, patch)
2004-11-01 22:28 UTC, Benjamin Kahn
needs-work Details | Review
Window History Patch (57.13 KB, patch)
2004-11-01 22:28 UTC, Benjamin Kahn
needs-work Details | Review
Patch against metacity CVS for two list window history saving (53.33 KB, patch)
2006-04-18 03:47 UTC, Benjamin Kahn
reviewed Details | Review

Description Havoc Pennington 2002-08-23 04:10:54 UTC
To get started, here is a reference pointed out by David Lazaro for OS X:

http://developer.apple.com/techpubs/macosx/Essentials/AquaHIGuidelines/AHIGWindows/Positioning_Windows.html

The Windows XP placement algorithm is totally inconsistent and random and
depends on the application, as far as I can tell.

What metacity does right now for normal application windows is:
 - attempts first fit, preferring to stack windows vertically 
   down the left side, then start a new column and stack, 
   then start a new column and stack
 - if no first fit, cascade from top-left corner

for dialogs, it centers horizontally over the parent window, and 
places a bit down from the parent's titlebar vertically; or if no 
parent, centers the dialog.

If an application sets the window position of course then metacity isn't
involved at all. This happens more than one might like.
Comment 1 Heath Harrelson 2002-10-30 15:45:08 UTC
Batch adding GNOME2 keyword to Metacity bugs.  Sorry for the spam.
Comment 2 Benjamin Kahn 2002-11-11 21:59:54 UTC
I believe that metacity should keep track of where the user placed the
window.  This would mean that metacity's placement would look like
this instead:

What metacity does right now for normal application windows is:
 - attempt to find the last place a window with that class and name
   were positioned.  If nothing matches, continue to the next 
   step.  If a window matches, check for a window placed somewhat 
   near there.  If nothing nearby is found, place the window there.
 - attempt first fit, preferring to stack windows vertically 
   down the left side, then start a new column and stack, 
   then start a new column and stack
 - if no first fit, cascade from top-left corner

Comment 3 Benjamin Kahn 2002-11-11 22:01:22 UTC
Created attachment 12240 [details] [review]
This patch adds window history to placement.  Doesn't yet handle multiple windows of the same type.
Comment 4 Benjamin Kahn 2002-11-11 22:03:13 UTC
Notice that the patch I've attached above SAVES window size
information but does not yet support restoring it.  I haven't decided
whether it's a good idea to restore this information.
Comment 5 Havoc Pennington 2002-11-11 22:14:29 UTC
class/name doesn't work, many apps have dozens of distinct windows
with the same class/name. There has been a lot of discussion of this on 
wm-spec-list@gnome.org, also some on kde-core-devel@kde.org
about using the role hint instead (though there are some problems with
using role as well that owen pointed out to me).

I'm not willing to remember window history if it's going to generate bug 
reports or be unreliable, because I'm not willing to make it
configurable; if it exists it has to work and for everyone.

Note that there's another bug maybe under "EWMH spec" component that
might have more discussion of "matched windows" or remembering window 
positions.
Comment 6 Rodd Clarkson 2002-11-12 03:29:54 UTC
I would agree with the MacOS reading except to say I don't believe
that window placement should be centered horizontally,  Centering
horizontally would contribute to a huge was of screen resources on the
left side of the screen with no other (apparent) user benefit.

Whatever happens, the default behaviour of metacity in regard to
window placement is inconsistent.  Moving from a 'best fit' model to a
'cascade' model might seem okay, but it makes new windows placement
hard to predict and therefor requires that the user most stop and
'think' where the window will appear instead of knowing were it will
appear.

Some worthwhile points:
'Best fit' rarely places the window in the right place.
Given that windows come in different shapes and sizes, moving between
best fit and cascade means that smaller windows will often be placed
in gaps on the desktop, while larger windows will often be placed at
the top left.

Both of these factors contribute to confusion about where the next
window will appear.

I would be much happier with window placement if metacity dropped the
best fit model and just used the current cascade model (which isn't
truly cascade as seems to reset itself).

Comment 7 Benjamin Kahn 2002-11-12 06:46:38 UTC
So it's pretty easy to change the way it hashes the windows.  (Role
seems like a nice thing to add, although it seems to always be blank.
 Maybe I'm missing something there.)  But notice that this patch
doesn't have problems with dozens of windows of all the same type --
it will place the first window and fall through to the old placement
systems for the rest of the windows.  In practice this seems to work
fairly well.

I'm working on the next generation of this patch which auto-numbers
windows to allow for dozens of similar windows with tracking of all of
them.  

I'll see if I can find the "window matching" discussions held
elsewhere.  (Although this isn't full window matching.  It's simply
remembering placement of windows similar to the macintosh scheme
described in the web link above.)
Comment 8 Benjamin Kahn 2002-11-20 04:04:48 UTC
Created attachment 12419 [details] [review]
New window history patch.  This patch auto-numbers similar windows
Comment 9 Benjamin Kahn 2002-11-20 04:21:35 UTC
This patch uses a hash of class/name/role/number.  Since name can
change, this patch also remembers the first name of the window and
uses that.  It also changes the placement mode to NOT ignore minimized
windows.  (Since that will almost certainly make numbering less
accurate.)  

Window numbers are created when the window is mapped.  It starts at 0,
and goes up.  When a window is moved, the number may be changed if a
lower number window was closed earlier.
Comment 10 Havoc Pennington 2002-11-20 05:16:36 UTC
I find this patch strangely appealing.

A random code comment: we should try to share some of the utility
functions etc. with session.c

A more substantive comment: I would feel much more comfortable if 
I could state definitively what an app has to do to make the window 
history reliable. 

What about this idea:
http://mail.gnome.org/archives/wm-spec-list/2001-August/msg00044.html

We make up a unique ID for every entry we save in the history, and set
it on the window. If an app restores that ID, we always prefer using
it rather than any of our heuristics (class/title/role/etc.)

Just an idea. There was a much longer thread on this on wm-spec-list
here: http://mail.gnome.org/archives/wm-spec-list/2002-May/msg00030.html

A hard implementation question: when do we garbage collect the window
history? Or does it grow infinitely? My suggestion is to expire
entries after some time of non-use, though this has the annoying
property that if you mess up your clock for a moment you'll lose all
your history, that's probably OK.

Another question: are we going to save other state?
I think it probably makes sense to save stickiness, but not the
current workspace, for example. I dunno.
Comment 11 Dave Bordoley [Not Reading Bug Mail] 2002-11-20 06:19:25 UTC
just for the sake of mentioning it, will this break/conflict with the
saving of window positions in nautilus, and furthermore is window
position for documents etc really just file metadata that should be
stored via some sort of generic wm agnostic way? I'm not sure of the
answers, but thought i'd raise the issues.
Comment 12 Benjamin Kahn 2002-11-20 23:03:07 UTC
I'm not sure it's a good idea to garbage collect the window history. 
It seems like pruning the list shouldn't be needed.  The list can get
pretty big, but that only delays metacity startup and shutdown times.
 (And even then, it shouldn't get too big.)  (It also can effect
memory usage, of course.)  I can add a time stamp to the entries in
the file for possible future pruning, I suppose...

The window ID isn't a bad idea, although I'm not all that excited
about generating such an id myself.  (I could just push the key I'm
using now if none exists, I suppose -- a sample GNOME Terminal looks
like this:  "Gnome Terminal/Gnome Terminal/ /3" and use a different
key if the application has set one.)

Saving an application's sticky state seems like a good idea.  I'll add
that.  I'm going to test restoring window size as well.  I imagine
I'll also want to save maximized and full screen state as well.  
Comment 13 Havoc Pennington 2002-11-20 23:15:30 UTC
If you save maximized state remember you have to save 
the window->saved_rect (though I just noticed that session.c does
this a stupid way; it should just save saved_rect as the 
actual window size, and not bother saving the while-maximized window 
size).

I'm not comfortable with an infinite file, I think it has to be
resolved somehow. Maybe "we keep up to N most recent windows per
window class" then timestamp the window class and drop all windows in
a class after some timeout with no windows in that class. That might 
be somewhat more robust than keeping the global N most recent entries, 
in case someone opens a ton of windows for some particular app, 
maybe wm-tester --evil. Also, we could only impose the "N entries" 
limit if the file reached some particular threshold size, 
e.g. as long as the file is below 64K we could let it contain 
any number of entries.

I do want some way for an app to reliably tell metacity "this 
window is a new window and is not any window you've saved history for"
and "this window is exactly the same as past window
<some-id-mechanism> you have saved in the past".
Really this mechanism should be in the EWMH though Matthias was 
not getting the need for it which is why it isn't. I think we need
to bring this up on wm-spec-list again prior to applying any patch.
Comment 14 Benjamin Kahn 2002-11-21 03:25:48 UTC
Okay.  I'm adding this information to my spec:

If the _NET_WM_GEOMETRY_SAVE_ID window property is set but blank, we
will ignore this window in terms of window history.

If the _NET_WM_GEOMETRY_SAVE_ID window property is set, we will use
this value as the key for the window.

If the _NET_WM_GEOMETRY_SAVE_ID window property is not set, we will
create the property with the key we use internally.  

I've changed the file format for windows to look like this:

<window id="bar" class="XTerm" name="xterm" title="/foo/bar"
role="blah" type="normal" num="0" timeset="1037848165">
  <geometry x="100" y="100" width="200" height="200" gravity="northwest"/>
  <sticky/>
  <maximized/>
</window>

I'm also fixing the patch to stop re-calculating the proper index
number during the actual window drag, and instead calculate it only
when the window is dropped.
Comment 15 Benjamin Kahn 2002-12-18 22:43:41 UTC
Created attachment 13098 [details] [review]
Third patch.  This adds saving of sticky/maximized/fullscreen state.  It also remmbers window size.
Comment 16 Benjamin Kahn 2002-12-18 22:52:32 UTC
I just added a new version of this patch.  (Sorry for the delay, this
patch is a lot bigger (almost twice the size!) of the last patch and
it was hard to debug function calls.)  

This patch is so large because I have also added debugging functions
and documented what I'd been doing.

Fixed a crasher when an application doesn't set a class or name of a
window (glxgears)
Added window size save/restore
Added maximized/sticky/full screen state save/restore
Began adding _NET_WM_GEOMETRY_SAVE_ID support
Only recalculate the window number when a window is "dropped" to
conserve CPU cycles.

Known Bugs:  Doesn't yet support the _NET_WM_GEOMETRY_SAVE_ID Xproperty.
Comment 17 Benjamin Kahn 2002-12-19 20:57:49 UTC
Created attachment 13116 [details] [review]
Update patch to work against 2.4.8
Comment 18 Benjamin Kahn 2003-01-02 23:39:46 UTC
Created attachment 13322 [details] [review]
Window History Patch with _NET_WM_GEOMETRY_SAVE_ID support
Comment 19 Benjamin Kahn 2003-01-03 00:03:12 UTC
This patch adds _NET_WM_GEOMETRY_SAVE_ID support.  If the
_NET_WM_GEOMETRY_SAVE_ID atom is set on the window before the window
appears, metacity will use that string to manage the window history. 
If the atom is set to a NULL string or an empty string, this patch
will make Metacity not save the history of that window.

Setting the _NET_WM_GEOMETRY_SAVE_ID atom AFTER the window has been
mapped will make Metacity switch to tracking the window with the new
identifier but, of course, will not look up the saved information for
that identifier and move the window.
Comment 20 Rob Adams 2003-01-04 15:06:07 UTC
this bug seems to have deviated from its initial thrust somewhat, but
I have a proposal for you.  I find the idea of putting windows in the
upper left corner a bit ugly, but you don't want to center them
because it's nice to be able to, say, open 4 xterms on a 1024x768
display and have them perfectly tile the screen.

So here's the idea: when placing windows onto an empty head, calculate
the effective width of the window (with decorations) and take fluff =
(head width) % (window width).  This is the amount of screen space
"left over" on the right of the screen if you tile as many windows of
that type as will fit.  Then take fluff/2 and leave that much room on
the left of the screen when the window is first mapped.  Do the same
thing for the height.

This has the effect of centering large windows that won't tile well
anyway, and putting smaller windows in "good" places to ensure that
they will tile well, while avoiding the upper left corner syndrome. 
It's also very easy to implement.  What do you think?
Comment 21 Rob Adams 2003-01-04 15:38:03 UTC
Actually, even better: for height add fluff/4 instead of fluff/2 to
visually center the windows.

I've experimented with this a bit and I find the results extremely
pleasant.  I'm going to have to rework the multihead placement a bit
to get it to work with multiple screens well, but that's my fault in
the first place so it's only fair...
Comment 22 Rob Adams 2003-01-04 16:21:41 UTC
Created attachment 13349 [details] [review]
Patch that implements the "visually centering" first-fit algorithm proposed
Comment 23 Benjamin Kahn 2003-01-07 22:58:30 UTC
Created attachment 13409 [details] [review]
History patch with SAVE_ID which actually works.
Comment 24 Benjamin Kahn 2003-01-07 23:00:35 UTC
Ahem.  Please ignore my last patch.  (#5)  It didn't work due to a
small problem with my logic dealing with the property.  This new patch
works perfectly.  BTW -- with readams's window placement patch as
well, metacity's placement policy feels perfect to me.  It's difficult
to go back to anything else.
Comment 25 Havoc Pennington 2003-01-07 23:32:34 UTC
Thanks, I want to look at this feature as one of the first things
after we branch the GNOME 2.2 stable branch.

Did we ever write up SAVE_ID and post to wm-spec-list?
Comment 26 Benjamin Kahn 2003-01-08 18:22:31 UTC
Nope.  I'm going to work on that today and tomorrow.  I just sub'd to
the list.
Comment 27 Benjamin Kahn 2003-01-09 18:23:08 UTC
crap.  I just realized I'm not saving information from windows with
_NET_WM_GEOMETRY_SAVE_ID set.  (I remember them in memory, but they do
not get saved to disk when metacity quits.)  It's actually a hard
problem because I save the _NET_WM_GEOMETRY_SAVE_ID property
information in the window structure, not the history structure.  I'll
tackle this today.

In the meantime, Havoc, I've deviated from your original suggestion
(
http://mail.gnome.org/archives/wm-spec-list/2001-August/msg00044.html
) slightly in that I do not set _NET_WM_GEOMETRY_SAVE_ID if the
application has not set it.
Comment 28 Benjamin Kahn 2003-01-09 21:40:06 UTC
Created attachment 13452 [details] [review]
Correct bug saving geometry information on some windows to disk.
Comment 29 Benjamin Kahn 2003-02-05 18:26:44 UTC
There are now two bugs that I know about in this patch:

* In some cases, windows with no title or class set can cause metacity
to crash.  (The fix is easy:  history.c:344 :   if (!key) return NULL;)

* Windows which are being resized by the placement algorithm can
sometimes appear at the wrong size, or with funny titlebars.  I
haven't been able to track this down yet, but I assume the problem is
around line 381 of history.c.

When I've tracked down the second issue, I will generate a new patch
against metacity HEAD.
Comment 30 Havoc Pennington 2003-02-25 09:00:22 UTC
Ah, here is the other history-saving bug. We should merge the two.
The rest of the placement stuff is in CVS already.
Comment 31 Benjamin Kahn 2003-02-26 20:23:17 UTC
I'll generate a new patch against HEAD.  (I assume the atom stuff will
need to be updated, for example.)

Note that I'm still having problems when resizing some windows.
Comment 32 Benjamin Kahn 2003-03-06 22:33:28 UTC
Created attachment 14822 [details] [review]
Window History Patch (part 1):  create the new atom
Comment 33 Benjamin Kahn 2003-03-06 22:34:22 UTC
Created attachment 14823 [details] [review]
Window History Patch (part 2): Adding History Saving
Comment 34 Benjamin Kahn 2003-03-06 22:39:09 UTC
What's new:
Atom addition in a separate patch.  This should make sure the patch is
easier to apply against a wider variety of metacitys.  I should have
split out the debugging stuff, which also causes merging problems.

Take out (some) direct manipulation of the window structure in
history.c which was causing redraw issues.

Take out ROLE from the unique string.  It was conflicting with
gnome-terminal and will probably conflict with other applications in
the future.

Still to do:

Rename and history file to contain screen resolution and screen number
to avoid conflicts on multi-rooted home directories.

Add auto-numbering for windows using NET_WM_SAVE_ID to avoid
conflicts.  It's better to remove some degree of application control
since these should be unique anyway.
Comment 35 Rob Adams 2003-03-30 11:20:01 UTC
what's the state of the window history patch?  Has this been abandoned?
Comment 36 Benjamin Kahn 2003-03-30 16:07:59 UTC
No.  Sorry for the lack of comments recently.  Here's the status:

GNOME Terminal sets an absurd role at the moment.  This leads to the
history never being restored, and thousands of bogus entries being
remembered.

I was testing a patch which just didn't use Role at all, but that lead
to all sorts of sadness.  Now I use the window role if set, or the
first value window title is set to if role isn't set.  I also have an
exception for GNOME Terminal which makes it ignore the role completely.

I have gotten bug reports from users that each unique window type
should have only one window size.  For example, Evolution's composer
window.  If I drag that to a specific size, all future windows should
be the same size.

I am currently testing this behavior.  I've made size the only
universal state.  Window position and maximization, sticky, and full
screen are all per window number.

If you have multiple screens or log in on multiple machine, things can
get weird.  So I'm renaming the history file to contain screen number
and resolution.  This also requires multiple history lists for
multiple screens.  I don't have this patch coded yet.

When the new behavior stuff is tested a little more, I'll post the new
patch.
Comment 37 Rob Adams 2003-03-31 16:13:06 UTC
*** Bug 81802 has been marked as a duplicate of this bug. ***
Comment 38 Rob Adams 2003-04-15 20:34:28 UTC
updating the summary on this bug to match current focus
Comment 39 Rob Adams 2003-06-04 00:43:28 UTC
*** Bug 114373 has been marked as a duplicate of this bug. ***
Comment 40 Rob Adams 2003-07-04 18:37:47 UTC
marking Normal priority and removing PATCH keyword, since the patch
isn't finished yet.
Comment 41 Rob Adams 2004-07-31 20:59:30 UTC
*** Bug 142645 has been marked as a duplicate of this bug. ***
Comment 42 Bryan W Clark 2004-08-02 17:21:51 UTC
I'm putting the PATCH keyword back in here, the new bugzilla allows us to mark
attachments with status so I'll mark the two patches as "needs_work"

Ben or Havoc, any status on this?
Comment 43 Benjamin Kahn 2004-08-02 17:39:17 UTC
No new status at this point due to seeming lack of interest from users and the
problems with applications setting random role values.  

At last count, the problems that need to be addressed are:

* Saved window coordinates need to be expired at certain times.
* The save file should be different at different screen resolution.
* Window size should not be saved
Comment 44 Benjamin Kahn 2004-09-12 03:09:43 UTC
Created attachment 31498 [details] [review]
Create the atom (second patch)
Comment 45 Benjamin Kahn 2004-09-12 03:11:18 UTC
Created attachment 31499 [details] [review]
Updated Window History Patch against metacity 2.8.1
Comment 46 Benjamin Kahn 2004-09-12 03:12:47 UTC
Okay.  This version should compile against recent versions of metacity.  I've
simply made the patch compile and done a few tests to make sure it seems to
work.  There could be regressions since I didn't test all the features.  I plan
to start testing this patch again soon.
Comment 47 Elijah Newren 2004-09-12 16:04:52 UTC
In regards to comment 36 and comment 43, isn't wm_window_role supposed to be
"random" in that it needs to be unique?  Or am I just reading
http://tronche.com/gui/x/icccm/sec-5.html wrong?
Comment 48 Peter O'Shea 2004-09-13 16:00:25 UTC
To follow-up on comment 43, I personally am very interested in this bug...it's
the only thing about metacity I don't like.  Every time I open an app, I end up
dragging it to exactly where I had it the last time.  Aren't computers supposed
to relieve us of such drudgery?

I tried applying the patch to metacity-2.8.1, but it doesn't seem to apply cleanly:
bugs{poshea}203: patch -p1 <metacity-patch.txt
patching file src/display.c
Hunk #2 succeeded at 3178 (offset -2 lines).
patching file src/history.c
patching file src/history.c~
patching file src/history.h
patching file src/main.c
patching file src/Makefile.am
patching file src/Makefile.in
patching file src/Makefile.in~
patching file src/place.c
patching file src/session.c
Hunk #1 succeeded at 662 (offset -18 lines).
patching file src/util.c
patching file src/util.h
patching file src/window.c
patching file src/window.c~
patching file src/window.c.orig
patching file src/window.h

If I proceed anyway and configure/compile (on sparc Solaris 8 with gcc-3.3.4), I
run into this:

gmake[2]: Entering directory `/home/poshea/usr/source/gnome2.6/metacity-2.8.1/src'
/home/poshea/usr/bin/gcc -DHAVE_CONFIG_H -I. -I. -I.. -DXTHREADS -DORBIT2=1
-threads -I/home/poshea/usr/include/gtk-2.0
-I/home/poshea/usr/lib/gtk-2.0/include -I/home/poshea/usr/include/atk-1.0
-I/home/poshea/usr/include/pango-1.0 -I/home/poshea/usr/include
-I/home/poshea/usr/include/freetype2 -I/usr/openwin/include
-I/home/poshea/usr/include/glib-2.0 -I/home/poshea/usr/lib/glib-2.0/include
-I/home/poshea/usr/include/gconf/2 -I/home/poshea/usr/include/orbit-2.0
-I/home/poshea/usr/include/startup-notification-1.0  
-DMETACITY_LIBEXECDIR=\"/home/poshea/usr/libexec\"
-DMETACITY_LIBDIR=\"/home/poshea/usr/lib\" -DHOST_ALIAS=\"\"
-DMETACITY_LOCALEDIR=\"/home/poshea/usr/share/locale\"
-DMETACITY_PKGDATADIR=\"/home/poshea/usr/share/metacity\"
-DMETACITY_DATADIR=\"/home/poshea/usr/share\" -DG_LOG_DOMAIN=\"metacity\"
-DSN_API_NOT_YET_FROZEN=1  -I/home/poshea/usr/include  -g -O2 -mcpu=ultrasparc
-I/home/poshea/usr/include -Wall -Wchar-subscripts -Wmissing-declarations
-Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wcast-align
-Wsign-compare -c window.c
In file included from display.h:32,
                 from screen.h:26,
                 from window.h:28,
                 from window.c:25:
/usr/openwin/include/X11/Xlib.h:32: warning: ignoring #pragma ident 
In file included from screen.h:27,
                 from window.h:28,
                 from window.c:25:
/usr/openwin/include/X11/Xutil.h:56: warning: ignoring #pragma ident 
window.c: In function `process_property_notify':
window.c:4456: error: structure has no member named `atom_net_wm_geometry_save_id'
window.c: In function `update_geometry_save_id':
window.c:4675: warning: implicit declaration of function `meta_prop_get_save_id'
window.c:4676: error: structure has no member named `atom_net_wm_geometry_save_id'
gmake[2]: *** [window.o] Error 1
Comment 49 Benjamin Kahn 2004-09-13 16:18:55 UTC
Hi Peter.  You need to apply both patches.  The atom patch has been separated
out because atoms (used to?) are created frequently, and I was tired of
releasing new versions of the patch simply to update the atom.
Comment 50 Benjamin Kahn 2004-11-01 22:28:00 UTC
Created attachment 33324 [details] [review]
New version of the atom patch.

This version is designed to work against 2.9.1 CVS HEAD.
Comment 51 Benjamin Kahn 2004-11-01 22:28:47 UTC
Created attachment 33325 [details] [review]
Window History Patch

I think I was drunk when I made the last version of this patch.  Man....
Comment 52 joh 2005-02-01 18:35:14 UTC
Is there any hope that this will go into Metacity some day? I was just about to
do a mass bug report against a zillion of applications not saving their window
size and position when I thought better about it and did a search for that problem. 

Most application developers seem to think that it's the job of the WM to care
for that. Metacity doesn't care, though. So users end up dragging their apps to
the same place over and over again and this definitely sucks big time. For me
this is the one most annoying GUI shortcoming in Gnome yet. Especially since
Nautilus went spatial users get used to have their windows open exactly where
they put them last time. Having most apps popping up their windows top-left
again and again feels straight broken, as if the apps were totally ignoring the
user's actions and expectations in one of the most visible ways.

Comment 53 Elijah Newren 2005-10-02 02:49:31 UTC
Perhaps not so surprisingly the patches don't cleanly apply right now, so I'll
go ahead and mark as needs-work.
Comment 54 Björn Lindqvist 2006-03-27 19:34:23 UTC
What are the outstanding issues with the patch that prevents it from being committed? Are you still (almost four years! :/) working on the patch Benjamin? Is it just a question of getting it to compile with CVS HEAD? 
Comment 55 Rob Adams 2006-03-27 19:40:17 UTC
Benjamin mentions in Comment 43 the outstanding issues with the feature.  Unfortunately, some difficult problems to solve.
Comment 56 Benjamin Kahn 2006-03-27 20:28:04 UTC
I am not actively working on this patch anymore, due to lack of perceived interest.  My impression is that most people think metacity is good enough, and applications are handling window positioning for the most part.  My notes on what's still needed are on another hard drive -- I'll get them off tonight and post them.
Comment 57 Benjamin Kahn 2006-03-27 20:29:58 UTC
The biggest issue, BTW, is that this patch works 95% of the time correctly.  But the remaining 5% is not only tricky to solve, but very annoying to users.
Comment 58 Björn Lindqvist 2006-03-29 16:20:41 UTC
From what I've seen written in this thread I can't see any unsurmountable outstanding issues. What are the annoying 5% cases? If this patch is updated to cvs head I would very much like to try it out and give feedback. Maybe the code in this patch can also be used to solve #121934?
Comment 59 Havoc Pennington 2006-03-30 15:43:43 UTC
One 5% anyway is that window class/role settings do not properly map to "what should be saved," nor does metacity always know some of the exceptions to when/what should be saved for particular windows.

This should definitely be in apps (or if you want to avoid some pain, in a toolkit feature). Or partially in apps and partially in the WM, I had a proposal for that on wm-spec-list once.

I've always opposed the class-based saving because it just doesn't work, it's a busted heuristic approach. And it isn't really clear what to tell app authors to fix when it goes wrong.
Comment 60 Benjamin Kahn 2006-03-30 16:26:15 UTC
Okay.  These are my latest notes.  Based on discussions with others, I believe this should work and feel fairly natural:

There are two lists maintained of window information.  

The first is a hash of: Class / Role / request width / request height.  This information is checked only the first time the window is mapped.  For example, my evolution composer window would have these values:

evolution / (null) / 752 / 595

This list is used to save and restore the width and height settings for this window, unless usize is set.  When a window is resized by the user, the saved setting is updated.

The second list is a hash of: Class / Role / Initial Title / number.  Except for number, this information is also checked only the first time the window is mapped.  For example, the Evolution composer window would have these values:

evolution / (null) / Compose Message / 0

This list is used to save and restore the x and y coordinates to place the window, and also the maximization state.  When the user moves the window, or changes the maximization state, the saved setting is updated.  

Number is the number of similar windows already being tracked.  It starts at 0 and increases.  If a window is closed (ie. there are gaps) and a window is moved or maximized, that window's number is changed to the smallest available number.

If, during placement, a window is already in the location to be placed, placement isn't restored, but falls back to normal placement methods.  This setting isn't saved unless the user moves the window.
Comment 61 Elijah Newren 2006-03-30 18:16:32 UTC
Isn't using Role for this buggy?  From the ICCCM:

The client must set the WM_WINDOW_ROLE property to a string that uniquely identifies that window among all windows that have the same client leader window. The property must:

    * be of type STRING;
    * be of format 8; and
    * contain a string restricted to the XPCS characters, encoded in ISO 8859-1. 

So, if a user closes an app and then sometime later opens that same app, we shouldn't expect them to have the same WM_WINDOW_ROLE yet the user would probably expect the window in the same place.
Comment 62 Elijah Newren 2006-03-30 18:17:25 UTC
(Oops, didn't mean to copy & paste the information on type-requirements for the property since they're superfluous to the discussion; sorry)
Comment 63 Benjamin Kahn 2006-03-30 18:24:44 UTC
But the bug in this case would be a cache miss.  And cache misses are probably okay since they simply fall through to normal placement routines.  It's the false positives that I worry about most.  The information we have to work with seems to be:

Requested Height, Requested Width, Requested position (x,y), title, icon title, app icon, class, name, role, parent, window type, pid

Is Class / request width / request height and Class / Initial Title / number going to be unique enough?  I'm not sure.  
Comment 64 Elijah Newren 2006-03-30 18:41:45 UTC
Don't cache misses that should be hits cause nearly as much annoyance at the feature as false positives?  

Either way, I don't see how role helps classify at all -- does it?  Maybe this comes from another assumption of mine:  If a user does happen to have more than one instance of a particular app running on a certain workspace, and then closes them, does the user really expect future launches of that app on that workspace to appear in one of those two places?  I'd think we'd only want to cache app locations if there's no more than one of them running on a given workspace, but maybe I'm missing an important use case.  I guess that brings up another assumption of mine -- shouldn't the cache depend on the workspace in some manner too?
Comment 65 Benjamin Kahn 2006-03-30 19:11:15 UTC
It does depend on workspace, actually.  The (number) portion only looks for windows on the current workspace.  Sorry for not making that more clear.  As a side note, minimized and shaded windows are counted for the. (number)

Looking at some sample apps to get values:

gnome-terminal: gnome-terminal / gnome-terminal-2548--1295285909-1143734014 / Terminal
Firefox: Gecko / (null) / Mozilla Firefox
TaskJuggler: TaskJugglerUI / TaskJuggler / TaskJuggler
Gaim: gaim / buddy_list / Buddy List
Nautilus: nautilus / (null) / File Browser
GNU Emacs: emacs / (null) / emacs@<hostname>
Tomboy: Tomboy / (null) / <note name>
XChat: xchat / (null) / XChat: ...
Evolution: evolution / (null) / Evolution

So...  Maybe you're right.  Only gaim seems to use the ROLE a useful way, and the window title is already specific.  I'd like to see some testing to make sure.
Comment 66 Björn Lindqvist 2006-04-02 01:43:38 UTC
(In reply to comment #59)
> One 5% anyway is that window class/role settings do not properly map to "what
> should be saved," nor does metacity always know some of the exceptions to
> when/what should be saved for particular windows.

That is true. But from what I've understood from this thread, that is the only way for Metacity to save information about window instaces. Doesn't Devil's Pie also does something similar with its "window matching"? Since Devil's Pie is a pretty popular utility, are those 5% really that bad?
 
> I've always opposed the class-based saving because it just doesn't work, it's a
> busted heuristic approach. And it isn't really clear what to tell app authors
> to fix when it goes wrong.
 
Don't app authors have the option to override Metacity's placement rules? I wonder also if this bug shouldn't be closed then if you are opposed to the idea. Or is there an alternative to class-based saving of window history information?
Comment 67 Benjamin Kahn 2006-04-18 03:47:05 UTC
Created attachment 63769 [details] [review]
Patch against metacity CVS for two list window history saving

This is a very quick and dirty patch for window history saving as detailed in the last few comments.  It maintains two lists -- one for placement and maximization/sticky state, etc and the other for window size.  This patch should only be used to test this idea.  Metacity's internals changed quite a bit since I last looked at them, so this patch should be re-worked to integrate better.  

KNOWN BUGS: 
* Restoring saved window information from the data file will fail
* Saving window information to the file only happens when metacity shuts down cleanly
* Information written to the file is never purged of old entries
Comment 68 Benjamin Kahn 2006-04-25 14:08:01 UTC
This patch has been working flawlessly for me now for quite a while.  I'll update the patch soon to fix saving and restoring history to disk.  But it would be nice if someone were to test this on their own systems, or review the patch to see what needs to be fixed before it can be considered for inclusion.
Comment 69 Elijah Newren 2006-06-05 02:01:46 UTC
*** Bug 343857 has been marked as a duplicate of this bug. ***
Comment 70 Elijah Newren 2007-04-09 20:14:08 UTC
The last time I met with Matthias, we talked about session saving and both of us were of the opinion that making apps responsible was the way to go.  This was mostly based off recent comments by Havoc on d-d-l.  Particular emails that look relevant doing a search now several months later:

http://mail.gnome.org/archives/desktop-devel-list/2006-August/msg00154.html
http://mail.gnome.org/archives/desktop-devel-list/2006-August/msg00160.html
http://mail.gnome.org/archives/desktop-devel-list/2006-September/msg00088.html

(Also worth reading:
http://blogs.msdn.com/oldnewthing/archive/2005/03/14/395271.aspx
http://live.gnome.org/GnomeGoals/SaveState)

That probably has to be incredibly frustrating for you, Benjamin, after all the work you put into these patches.  But I really think that's the route we should take.
Comment 71 Benjamin Kahn 2007-04-18 18:24:04 UTC
Elijah,
   Actually, I'm okay with this because:

    * Metacity seems to be a lot better at placement now.  When I first started working on this, Metacity had terrible placement algorithms.
    * Most applications have given up and now perform placement themselves.  This makes working on my patch frustrating because it is usually not even being used, so bugs are hard to detect.
    * I've been working on other things lately and have been ignoring this bug.
Comment 72 Thomas Thurman 2007-08-01 11:54:28 UTC
Can we close this bug OBSOLETE or something, then, and obsolete the patches?
Comment 73 Elijah Newren 2007-08-02 01:41:18 UTC
Sounds good; no need to change patch status though, since patches in closed bugs aren't listed on the unreviewed list.  :)
Comment 74 Elijah Newren 2007-09-21 23:14:15 UTC
*** Bug 478721 has been marked as a duplicate of this bug. ***