<AWT Dev> Review request #2: 6863566 (Java should support the freedesktop.org startup notification specification)

[Artem Ananiev]

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