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.
No more comments from my side :)
Thanks,
Artem
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
