Hi, Anthony, Damjan,
here are a couple of comments from my side:
1. _NET_STARTUP_INFO atom looks redundant. Check the example from
http://www.freedesktop.org/wiki/Specifications/startup-notification-spec
for details.
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?
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.
Thanks,
Artem
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
