1285128 - Should not generate pointer events for those synthesized events that are not dispatched to DOM

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Ming-Chou Shih [:stone]]

[1] implies that the pending pointer capture should be processed when firing pointer events (except gotpointercapture or lostpointercapture). Some synthesized pointer events like [2] should also be excluded since they are transparent to content. Maybe we should never generate the corresponding pointer events.

[1] https://w3c.github.io/pointerevents/#process-pending-pointer-capture
[2] nsRefreshDriver will trigger synthesized mousemove event which generates pointermove event

Comment 1

[Ming-Chou Shih [:stone]]

Attachment: Should not generate pointer events for those synthesized events that are not dispatched to DOM

Is it make sense to not generate pointermove events for those synthesized mousemove events? Is there anything I should also take into consideration?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8769525 [details] [diff] [review]
Should not generate pointer events for those synthesized events that are not dispatched to DOM

makes sense. Synthetic mousemoves are for :hover handling and such.

Comment 3

[Ming-Chou Shih [:stone]]

Attachment: Should not generate pointer events for those synthesized events that are not dispatched to DOM

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8770436 [details] [diff] [review]
Should not generate pointer events for those synthesized events that are not dispatched to DOM

Review of attachment 8770436 [details] [diff] [review]:
-----------------------------------------------------------------

Let rename aIsSynthesized to aIsDOMEventSynthesized and aTriggerReason to aIsWidgetEventSynthesized to better understanding and we can get rid of the constants created in nsIDOMMouseEvent.idl with this approach. :)

Please revise and ask for feeback again.

Thanks!

::: dom/base/nsContentUtils.cpp
@@ +8028,5 @@
>                                 unsigned short aInputSourceArg,
>                                 bool aToWindow,
>                                 bool *aPreventDefault,
> +                               bool aIsSynthesized,
> +                               unsigned short aTriggerReason)

s/aIsSynthesized/aIsDOMEventSynthesized
s/unsigned short aTriggerReason/ bool aIsWidgetEventSynthesized

@@ +8065,5 @@
>  
> +  WidgetMouseEvent event(true, msg, widget,
> +                         aTriggerReason == nsIDOMMouseEvent::MOZ_REASON_REAL ?
> +                           WidgetMouseEvent::eReal :
> +                           WidgetMouseEvent::eSynthesized,

just replace with aIsWidgetEventSynthesized typed in bool.

::: dom/base/nsContentUtils.h
@@ +2533,5 @@
>                                   unsigned short aInputSourceArg,
>                                   bool aToWindow,
>                                   bool *aPreventDefault,
> +                                 bool aIsSynthesized,
> +                                 unsigned short aTriggerReason);

s/aIsSynthesized/aIsDOMEventSynthesized
s/unsigned short aTriggerReason/ bool aIsWidgetEventSynthesized

::: dom/base/nsDOMWindowUtils.cpp
@@ +628,5 @@
>                                   bool aIgnoreRootScrollFrame,
>                                   float aPressure,
>                                   unsigned short aInputSourceArg,
>                                   bool aIsSynthesized,
> +                                 unsigned short aTriggerReason,

s/aIsSynthesized/aIsDOMEventSynthesized
s/unsigned short aTriggerReason/ bool aIsWidgetEventSynthesized

@@ +635,5 @@
>  {
>    return SendMouseEventCommon(aType, aX, aY, aButton, aClickCount, aModifiers,
>                                aIgnoreRootScrollFrame, aPressure,
>                                aInputSourceArg, false, aPreventDefault,
> +                              aOptionalArgCount >= 4 ? aIsSynthesized : true,

s/aIsSynthesized/aIsDOMEventSynthesized

@@ +637,5 @@
>                                aIgnoreRootScrollFrame, aPressure,
>                                aInputSourceArg, false, aPreventDefault,
> +                              aOptionalArgCount >= 4 ? aIsSynthesized : true,
> +                              aOptionalArgCount >= 5 ? aTriggerReason :
> +                                                       nsIDOMMouseEvent::MOZ_REASON_REAL);

aOptionalArgCount >= 5 ? aIsWidgetEventSynthesized : false);

@@ +651,5 @@
>                                           bool aIgnoreRootScrollFrame,
>                                           float aPressure,
>                                           unsigned short aInputSourceArg,
>                                           bool aIsSynthesized,
> +                                         unsigned short aTriggerReason,

ditto

@@ +662,5 @@
>                                aIgnoreRootScrollFrame, aPressure,
>                                aInputSourceArg, true, nullptr,
> +                              aOptionalArgCount >= 4 ? aIsSynthesized : true,
> +                              aOptionalArgCount >= 5 ? aTriggerReason :
> +                                                       nsIDOMMouseEvent::MOZ_REASON_REAL);

ditto

@@ +678,5 @@
>                                         unsigned short aInputSourceArg,
>                                         bool aToWindow,
>                                         bool *aPreventDefault,
> +                                       bool aIsSynthesized,
> +                                       unsigned short aTriggerReason)

ditto.

@@ +684,5 @@
>    nsCOMPtr<nsIPresShell> presShell = GetPresShell();
>    return nsContentUtils::SendMouseEvent(presShell, aType, aX, aY, aButton,
>        aClickCount, aModifiers, aIgnoreRootScrollFrame, aPressure,
> +      aInputSourceArg, aToWindow, aPreventDefault, aIsSynthesized,
> +      aTriggerReason);

ditto.

::: dom/base/nsDOMWindowUtils.h
@@ +93,5 @@
>                                    unsigned short aInputSourceArg,
>                                    bool aToWindow,
>                                    bool *aPreventDefault,
> +                                  bool aIsSynthesized,
> +                                  unsigned short aTriggerReason);

s/aIsSynthesized/aIsDOMEventSynthesized
s/unsigned short aTriggerReason/ bool aIsWidgetEventSynthesized

::: dom/interfaces/base/nsIDOMWindowUtils.idl
@@ +304,5 @@
>     *        defaults to mouse input.
>     * @param aIsSynthesized controls nsIDOMEvent.isSynthesized value
>     *                       that helps identifying test related events,
>     *                       defaults to true
> +   * @param aTriggerReason controls WidgetMouseEvent.mReason value

s/aIsSynthesized/aIsDOMEventSynthesized
s/unsigned short aTriggerReason/ bool aIsWidgetEventSynthesized

@@ +305,5 @@
>     * @param aIsSynthesized controls nsIDOMEvent.isSynthesized value
>     *                       that helps identifying test related events,
>     *                       defaults to true
> +   * @param aTriggerReason controls WidgetMouseEvent.mReason value
> +   *                       defaults to WidgetMouseEvent::eReal

defaults to false ??

@@ +320,5 @@
>                           [optional] in boolean aIgnoreRootScrollFrame,
>                           [optional] in float aPressure,
>                           [optional] in unsigned short aInputSourceArg,
> +                         [optional] in boolean aIsSynthesized,
> +                         [optional] in unsigned short aTriggerReason);

s/aIsSynthesized/aIsDOMEventSynthesized
s/unsigned short aTriggerReason/ bool aIsWidgetEventSynthesized

@@ +440,5 @@
>                                [optional] in boolean aIgnoreRootScrollFrame,
>                                [optional] in float aPressure,
>                                [optional] in unsigned short aInputSourceArg,
> +                              [optional] in boolean aIsSynthesized,
> +                              [optional] in unsigned short aTriggerReason);

s/aIsSynthesized/aIsDOMEventSynthesized
s/unsigned short aTriggerReason/ bool aIsWidgetEventSynthesized

::: dom/interfaces/events/nsIDOMMouseEvent.idl
@@ +67,5 @@
>  
>    bool getModifierState(in DOMString keyArg);
> +
> +  const unsigned short    MOZ_REASON_REAL        = 0;
> +  const unsigned short    MOZ_REASON_SYNTHESIZED = 1;

We don't need these anymore.

::: gfx/layers/apz/util/APZCCallbackHelper.cpp
@@ +499,5 @@
>    bool defaultPrevented = false;
>    nsContentUtils::SendMouseEvent(aPresShell, aType, aPoint.x, aPoint.y,
>        aButton, aClickCount, aModifiers, aIgnoreRootScrollFrame, 0,
> +      aInputSourceArg, false, &defaultPrevented, false,
> +      nsIDOMMouseEvent::MOZ_REASON_REAL);

aInputSourceArg, false, &defaultPrevented, false, true);

::: layout/base/nsPresShell.cpp
@@ +7130,5 @@
>  {
>    EventMessage pointerMessage = eVoidEvent;
>    if (aEvent->mClass == eMouseEventClass) {
>      WidgetMouseEvent* mouseEvent = aEvent->AsMouseEvent();
>      // if it is not mouse then it is likely will come as touch event

// 1. If it is not mouse then it is likely will come as touch event.

@@ +7131,5 @@
>    EventMessage pointerMessage = eVoidEvent;
>    if (aEvent->mClass == eMouseEventClass) {
>      WidgetMouseEvent* mouseEvent = aEvent->AsMouseEvent();
>      // if it is not mouse then it is likely will come as touch event
> +    // We don't synthesize pointer events for those events that are not

// 2. We don't synthesize pointer events for those events that are not dispatched to DOM.

::: testing/mochitest/tests/SimpleTest/EventUtils.js
@@ +360,5 @@
>      var clickCount = aEvent.clickCount || 1;
>      var modifiers = _parseModifiers(aEvent, aWindow);
>      var pressure = ("pressure" in aEvent) ? aEvent.pressure : 0;
>      var inputSource = ("inputSource" in aEvent) ? aEvent.inputSource : 0;
>      var synthesized = ("isSynthesized" in aEvent) ? aEvent.isSynthesized : true;

s/synthesized/isDOMEventSynthesized

@@ +361,5 @@
>      var modifiers = _parseModifiers(aEvent, aWindow);
>      var pressure = ("pressure" in aEvent) ? aEvent.pressure : 0;
>      var inputSource = ("inputSource" in aEvent) ? aEvent.inputSource : 0;
>      var synthesized = ("isSynthesized" in aEvent) ? aEvent.isSynthesized : true;
> +    var triggerReason = ("triggerReason" in aEvent) ? aEvent.triggerReason : _EU_Ci.nsIDOMMouseEvent.MOZ_REASON_REAL;

s/var triggerReason/var isWidgetEventSynthesized
var isWidgetEventSynthesized = ("triggerReason" in aEvent) ? aEvent.triggerReason : false;

@@ +367,5 @@
>      if (("type" in aEvent) && aEvent.type) {
>        defaultPrevented = utils.sendMouseEvent(aEvent.type, left, top, button,
>                                                clickCount, modifiers, false,
>                                                pressure, inputSource,
> +                                              synthesized, triggerReason);

s/synthesized/isDOMEventSynthesized
s/triggerReason/isWidgetEventSynthesized

@@ +372,4 @@
>      }
>      else {
> +      utils.sendMouseEvent("mousedown", left, top, button, clickCount, modifiers, false,
> +                           pressure, inputSource, synthesized, triggerReason);

ditto

@@ +373,5 @@
>      else {
> +      utils.sendMouseEvent("mousedown", left, top, button, clickCount, modifiers, false,
> +                           pressure, inputSource, synthesized, triggerReason);
> +      utils.sendMouseEvent("mouseup", left, top, button, clickCount, modifiers, false,
> +                           pressure, inputSource, synthesized, triggerReason);

ditto

Comment 5

[Ming-Chou Shih [:stone]]

Attachment: Should not generate pointer events for those synthesized events that are not dispatched to DOM

Refined the patch.

Comment 6

[Ming-Chou Shih [:stone]]

Attachment: Should not generate pointer events for those synthesized events that are not dispatched to DOM

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8771338 [details] [diff] [review]
Should not generate pointer events for those synthesized events that are not dispatched to DOM

Review of attachment 8771338 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, thanks!

Comment 8

[Ming-Chou Shih [:stone]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1c619079719b4bdf8dc91ab53f6f0002b7140cd

Comment 9

[Ming-Chou Shih [:stone]]

Attachment: Should not generate pointer events for those synthesized events that are not dispatched to DOM. r=smaug

Rebased the patch and refined the patch summary

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/65f7f91ed630
Should not generate pointer events for those synthesized events that are not dispatched to DOM. r=smaug

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/65f7f91ed630