Created attachment 109779 [details] [review]
Proposed patch

I do not like it much, it doesn't remember the position of the main window, thus it starts always in left top-corner, not nice, really. I know about other bug with calendar (I do not have bug number handy, I'm sorry), where reporter complains about strange positioning of the memo & task part of the calendar window, when the main window is maximized, and the memo & task part is completely hidden (or in some strange position), then it doesn't remember the position. I can see this happen with this patch applied. Seems like the editing dialog for appointment always start at 0,0. I didn't tested it more, just these things (for now) and I also didn't read the whole patch.

The patch doesn't claim to remember window positions, only the window dimensions and whether it's maximized.  Remembering window positions will require adding two new GConf keys per window (for x and y coordinates).

I'm fine with remembering the main window position, but I don't think we should be doing that with child windows.  Child windows should appear centered over the parent window.  Unfortunately this is hard to do at the moment thanks to Bonobo. Once we ditch BonoboUI we can position child windows properly.

Also, I'm not claiming to fix whatever pane positioning issues we already have.  The patch only simplifies the existing logic.  It sound like the memo/task pane position issue should be dealt with in a separate bug.  (The problem is likely because we remember pane positions as absolute values rather than percentages.)

Yes, that's a different bug, I know.

I see now it starts in 0,0 even without a patch, so no problem with that, I'm sorry for confusion.

in chunk
@@ -487,9 +455,13 @@ eab_view_new (void)
you have "MAX (144, position)". That's not the same as the old code.

I'm sorry, but I'm not able to review this completely today :(

(In reply to comment #4)
> in chunk
> @@ -487,9 +455,13 @@ eab_view_new (void)
> you have "MAX (144, position)". That's not the same as the old code.

It's ensuring the top contacts pane is at least 144 pixels tall.  But now that I look at the original code again, you're right.  The original code sets the pane position to 144 only if the stored position is zero.  Must've read it wrong the first time.  I'll fix it, but won't post a revised patch unless there's other issues.

 
> I'm sorry, but I'm not able to review this completely today :(

No problem.  Not at all urgent.  Enjoy your vacation!

I'm sorry to say, but there is really something wrong with the patch/gconf_bridge code. When I have this patch applied (and beware, it failed in one hunk), then I can see the problem in contacts:
a) go to contacts, turn on preview pane
b) set View->Current View->Address Cards
c) position the size of the cards and preview to the middle (approx)
d) close and reopen evo - the set size of preview pane is not remembered.

With memos it works fine. With tasks it seems the position is remembered, but when closing evolution in tasks, then I got couple runtime warnings on console.
GLib-GObject-WARNING **: instance of invalid non-instantiatable type `<invalid>'
GLib-GObject-CRITICAL **: g_signal_handlers_disconnect_matched: assertion `G_TYPE_CHECK_INSTANCE (instance)' failed
and one more time both of them.

Okay, I'll go another round on this patch.  Thanks for the review.

Hi Matthew; 

You requested that I add a comment as a reminder.  I have pasted some of an exchange on the evolution users mail list.

"In gnome, an application remembers its
previous position as long as it is the first application on the screen.
Subsequently opened applications always position themselves in the upper
left corner.  

I have set aside workspace 3 for email & chat.  That means I always have
two applications open in that workspace; Evolution and Xchat.  Evolution
is sized to cover 80% of the width of the screen and about 75% of the
height of the screen from the top down.  Xchat, which allows me to
configure its position, fills the bottom 25% and is equal in width to
Evolution.  For the way I like to work, the above arrangement serves me
well; nice, neat, easy to read."

"Some applications let the user set the x and y position in gconf.  Other applications have a Xwindows --geometry +x+y command line switch."


And from comment #3
"I'm fine with remembering the main window position, but I don't think we should
be doing that with child windows."

Thanks Bill.  I'll be sure to add persistent main window coordinates in the next revision of the patch.

I guess, when moving from bonobo to gtk, then there will be way to tell the position/size in a command line with the --geometry parameter as Bill mentioned can be done for other applications. Am I right?

It's not automatic; we'd have to implement that manually.  But I don't think there's anything stopping us from doing it today if we wanted to.  Basically you just add a --geometry entry to the GOption list and pass the value to

    gtk_window_parse_geometry (main_window, geometry_string)

I'd still prefer to track the window geometry in GConf so it's persistent by default, without having to give some obscure command line argument.  If we can manage to support both methods without them stepping on each other then all the better, I guess.  But that's a bit out of scope for this bug.

Hi;

I'll try not to keep intervening. You guys are the experts.

An example of command line --g +x+y is in Emacs.  There are others, I just don't remember them offhand.  Running $ programX --help-all usually reveals a --geometry switch.

The problem with the command line switch is that it has to be setup for every different command line launch i.e. terminal command, launcher, session startup and whatever else.

If it can be set as a default in the gconfd, it only has to be set once.  Besides, even if it is a bit fiddly it would be consistent with other Gnome programs - so once learnt it would be easy to remember.

Regards Bill 

(In reply to comment #12)
> An example of command line --g +x+y is in Emacs.  There are others, I just
> don't remember them offhand.  Running $ programX --help-all usually reveals a
> --geometry switch.

I noticed such a command-line argument in Nautilus and Gnumeric as well, and found they both implement it manually as described in comment #11.  So it is indeed a manual effort from application to application, which explains the inconsistency across GNOME.


> The problem with the command line switch is that it has to be setup for every
> different command line launch i.e. terminal command, launcher, session startup
> and whatever else.

Right, which is why I prefer the GConf approach.  Although that too has to be implemented manually from application to application.

I'm all in favor of consistency across applications, but I'm having trouble coming up with a compelling enough use case for adding such an option when we already have (or /will/ have) GConf-based window geometry tracking.

From Comment 13

"I'm all in favor of consistency across applications, but I'm having trouble
coming up with a compelling enough use case for adding such an option when we
already have (or /will/ have) GConf-based window geometry tracking."

The position of a widow on closing isn't always necessarily the position one wants on launching.  By being able to set a default which overrides the position on closing, a user (like in my case of Evolution and Xchat) can be sure that a setup remains consistent.  I acknowledge that the default would only be needed occasionally, but on the other hand, a user could be sure a window is going to do what he or she wants. 

It seems to me that it is only two extra keys in the gconfd with the 'default' and 'mandatory' mode capability.  For example, xchat-gnome has under main_window in gconf-editor an x key and y key that records the current position.  That position can be made the default position by root by right clicking on each key and setting 'default' or 'mandatory'. The position can also be set by typing in the pixel calculation for the desired positioning.  It is probably a consistency that should be standard in Gnome. 

I remember the debate two or three years ago over what I believe was called 'smart windows' which kept growing more and more complex.  Finally Gnome decided to rip the whole thing out and go with the simplified window creation they now use.  I agreed at the time.  But no system for the user to control window placement was re-introduced.  That it would be on a application by application basis is not a problem to my mind, particularly if combined with some form of placement tracking.  A pre-set window placement is only needed in just a few situations. I prefer to decide such things myself, being able to occasionally override what programmers think I should want.

Regards Bill

Okay, I'm convinced of its usefulness.  Btw, I started a discussion about this on the desktop-devel-list mailing list, to see what other GNOME developers think.

Though it's somewhat tangential to this particular bug, I'll see if I can work a --geometry option into the patch.  If it gets to be too much of a hassle I'll punt for now and open a separate bug for the feature.

Bill, you'll be happy to know that Evolution 2.29.3 will include a --geometry option that works as you'd expect.  To avoid complications, the window's size and position is -not- tracked in GConf when the option is used.

This is all pretty much implemented now in 2.29.  Closing as FIXED.