As per bug 1452536 comment 10 to 12, we should gesture activate before keydown/mousedown processing, rather than on key/mouse up. It makes sense to gesture activate before running any key/mouse handlers, so that any handlers for these events are able to play video in the handlers themselves. Also, Chrome is activating before key/mouse down, so if we diverge, we'll likely get compat issues.

Comment on attachment 8970073 [details] Bug 1456037 - Gesture activate documents on key/mouse down not up. https://reviewboard.mozilla.org/r/238834/#review244518 ::: dom/events/EventStateManager.cpp:629 (Diff revision 1) > case eMouseDown: { > switch (mouseEvent->button) { > case WidgetMouseEvent::eLeftButton: > BeginTrackingDragGesture(aPresContext, mouseEvent, aTargetFrame); > mLClickCount = mouseEvent->mClickCount; > SetClickCount(mouseEvent, aStatus); > sNormalLMouseEventInProcess = true; > break; > case WidgetMouseEvent::eMiddleButton: > mMClickCount = mouseEvent->mClickCount; > SetClickCount(mouseEvent, aStatus); > break; > case WidgetMouseEvent::eRightButton: > mRClickCount = mouseEvent->mClickCount; > SetClickCount(mouseEvent, aStatus); > break; > } > + NotifyTargetUserActivation(aEvent, aTargetContent); > break; How about ePointerDown? If pointer event is enabled, ePointerDown is a preceding event of eMouseDown. https://www.w3.org/TR/pointerevents2/#the-pointerdown-event https://www.w3.org/TR/pointerevents2/#tracking-the-effective-position-of-the-legacy-mouse-pointer ::: dom/events/EventStateManager.cpp:893 (Diff revision 1) > case eTouchEnd: > NotifyTargetUserActivation(aEvent, aTargetContent); Don't we need to change eTouchEnd to eTouchStart?

Comment on attachment 8970074 [details] Bug 1456037 - Test that documents gesture activate on key/mouse down instead of up. https://reviewboard.mozilla.org/r/238836/#review244520 ::: dom/media/test/file_autoplay_policy_eventdown_activation.html:35 (Diff revision 1) > + document.body.appendChild(element); > + > + await once(element, "loadedmetadata"); > + > + let played = await element.play().then(() => true, () => false); > + is(played, false, "Document should start out not activated, with playback blocked."); nit: ok(!played, "..."); ? ::: dom/media/test/file_autoplay_policy_eventdown_activation.html:42 (Diff revision 1) > + let x = eventNames.map( > + (eventName) => { > + return new Promise(async function (resolve, reject) { > + window.addEventListener(eventName, async function (event) { > + let played = await element.play().then(() => true, () => false); > + is(played, true, "Expect to be activated already in " + eventName); nit: ok(played, "..."); ?

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #3) > Comment on attachment 8970073 [details] > ::: dom/events/EventStateManager.cpp:893 > (Diff revision 1) > > case eTouchEnd: > > NotifyTargetUserActivation(aEvent, aTargetContent); > > Don't we need to change eTouchEnd to eTouchStart? I need to spend some time investigating how to handle touch properly. I don't think just activating on touchstart or touchend is a good idea, as then we'll activate on scroll, which is not what we want. So I'd prefer to handle touch in a follow up, after I've had a chance to figure out the best strategy for touch.

(In reply to Chris Pearce (:cpearce) from comment #5) > (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #3) > > Comment on attachment 8970073 [details] > > ::: dom/events/EventStateManager.cpp:893 > > (Diff revision 1) > > > case eTouchEnd: > > > NotifyTargetUserActivation(aEvent, aTargetContent); > > > > Don't we need to change eTouchEnd to eTouchStart? > > I need to spend some time investigating how to handle touch properly. I > don't think just activating on touchstart or touchend is a good idea, as > then we'll activate on scroll, which is not what we want. So I'd prefer to > handle touch in a follow up, after I've had a chance to figure out the best > strategy for touch. On Android, Chrome is gesture activating upon pointerup, if there have been no pointercancel or touchmove events between the pointer down and the pointer up. I think that makes sense, as we want to gesture activate on a "click", not on a touch scroll. I will make that change for Android in a follow up bug.

I filed bug 1456322 for the Android work.

Comment on attachment 8970073 [details] Bug 1456037 - Gesture activate documents on key/mouse down not up. https://reviewboard.mozilla.org/r/238834/#review244870 Although for making Android's behavior on desktop OSes testable, #ifndef is not good, but I'm not sure if it's worthwhile to make it a pref. So, it looks okay to use #ifndef.

Pushed by cpearce@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e3b70cecf471 Gesture activate documents on key/mouse down not up. r=masayuki https://hg.mozilla.org/integration/autoland/rev/10ac210e88db Test that documents gesture activate on key/mouse down instead of up. r=masayuki

Backed out 2 changesets (bug 1456037) for bustage in z:/build/build/src/dom/events/EventStateManager.cpp on a CLOSED TREE Problematic push https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=10ac210e88db65412b3a62ae99a15dbb883ba0d1&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified Bustage: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=175263207 Backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b22046e5f4d540dfe506a43e9810c03516c3746b&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified Log: https://treeherder.mozilla.org/logviewer.html#?job_id=175263207&repo=autoland&lineNumber=22005 05:16:45 INFO - z:/build/build/src/dom/events/EventStateManager.cpp(937): error C2121: '#': invalid character: possibly the result of a macro expansion 05:16:45 INFO - z:/build/build/src/dom/events/EventStateManager.cpp(937): error C2065: 'ifndef': undeclared identifier 05:16:45 INFO - z:/build/build/src/dom/events/EventStateManager.cpp(937): error C2146: syntax error: missing ')' before identifier 'MOZ_WIDGET_ANDROID' 05:16:45 INFO - z:/build/build/src/dom/events/EventStateManager.cpp(937): error C2146: syntax error: missing '>' before identifier 'aEvent' 05:16:45 INFO - z:/build/build/src/dom/events/EventStateManager.cpp(937): error C2947: expecting '>' to terminate template-argument-list, found '>' 05:16:45 INFO - z:/build/build/src/dom/events/EventStateManager.cpp(937): error C2955: 'mozilla::detail::AssertionConditionType': use of class template requires template argument list 05:16:45 INFO - z:\build\build\src\obj-firefox\dist\include\mozilla/Assertions.h(396): note: see declaration of 'mozilla::detail::AssertionConditionType' 05:16:45 INFO - z:/build/build/src/dom/events/EventStateManager.cpp(937): error C2057: expected constant expression 05:16:45 INFO - z:/build/build/src/dom/events/EventStateManager.cpp(937): error C2143: syntax error: missing ';' before '{' 05:16:45 INFO - z:/build/build/src/config/rules.mk:1024: recipe for target 'EventStateManager.obj' failed 05:16:45 INFO - mozmake.EXE[4]: *** [EventStateManager.obj] Error 2 05:16:45 INFO - mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/events' 05:16:45 INFO - mozmake.EXE[4]: *** Waiting for unfinished jobs.... 05:16:45 INFO - mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/base'

Pushed by cpearce@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f38381338fee Gesture activate documents on key/mouse down not up. r=masayuki https://hg.mozilla.org/integration/autoland/rev/a5fe3122bc59 Test that documents gesture activate on key/mouse down instead of up. r=masayuki

https://hg.mozilla.org/mozilla-central/rev/f38381338fee https://hg.mozilla.org/mozilla-central/rev/a5fe3122bc59