Hi James, thanks for taking this up. I'd like to have nice docked system tray icons, too. Some small nits on your patches:
+ 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.
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.
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?) --Juan
__________________________________________________ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com
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 workproperly.
* Furthermore, an XSync is shown in an example of how to sendproper
* 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
James Liggett jrliggett@cox.net writes:
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.
XSync is very bad on remote links, and it shouldn't be necessary. If it behaves better with it there's a problem somewhere.
-
Alexandre Julliard -
James Liggett -
Juan Lang