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

RESOLVED FIXED in Firefox 51

Status

()

Core
DOM: Events
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: stone, Assigned: stone)

Tracking

(Blocks: 1 bug)

unspecified
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

(Whiteboard: btpp-active)

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

2 years ago
[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)

Updated

2 years ago
Assignee: nobody → sshih
(Assignee)

Updated

2 years ago
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

Updated

2 years ago
Blocks: 1166347
Whiteboard: btpp-active
(Assignee)

Updated

2 years ago
Blocks: 1267311
(Assignee)

Updated

2 years ago
Blocks: 1270903
(Assignee)

Updated

2 years ago
Blocks: 1267310
(Assignee)

Comment 1

2 years ago
Created attachment 8769525 [details] [diff] [review]
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?
Attachment #8769525 - Flags: feedback?(bugs)

Comment 2

2 years ago
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+
(Assignee)

Comment 3

2 years ago
Created attachment 8770436 [details] [diff] [review]
Should not generate pointer events for those synthesized events that are not dispatched to DOM
Attachment #8769525 - Attachment is obsolete: true
Attachment #8770436 - Flags: feedback?(btseng)
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)
(Assignee)

Comment 5

2 years ago
Created attachment 8771298 [details] [diff] [review]
Should not generate pointer events for those synthesized events that are not dispatched to DOM

Refined the patch.
Attachment #8770436 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8771298 - Flags: feedback?(btseng)
(Assignee)

Comment 6

2 years ago
Created attachment 8771338 [details] [diff] [review]
Should not generate pointer events for those synthesized events that are not dispatched to DOM
Attachment #8771298 - Attachment is obsolete: true
Attachment #8771298 - Flags: feedback?(btseng)
(Assignee)

Updated

2 years ago
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+
(Assignee)

Updated

2 years ago
Attachment #8771338 - Flags: review?(bugs)

Updated

2 years ago
Attachment #8771338 - Flags: review?(bugs) → review+
(Assignee)

Comment 9

2 years ago
Created attachment 8776485 [details] [diff] [review]
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
Attachment #8771338 - Attachment is obsolete: true
Attachment #8776485 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 10

2 years ago
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

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/65f7f91ed630
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
(Assignee)

Updated

2 years ago
Blocks: 1180188
You need to log in before you can comment on or make changes to this bug.