In particular, we want to do this after we've done the normal refresh driver layout flushing (because mousemove flushes layout), so do it as a Flush_Display observer. This fixes the Tdhtml regression from bug 716549.

And backed out. Some of our tests apparently actually rely on this event firing "very soon", and others are randomly failing for some reason with antialiasing differences with this patch....

Oh, and the "very soon" tests are of course known randomorange, because of course sometimes it doesn't fire soon even without the change in this bug!

OK. So comment 4 was only half right. One of the issues that clearly need changes in the tests was a randomorange; the other was not. I have both of those fixed now. The remaining issue are the new random oranges on layout/reftests/bugs/478811-*.html. For some reason, this patch causes antialiasing differences on the edges of the green rectangles....

The remaining fails seem to deal with testcases that have a div inside a large button; it's possible that a difference in mouseover timing changes how many times we repaint that button or something. I'm going to try adding a big div covering the whole rendering area to those testcases, in the hope of keeping the mouse away from that button, I guess.

sounds good

or make the button pointer-events:none, that's probably easier and less fragile

Hmm. I have a green try run with the other, actually, but I'll try the pointer-events thing. I agree it's less fragile.

This is the same C++ as I landed originally; all the new stuff is in the tests

Comment on attachment 587871 [details] [diff] [review] With test fixes Review of attachment 587871 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/tests/native_mouse_mac_window.xul @@ +166,5 @@ > + } > + } else { > + timeoutFunc = callbackFunc; > + } > + gAfterLoopExecution = setTimeout(timeoutFunc, 0); We're expecting the event to fire here, right? Why do we even need a timeout at all? Why not just let the test hang if the test fails due to the event not being delivered? @@ +369,5 @@ > + function () { > + left.mozRequestAnimationFrame(function() { > + SimpleTest.executeSoon(callback); > + }); > + }); Same here... ::: widget/tests/test_bug596600.xul @@ +119,5 @@ > + SimpleTest.executeSoon(callback); > + }); > + }, winToFocus); > + } > + Why not just wait for a mouse event here?

> We're expecting the event to fire here, right? Why do we even need a timeout at all? Why > not just let the test hang if the test fails due to the event not being delivered? The test seems to go to some pains to abort early on random unexpected events. I was more or less just preserving that, but I could just skip the setTimeout part. > Same here... Not sure what you mean. Every part of the quoted code is needed, I believe. > Why not just wait for a mouse event here? We could, but it'd actually take bigger changes to the test, I think. Sometimes it needs to be a mouseover and sometimes a mouseout. I might be able to get away with watching for mousemove, but I'd still need to remove event listeners and such... I can do that if you prefer, of course.

(In reply to Boris Zbarsky (:bz) from comment #12) > The test seems to go to some pains to abort early on random unexpected > events. I was more or less just preserving that, but I could just skip the > setTimeout part. Right, it shouldn't be a problem to remove the setTimeout but still check for unexpected events and continue the test when all expected events have been received. > > Same here... > > Not sure what you mean. Every part of the quoted code is needed, I believe. Instead of doing executeSoon or RAF, why not just wait for the mouseover to fire before continuing? > > Why not just wait for a mouse event here? > > We could, but it'd actually take bigger changes to the test, I think. > Sometimes it needs to be a mouseover and sometimes a mouseout. I might be > able to get away with watching for mousemove, but I'd still need to remove > event listeners and such... I can do that if you prefer, of course. Maybe all this is too much work, but it does seem to me that the most logical way to have a reliable test here is to listen for the particular DOM events we're expecting and keep going only after they've been received. We don't need any extra constraints on their timing.

> Instead of doing executeSoon or RAF, why not just wait for the mouseover to fire before > continuing? I can do that, I guess. It'll take a bit more surgery on the test. Will do it tomorrow.

Maybe it doesn't matter too much, but with your patch we still depend quite a bit on the exact timing of the synthetic event, which we still might want to change later.

Yeah, agreed. That's why I'll fix things up. Just not tonight. ;)