Re: window state stuff

[Owen Taylor <otaylor redhat com>]

Tim Janik <timj gtk org> writes:

> On 23 Feb 2001, Havoc Pennington wrote:
> 
> > Hi,
> 
> ok, in principle the patch looks very good to me.
> however, i managed to still catch a couple of things ;)
> 
> 
> > struct _GdkEventWindowState
> > {
> >   GdkEventType type;
> >   GdkWindow *window;
> >   gint8 send_event;
> >   GdkWindowState changed;
> 
> is this what the state should be changed to, or has the state
> already changed to
> 
> >   GdkWindowState current;
> 
> and "changed" is just the old state.
> if the former, that field should probably say new_state, if the latter
> it should say old_state (or previous_state as the naming is in
> GtkWidget::state_set).

I suggested to Havoc earlier that it should either be:

 GdkWindowState changed_mask;
 GdkWindowState new_window_state;

or:

 GdkWindowState old_window_state;
 GdkWindowState new_window_state;

> make this function public, progs like the tasklist prolly want to use it.
> also it should be named _gdk_wmspec_supports() since you actively use it
> for repetitive checks throughout the code.

I'm not sure what the principle is here that would suggest that...

[ wmspec_supported is pretty clearly an abbreviation for 
  wmspec_is_supported() rather than wmspec_was_supported()... ]

> > +  gdk_error_trap_push ();
> > +
> > +  /* Find out if this WM goes away, so we can reset everything. */
> > +  XSelectInput (gdk_display, xwindow,
> > +                StructureNotifyMask);
> > +
> > +  if (gdk_error_trap_pop ())
> > +    return FALSE;
> > +
> > +  gdk_error_trap_push ();
> > +  
> > +  XGetWindowProperty (gdk_display, xwindow,
> > +		      wmspec_check_atom, 0, G_MAXLONG,
> > +		      False, XA_ATOM, &type, &format, &n_atoms,
> > +		      &bytes_after, (guchar **)&atoms);

This needs to check what you got and if you got anything!

> > +  if (gdk_error_trap_pop ())
> > +    return FALSE;
> > +  
> > +  wmspec_check_window = xwindow;
> > +
> > +  /* since wmspec_check_window != None this isn't infinite. ;-) */
> > +  return _gdk_wmspec_supported (property);
> > +}
> 
> you need to gdk_sync() before popping error traps, otherwise they don't
> help a single bit.

Only if the calls aren't round trips - necessary for XSelectInput() - but 
not necesary for XGetWindowProperty. 

[ Actually I don't see much reason to have two separate error traps
here - one would do as well. ]

> >    if (window->frame)
> >      gdk_window_show (window->frame);
> >  }
> > @@ -1398,7 +1420,8 @@
> >    gtk_style_set_background (widget->style, widget->window, GTK_STATE_NORMAL);
> >    if (window->frame)
> >      gtk_style_set_background (widget->style, window->frame, GTK_STATE_NORMAL);
> > -  
> > +
> > +  /* This is a bad hack to set the window background. */
> >    gtk_window_paint (widget, NULL);
> 
> could you elaborate on _why_ this is a bad hack?

Why calling paint out of realize before showing the window is
a bad hack? Seems rather obvious to me. The fact that a
paint() method is allowed to set the window background and
on the drawable when width/height -1 is definitely a hack.

Or elaborate on why why it's necessary at all?

> > Index: gtk/gtkwindow.h
> > ===================================================================
> > RCS file: /cvs/gnome/gtk+/gtk/gtkwindow.h,v
> > retrieving revision 1.25
> > diff -u -u -r1.25 gtkwindow.h
> > --- gtk/gtkwindow.h	2001/01/08 17:04:17	1.25
> > +++ gtk/gtkwindow.h	2001/02/24 00:43:43
> > @@ -85,6 +85,11 @@
> >    
> >    guint has_frame : 1;
> >  
> > +  /* gtk_window_iconify() called before realization */
> > +  guint iconify_initially : 1;
> > +  guint stick_initially : 1;
> > +  guint maximize_initially : 1;
> > +  
> 
> i think having present_initially : 1; wouldn't be a too bad idea.

What would that mean? The meaning of "present" is basically
that of "act like this window is newly popped up"

                                        Owen