Fix periodic focus bug

Fix periodic focus bug

Pedro Gimeno explains:

"As you most likely already know, every X Window event comes with a
timestamp. You can see it using e.g. xev.

I don't know if this is Debian-specific and I'm no X Window expert, but
the fact is that in my machine this timestamp, according to my
/usr/include/X11/X.h, is a 32-bit unsigned integer and it holds the
event time in milliseconds according to my tests.

This timestamp appears to be in milliseconds from the start of the Unix
epoch, modulo 2^32. The problem is that 32 bits are too little for this
purpose: 2^32 milliseconds are exactly 49 days, 17 hours, 2 minutes and
47.296 seconds.

Now, the WindowMaker code rejects any focus event whose timestamp is
less than the last one seen, using code similar to this:

 if (timestamp < LastTimeStamp) return;
 LastTimeStamp = timestamp;

This is a disaster when timestamp wraps around because of the 32-bit
limit. Say LastTimeStamp equals 2^32-1 and a focus event comes two
milliseconds after. Its timestamp will equal 1 because of the
wraparound. Obviously, 1 < 2^32-1 so the event will be rejected even if
it comes from the future (relative to LastTimeStamp) and not from the
past. Focus events are no longer accepted because nothing can be greater
than 2^32-1 (unless you click on that exact millisecond, 49 days after).

If you look in http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=102314
you'll see two people confirming the periodicity and predictability of
the bug. Arnaud Giersch was who noted that the period was 2^32
milliseconds (and not 2^31 as I thought it could be because of a signed
int involved) and who noted the coincidence with the Unix epoch.

My fix works by subtracting one timestamp from the other (an operation
which is done modulo 2^32) and rejecting the focus event only if the new
timestamp is within 1 minute (60000 milliseconds) in the past. Given the
periodicity of the timestamp, actually after 49.7 days there will be
another minute in the future during which focus events will not be
accepted; again after 99.4 days, etc., but thanks to the subtraction,
they will now be relative to the *last* focus change and not to the Unix
epoch.

The patch could be simplified to get rid of the compareTimes function.
It's written like that because it comes from the previous patch (which
you can see in the same bug report).

So the date comparison line could end up looking like this:

   if (scr->flags.ignore_focus_events || LastFocusChange - timestamp <
60000)

so you can get rid of the compareTimes function. However please respect
the other change of 'int' to 'Time'.

As for why you aren't experiencing this problem, perhaps the event
timestamp does not come from the Unix epoch in your X Window system, but
rather starts counting when you start it or your computer, I don't know.
Or maybe your X Window server's timestamps are 64 bits wide instead of
32 bits, which I don't think because I doubt the X11 protocol specifies
a 64-bit record for the transmission of the timestamp. In either case,
the output of xev will probably help understanding the cause.

Or maybe even it's a click-to-focus only bug and you're not using that
mode. I haven't checked who calls wSetFocusTo.

Note that I can reproduce it just by manipulating the system clock,
which seems to be tied to the event timestamps."

For more information see:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=102314

Reported-by: Joey Hess <joeyh@debian.org>
Signed-off-by: Pedro Gimeno <parigalo@formauri.es>