On Wed, Nov 11, 2009 at 5:05 PM, Artem Ananiev <[email protected]> wrote: > Hi, Damjan, > > I wonder if WM teams (kwin, metacity, xfwm, etc.) are aware of the errors in > the specification/examples, otherwise they'll wait for incorrect atom names > and use incorrect windows and will miss the information what Java > applications provide to them.
The widely used GUI framework gtk+ has been sending startup notification messages this way for many years, so it must work - for example follow the function gdk_x11_display_broadcast_startup_message in http://git.gnome.org/cgit/gtk+/tree/gdk/x11/gdkdisplay-x11.c > No more comments from my side :) > > Thanks, > > Artem Thank you Damjan Jovanovic > Damjan Jovanovic wrote: >> >> On Tue, Oct 27, 2009 at 2:15 PM, Artem Ananiev <[email protected]> >> wrote: >>> >>> Hi, Anthony, Damjan, >> >> Hello Artem, Anthony >> >>> here are a couple of comments from my side: >> >> You'll find they're followed by my counterarguments :-) >> >>> 1. _NET_STARTUP_INFO atom looks redundant. Check the example from >>> http://www.freedesktop.org/wiki/Specifications/startup-notification-spec >>> for >>> details. >> >> That example has at least 2 problems, the wrong atom name being one of >> them. I've complained to freedesktop.org some time ago >> (http://lists.freedesktop.org/archives/xdg/2008-December/010106.html), >> but things are a bit nebulous that side :-). >> >> If it makes you feel any better, the equivalent patch I sent to the >> Wine project works the same way as this one >> (http://www.winehq.org/pipermail/wine-cvs/2009-January/051514.html). >> It's been working fine since it got committed on 7 January 2009. >> >>> 2. I see the client message is currently send to the root window, while >>> the >>> same example from freedesktop.org sends it to some "target window". Is >>> the >>> fix tested, say, for different virtual desktops? >> >> The spec says "In multihead setups, the messages should go to the root >> window of the X screen where the launchee application is being >> launched." >> >> I think that's what my patch does: >> + XlibWrapper.XSendEvent(XToolkit.getDisplay(), >> + XlibWrapper.RootWindow(XToolkit.getDisplay(), >> getScreenNumber()), >> >> Now as for different virtual desktops, my tests show that both patched >> Java and native applications have the startup notification follow you >> and continue, with the window opening on the new desktop, if you >> switch virtual desktops during startup. >> >>> 3. Here is a code from removeStartupNotification(): >>> >>> 1240 final int msglen = Math.min(message.length - pos, 20); >>> 1241 int i = 0; >>> 1242 for (; i < msglen; i++) { >>> 1243 XlibWrapper.unsafe.putByte(req.get_data() + i, message[pos + i]); >>> 1244 } >>> 1245 for (; i < 20; i++) { >>> 1246 XlibWrapper.unsafe.putByte(req.get_data() + i, (byte)0); >>> 1247 } >>> >>> We first set "msglen" bytes from message array and then 20 zero bytes. It >>> seems that 20 should be replaced with (20-message.length % 20) % 20, >>> otherwise we'll get out of XClientMessageEvent bounds. >> >> No, we don't reset the variable i to 0, it starts where it left off >> and run up to 20. There can't be an overrun. >> >>> Thanks, >>> >>> Artem >> >> Thank you >> Damjan >> >>> Anthony Petrov wrote: >>>> >>>> Hello, >>>> >>>> Please review the latest version of the fix contributed by Damjan >>>> Jovanovic: >>>> >>>> RFE: https://bugs.openjdk.java.net/show_bug.cgi?id=100094 >>>> There you can also find some latest comments regarding the fix. >>>> >>>> webrev: >>>> http://cr.openjdk.java.net/~anthony/7-24-startupNotify-6863566.2/ >>>> >>>> The patch no longer unsets the environment variable, and hence does not >>>> need core-libs review. >>>> >>>> -- >>>> best regards, >>>> Anthony >
