Re: Support for WM spec

[Owen Taylor <otaylor redhat com>]

Alexander Larsson <alla lysator liu se> writes:

> Here is a patch that adds support for some stuff in the WM spec. Compared 
> to my previous proposal I removed everything that isn't generally usefull
> for application authors or Gtk+ internals.

OK, this looks pretty good. Quite a few comments below, but
a lot of them are just edits to the docs. If you don't have
questions, should be ready to commit with these changes.

> This version uses
> 
> * _NET_WM_WINDOW_TYPE
>  Only NORMAL, DIALOG, MENU and TOOLBAR are exported.
> * _NET_WM_STATE_MODAL
>  The other states (the ones havoc isn't working on) are not generally
>  usefull for apps i think.
> * _NET_WM_ICON_NAME
> * _NET_WM_ICON  
>  Using the same packing as the trolltech sample implementation.
> 
> I didn't combine the MODAL state with the window type hint, because
> separating it fit better with the Gtk changes. All classes deriving from
> GtkDialog get a DIALOG type, and all modal GtkWindows gets the MODAL
> state.

The implementation (though not the API, I think) of MODAL should
probably be integrated in some way with what Havoc is working on.
 

> +/**
> + * gdk_window_set_type_hint:
> + * @window: A #GdkWindow
> + * @hint: A hint of the function this window will have
> + *
> + * The application can use this call to hint to the window
                                           ^^^^
provide a hint
                           
> + * manager about the functionallity of a window. The window manager
                                  ^

> + * can use this information to decide the decoration and behaviour
                               ^^^^^^^^^
                               when determining

> + * of the window.


> + * gdk_window_set_modal_hint:
> + * @window: A #GdkWindow
> + * @modal: TRUE if the window is modal, FALSE otherwise.
> + *
> + * The application can use this hint to tell the window manager
> + * that a certain window has modal behaviour. The window manager
> + * can use this information to handle modal windows in a special
> + * way.

It would be nice to have a ", such as..." here to clarify the
behavior. but I'm not sure what that would be. I don't really
understand the purpose of this hint in the wmspec.

> + * The #gdk_window_set_transient_for() function must be used to
> + * tell the window manager which window the window is modal to.

The window manager spec language is better: "modal for, not modal
to".

But, I don't particularly like what the window manager seems to
specify here, which is that the modal dialog can be modal for one
window or for the window group, which seems odd.

Either it should always apply to the entire group, or there should be
a concept of modality groups separate from groups

Anyways, not your responsibility... Perhaps, though, the doc comment
should say "You should only use this on windows for which you have
previously called #gdk_window_set_transient_for()" And leave the
purpose of the transient_for_call() up in the air.

> + **/
> +void
> +gdk_window_set_modal_hint (GdkWindow *window,
> +			   gboolean   modal)
> +{
> +  XEvent xev;
> +  
> +  g_return_if_fail (window != NULL);
> +  g_return_if_fail (GDK_IS_WINDOW (window));
> +  
> +  if (GDK_WINDOW_DESTROYED (window))
> +    return;
> +  
> +  xev.xclient.type = ClientMessage;
> +  xev.xclient.serial = 0;
> +  xev.xclient.send_event = True;
> +  xev.xclient.window = gdk_root_window;
> +  xev.xclient.display = gdk_display;
> +  xev.xclient.window = gdk_root_window;
> +  xev.xclient.message_type = gdk_atom_intern ("_NET_WM_STATE", FALSE);
> +  xev.xclient.format = 32;
> +  xev.xclient.data.l[0] = (modal) ? gdk_atom_intern ("_NET_WM_STATE_ADD", FALSE) : gdk_atom_intern ("_NET_WM_STATE_REMOVE", FALSE);
> +  xev.xclient.data.l[1] = gdk_atom_intern ("_NET_WM_STATE_MODAL", FALSE);
> +  xev.xclient.data.l[2] = 0;
> +  
> +  XSendEvent (gdk_display, gdk_root_window, False,
> +	      SubstructureRedirectMask | SubstructureNotifyMask,
> +	      &xev);
> +}

This is, I believe, not correct when the window is in the withdrawn
state. But that falls basically into my comment above about needing
to share implementaiton with Havoc's stuff.

> +
> +/**
> + * gdk_window_set_icon_list:
> + * @window: The #GdkWindow toplevel window to set the icon of.
> + * @pixbufs: A list of pixbufs, of different sizes.
> + * @Returns: TRUE if the icons where set, false otherwise
                                   ^

> + * Sets a list of icons for the window. These are used to represent
                                           One of these will be used...

> + * the window when it has been iconized. It is usually shown in
                                  iconified. The icon...

> + * an icon box or some sort of task bar. Which icon size is shown

  depends on the window manager. The window manager can scale the icon
  but ...

> + * setting several size icons can give better image quality since
> + * the icons might not need to be scaled. Ownership of the pixbufs
    
      the window manager may only need to scale the icon by a    
      small amount or not at all.

> + * and the list is transfered to this function.

[ This is wrong. See below. ]

> + * On the X11 backend this call might fail if the window manager
> + * doesn't support the Extended Window Manager Hints. Then this
> + * function returns FALSE, and the application should fall back
> + * to #gdk_window_set_icon().

This is rather peculiar compared to most of GTK+ and I almost think
that the backend should take care of emulation in the case it 
failure. 

But perhaps you are right that leaving this to GtkWindow is better
if there is significant variation on how the fallback should
be handled.

(We perhaps should wrap the ICCCM ICON_SIZES property, though,
if we want GtkWindow to be able to do an accurate fallback.)

> + **/
> +gboolean
> +gdk_window_set_icon_list (GdkWindow *window,
> +			  GList     *pixbufs)

I think this function should copy the list and ref the pixbufs if it
needs to do so. Transfer of ownership should never be done except when
absolutely necesary.
>  
> Index: gtk/gtkmenu.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gtk/gtkmenu.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 gtkmenu.c
> --- gtk/gtkmenu.c	2001/02/17 06:04:40	1.54
> +++ gtk/gtkmenu.c	2001/02/23 15:02:02
> @@ -246,6 +246,8 @@ gtk_menu_init (GtkMenu *menu)
>  				   NULL);
>    gtk_window_set_policy (GTK_WINDOW (menu->toplevel),
>  			 FALSE, FALSE, TRUE);
> +  gtk_window_set_type_hint (GTK_WINDOW (menu->toplevel),
> +			    GDK_WINDOW_TYPE_HINT_MENU);

No point in this menu->toplevel is override-redirect and will
never be seen by the window manager.

>  /**
> + * gtk_window_set_type_hint:
> + * @window: a #GtkWindow
> + * @hint: the window type
> + *
> + * By hinting about the intended function of the window you allow
> + * window managers to decorate and handle different kind of windows

      By setting the type hint for the window, you allow the window
      manager to decorate and handle the window in a way which is
      suitable to the function of the window in your application.

> + * in a consistant way. gtk_dialog_new_with_buttons() and other
> + * convenience functions in GTK+ will sometimes call
> + * gtk_window_set_type_hint() on your behalf.

Doesn't there need to be a caution here that this should not be called
while the window is visible?

> + **/
> +void
> +gtk_window_set_type_hint (GtkWindow           *window, 
> +			  GdkWindowTypeHint    hint)
> +{
> +  g_return_if_fail (GTK_IS_WINDOW (window));

And add

     g_return_if_fail (!GTK_IS_VISIBLE (window))l

> @@ -89,6 +89,8 @@ struct _GtkWindow
>    guint frame_top;
>    guint frame_right;
>    guint frame_bottom;
> +
> +  GdkWindowTypeHint type_hint;
>  };
>  

You might want to use a bitfield here to save some structure
space.

Regards,
                                        Owen