1299195 - [Pointer Events] Filter double click event (input source=touch) to prevent dispatching extra mousedown and pointerdown events to content

Status: RESOLVED FIXED

(Core :: DOM: Events, defect, P2)

Description

[Ben Hsu [:HoPang]]

Now, Windows fires both mouse platform messages and touch platform messages for touch inputs. This will cause Gecko fire two pointer events for a single user touch input.

However, when APZ is disabled, Gecko only handle mouse platform messages. Thus, if we filter mouse platform message derived from touch, we won't fire pointer event for touch inputs on NON-APZ configuration.

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Is this something we want to have in the next release/months, i.e. P2? Or is it closer to a nice-to-have (P3) speaking from now?

Comment 2

[Ben Hsu [:HoPang]]

IMHO, this is one of the fundamental issues of the entire Pointer Events implementation. Thus, if we are going to launch Pointer Events in any form (i.e., pre-release), then we have to "at least" have an answer to how do we deal this issue. Extremely, we can neglect and invalid this issue.

Comment 3

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Ben Hsu [:HoPang] from comment #2)
> IMHO, this is one of the fundamental issues of the entire Pointer Events
> implementation. Thus, if we are going to launch Pointer Events in any form
> (i.e., pre-release), then we have to "at least" have an answer to how do we
> deal this issue. Extremely, we can neglect and invalid this issue.

Thanks Ben!
@Stone, according to the milestone plan [1], which one this bug falls into?
[1] https://wiki.mozilla.org/TPE_DOM/Pointer_events#PHASE_1_-_Enabling_PE_on_Nightly

Comment 4

[Ming-Chou Shih [:stone]]

Is this the bug that we fire extra pointerdown event when double tap the screen? If it is, I can investigate it and update the fix plan.

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Ming-Chou Shih [:stone] from comment #4)
> Is this the bug that we fire extra pointerdown event when double tap the
> screen? If it is, I can investigate it and update the fix plan.

Ben, mind clarifying it? Thanks!

Comment 6

[Ben Hsu [:HoPang]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #5)
> (In reply to Ming-Chou Shih [:stone] from comment #4)
> > Is this the bug that we fire extra pointerdown event when double tap the
> > screen? If it is, I can investigate it and update the fix plan.
> 
> Ben, mind clarifying it? Thanks!

Thank both of you for investigating this :)
The summary here is one possible solution for that symptom.
However, if there is any other solution to fix the root cause,
please feel free to set this but to `WONTFIX`, or update the summary correspondingly.

Comment 7

[Ming-Chou Shih [:stone]]

The extra pointerdown is caused by WM_LBUTTONDBLCLK (input source=touch) and we fires following events. It only happened when e10s enabled.
  pointerdown
  pointerup
  pointercancel
  mousedown
  mouseup
  pointerdown (from WM_TOUCH)
  pointerdown (from WM_LBUTTONDBLCLK)
  mousedown (from WM_LBUTTONDBLCLK)
  pointerup
  pointercancel
  mousedown
  mouseup

With e10s disabled, we fires
  pointerdown
  mousedown
  pointerup
  mouseup
  pointerdown
  mousedown
  pointerup
  mouseup
  dblclick

We should keep the behaviors of e10s and non-e10s the same.

Test the double tap behavior on other browsers for reference
Canary fires following events
  pointerdown
  pointerup
  mousedown
  mouseup
  pointerdown
  pointerup
  mousedown
  mouseup
  dblclick

Edge fires following events and handle it as zoom in/out
  pointerdown
  pointerup

Comment 8

[Ming-Chou Shih [:stone]]

Hi Kats,
On e10s, we fire one extra pointerdown and one extra mousedown becuase of WM_LBUTTONDBLCLK. We don't fire dblclick. Should we add eMouseDoubleClick in [1] and synthesize eMouseDoubleClick in APZ to keep the behaviors of e10s and non-e10s the same? Or any suggestions?

[1] https://dxr.mozilla.org/mozilla-central/source/widget/windows/WinUtils.cpp#1103

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Hm, I don't think we should be mapping a double-tap to a double-click at all, but I could be convinced otherwise. But yes, I think it would be a good idea to add eMouseDoubleClick to the GetIsMouseFromTouch function so that we don't send it to content if we have touch support enabled.

Comment 10

[Ming-Chou Shih [:stone]]

Attachment: [Pointer Events] Filter double click event (input source=touch) to prevent dispatching extra mousedown and pointerdown events to content (V1)

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8793208 [details] [diff] [review]
[Pointer Events] Filter double click event (input source=touch) to prevent dispatching extra mousedown and pointerdown events to content (V1)

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

r=me with the comment below addressed.

::: gfx/layers/apz/test/mochitest/test_bug1299195.html
@@ +10,5 @@
> +  <script type="application/javascript" src="apz_test_utils.js"></script>
> +  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
> +  <script type="application/javascript">
> +  var subtests = [
> +    {'file': 'helper_bug1299195.html', 'prefs': [["dom.w3c_pointer_events.enabled", true]]}

I would prefer if you renamed test_bug1285070.html to test_group_pointerevents.html and then moved this one line into that file. The runSubtestsSeriallyInFreshWindows function already handles multiple subtests so there's no point in creating a separate top-level test for this.

Comment 12

[Ming-Chou Shih [:stone]]

Attachment: Bug 1299195 - [Pointer Events] Filter double click event (input source=touch) to prevent dispatching extra mousedown and pointerdown events to content. r=kats.

Updated the patch summary

Comment 13

[Ming-Chou Shih [:stone]]

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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6bdaf25ebce9
[Pointer Events] Filter double click event (input source=touch) to prevent dispatching extra mousedown and pointerdown events to content. r=kats

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6bdaf25ebce9