This is a follow-up from bug 1289435, to enable tests on Windows that exercise the APZ touch events code paths. In particular this includes test_group_touchevents in the APZ mochitest folder and the touch_action_tests in the DOM pointerevents test folder. Right now just enabling these on Windows results in try failures because somehow a mouse button event gets put into the event stream, and that creates a mouse-drag block which interrupts the touch events. Ideally we should figure out why that's happening but if we fix bug 1289432 then it may become a moot point.

I should see if this works now with the new ImputQueue code.

Seems like there's still some issues.

Gonna pick this up again and dig into it.

I think the new InputQueue code did in fact fix the issues that were happening before. The new issues are just that the Windows long-tap event sequence is different from other platforms (contextmenu is fired after touchend, rather than before) and a couple of tests weren't expecting that. I have patches that appear to fix this, just waiting on some retriggers before I clean them up and push them for review. https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f5425c36eaeaec8aa9ad9ae75d3ff63ff3068ee&group_state=expanded

Comment on attachment 8798853 [details] Bug 1291381 - Fix helper_long_tap.html and helper_tap_passive.html to work with the Windows long-press event sequence. https://reviewboard.mozilla.org/r/84232/#review82880 Not a comment on the test per se, but isn't this difference between platform behaviours a potential headache for web developers who want to listen to these events? ::: gfx/layers/apz/test/mochitest/helper_long_tap.html:60 (Diff revision 1) > - if (eventsFired == 3) { > + if (eventsFired == 3) { > - synthesizeNativeTouch(document.getElementById('b'), 5, 5, SpecialPowers.DOMWindowUtils.TOUCH_REMOVE, function() { > + synthesizeNativeTouch(document.getElementById('b'), 5, 5, SpecialPowers.DOMWindowUtils.TOUCH_REMOVE, function() { > - dump("Finished synthesizing touch-end, doing an APZ flush to see if any more unexpected events come through...\n"); > + dump("Finished synthesizing touch-end, doing an APZ flush to see if any more unexpected events come through...\n"); > - flushApzRepaints(function() { > + flushApzRepaints(function() { > - dump("Done APZ flush, ending test...\n"); > + dump("Done APZ flush, ending test...\n"); > - subtestDone(); // closing the window should dismiss the context menu dialog > + subtestDone(); I liked this comment

Comment on attachment 8798854 [details] Bug 1291381 - Enable touch tests on Windows. https://reviewboard.mozilla.org/r/84234/#review82882 ::: dom/events/test/pointerevents/mochitest.ini (Diff revision 1) > [test_pointerevent_suppress_compat_events_on_drag_mouse.html] > support-files = pointerevent_suppress_compat_events_on_drag_mouse.html > disabled = should be investigated > [test_touch_action.html] > - # Windows touch injection doesn't work in automation, but this test can be run locally on a windows touch device. > - skip-if = (toolkit == 'windows') I like the indentation here (it makes it more clear that skip-if and such pertain to the test above, not the test below). Can we adopt it in gfx/layers/apz/test?

(In reply to Botond Ballo [:botond] from comment #8) > Comment on attachment 8798853 [details] > Bug 1291381 - Fix helper_long_tap.html and helper_tap_passive.html to work > with the Windows long-press event sequence. > > https://reviewboard.mozilla.org/r/84232/#review82880 > > Not a comment on the test per se, but isn't this difference between platform > behaviours a potential headache for web developers who want to listen to > these events? > The mouselongtap event isn't exposed to web content at all. The web-observable difference is with the contextmenu event, but web developers shouldn't be using the contextmenu event to detect a "long-press" gesture anyway. The event can fire for other reasons, such as mouse right-clicking and keyboard shift-F10 (or whatever the platform-specific keyboard combo is). So as long as they are using the contextmenu for its intended purpose (detecting when the context menu is about to be displayed, and/or preventing it) they should be fine. > ::: gfx/layers/apz/test/mochitest/helper_long_tap.html:60 > > + dump("Done APZ flush, ending test...\n"); > > - subtestDone(); // closing the window should dismiss the context menu dialog > > + subtestDone(); > > I liked this comment Sadly the comment was wrong. The test does a preventDefault() on the contextmenu event, so the context menu dialog never actually appears. While modifying this test I inadvertently forgot to do the preventDefault() in the windows branch, and a later test hung because (a) closing the window doesn't actually dismiss the context menu dialog and (b) the test needed the firefox window to have focus, which it didn't. So if we ever actually spawn context menu dialogs in a test bad things will happen since I don't think we have any good way of dismissing them.

(In reply to Botond Ballo [:botond] from comment #9) > I like the indentation here (it makes it more clear that skip-if and such > pertain to the test above, not the test below). Can we adopt it in > gfx/layers/apz/test? SGTM. I'll add a patch to the series.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #10) > > ::: gfx/layers/apz/test/mochitest/helper_long_tap.html:60 > > > + dump("Done APZ flush, ending test...\n"); > > > - subtestDone(); // closing the window should dismiss the context menu dialog > > > + subtestDone(); > > > > I liked this comment > > Sadly the comment was wrong. The test does a preventDefault() on the > contextmenu event, so the context menu dialog never actually appears. Ah, I overlooked that! Never mind then :)

Comment on attachment 8798982 [details] Bug 1291381 - Add some indenting to mochitest.ini for readaibility. https://reviewboard.mozilla.org/r/84294/#review82896 Thanks!

Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0412d58b754e Fix helper_long_tap.html and helper_tap_passive.html to work with the Windows long-press event sequence. r=botond https://hg.mozilla.org/integration/mozilla-inbound/rev/f88a205fc4ff Enable touch tests on Windows. r=botond https://hg.mozilla.org/integration/mozilla-inbound/rev/ecc7a268020f Add some indenting to mochitest.ini for readaibility. r=botond

https://hg.mozilla.org/mozilla-central/rev/0412d58b754e https://hg.mozilla.org/mozilla-central/rev/f88a205fc4ff https://hg.mozilla.org/mozilla-central/rev/ecc7a268020f