Closed Bug 1285128 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: DOM: Events, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: stone, Assigned: stone)

References

Details

(Whiteboard: btpp-active)

Attachments

(1 file, 4 obsolete files)

[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
Assignee: nobody → sshih
Summary: Should not trigger the handling for pointer capture states by pointer events which are disallowed to dispatch to DOM → Should not generate pointer events for those synthesized events that are not dispatched to DOM
Blocks: 1166347
Whiteboard: btpp-active
Blocks: 1267311
Blocks: 1270903
Blocks: 1267310
Is it make sense to not generate pointermove events for those synthesized mousemove events? Is there anything I should also take into consideration?
Attachment #8769525 - Flags: feedback?(bugs)
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.
Attachment #8769525 - Flags: feedback?(bugs) → feedback+
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
Attachment #8770436 - Flags: feedback?(btseng)
Refined the patch.
Attachment #8770436 - Attachment is obsolete: true
Attachment #8771298 - Flags: feedback?(btseng)
Attachment #8771338 - Flags: feedback?(btseng)
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!
Attachment #8771338 - Flags: feedback?(btseng) → feedback+
Attachment #8771338 - Flags: review?(bugs)
Attachment #8771338 - Flags: review?(bugs) → review+
Rebased the patch and refined the patch summary
Attachment #8771338 - Attachment is obsolete: true
Attachment #8776485 - Flags: review+
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/65f7f91ed630
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Blocks: 1180188
You need to log in before you can comment on or make changes to this bug.