[UPDATE] RFC: XEmbed System Tray Patches

[James Liggett]

On Fri, 2006-08-11 at 09:02 -0700, Juan Lang wrote:
> +        wine_tsx11_lock();
> +        /* set XEMBED protocol data on the window */
> +        info[0] = 0; /* protocol version */
> +        info[1] = 1; /* mapped = true */
> +        XChangeProperty( display, data->whole_window,
> +                         x11drv_atom(_XEMBED_INFO),
> +                         x11drv_atom(_XEMBED_INFO), 32, PropModeReplace,
> +                         (unsigned char*)info, 2);
> +    
> +        /* send the docking request message */
> +        ZeroMemory( &ev, sizeof(ev) ); 
> +        ev.xclient.type = ClientMessage;
> +        ev.xclient.window = systray_window;
> +        ev.xclient.message_type = x11drv_atom(_NET_SYSTEM_TRAY_OPCODE);
> +        ev.xclient.format = 32;
> +        ev.xclient.data.l[0] = CurrentTime;
> +        ev.xclient.data.l[1] = SYSTEM_TRAY_REQUEST_DOCK;
> +        ev.xclient.data.l[2] = data->whole_window;
> +        ev.xclient.data.l[3] = 0;
> +        ev.xclient.data.l[4] = 0;
> +        XSendEvent( display, systray_window, False, NoEventMask, &ev );
> +        wine_tsx11_unlock();
> 
> You do more with the lock held than you need to.  Initializing info and ev
> could both be down without holding the lock.
Good point.
> 
> diff --git a/include/wine/server_protocol.h
> b/include/wine/server_protocol.h
> index b8c8a7c..3b4bd08 100644
> --- a/include/wine/server_protocol.h
> +++ b/include/wine/server_protocol.h
> @@ -2741,6 +2741,7 @@ struct set_window_pos_reply
>  {
>      struct reply_header __header;
>      unsigned int   new_style;
> +    unsigned int   ex_style;
>  };
> 
> This diff should be against server/protocol.def, not against
> server_protocol.h.
I didn't know about protocol.def...thanks for pointing this out.
> 
> diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c
> index b26e14f..69c32ef 100644
> --- a/dlls/winex11.drv/window.c
> +++ b/dlls/winex11.drv/window.c
> @@ -400,6 +400,13 @@ static void systray_dock_window( Display
>          ev.xclient.data.l[3] = 0;
>          ev.xclient.data.l[4] = 0;
>          XSendEvent( display, systray_window, False, NoEventMask, &ev );
> +        
> +        /* This XSync call is needed to properly send the message to the 
> +         * systray window and is required to have icon docking work
> properly.
> +         * Furthermore, an XSync is shown in an example of how to send
> proper
> +         * docking messages, see the XEmbed system tray spec for more 
> +         * information. */
> +        XSync(display, False);
> 
> Why not include this in the original patch (0005 in the series?)
That was the original plan, but I had some concerns about whether or not
Alexandre would take this or not. When Rob Shearman tried to get these
in last year, Alexandre asked him why it was necessary. At the time Rob
said that without that call, the icons that docked were only 1 pixel
wide. Now that isn't the case--without the call the icon width is
perfectly fine, but docking isn't as reliable. Besides, doing an XSync
is part of the canonical recipe for sending X events. Regardless, you've
got a valid point--it does seem a little odd that this isn't part of
patch #5. I think I'll try to work this in for the next update of these
patches. 

Thanks for your feedback.

James