Iulia Cristescu, QA [:JuliaC] (away, please needinfo? cornel.ionce@softvision.ro) Reporter
	Description • 7 years ago

[Note]:
- This issue is only reproducible in tablet mode

[Affected versions]:
- Firefox 58.0a1 (2017-11-03)

[Affected platforms]:
- Windows 10 x64

[Steps to reproduce]:
1. Launch Firefox
2. - go to "about:config", add the pref "browser.tabs.remote.force-enable" as a boolean and set it to "true"
   - ensure that the "dom.w3c_pointer_events.enabled" pref is set to "true"
3. Go to calacademy.org and try to access the main menu from the site's top side
4. Return to "about:config" and set the "dom.w3c_pointer_events.enabled" pref to "false"
5. Go to calacademy.org and try to access the main menu from the site's top side

[Expected result]:
- [step3] and [step5] The user is able to properly access the calacademy.org main menu and its submenus 

[Actual result]:
- [step3] The main menu cannot be accesed
- [step5] The main menu and its submenus work properly

[Additional notes]:
- Reproduced this issue using Microsoft Surface 4 and Dell XPS 12 devices

	Julien Cristau [:jcristau]
	Comment 1 • 7 years ago

Sounds like we should track this.

	Ming-Chou Shih [:stone] Assignee
	Comment 2 • 7 years ago

We found this may be related to bug 1420589 and planned to ship pointer event after fixing it. It may take some time so we won't enable pointer event in FF58.

	Hsin-Yi Tsai (she/her) [:hsinyi]
	Comment 3 • 7 years ago

[Tracking Requested - why for this release]:
request to de-nominate tracking-firefox 58 per comment 2 that this feature will remain pref-off in 58.

	Julien Cristau [:jcristau]
	Comment 4 • 7 years ago

Sure.

	Ming-Chou Shih [:stone] Assignee
	Comment 5 • 6 years ago

This should be fixed after landing bug 1420589. Verified with nightly build 59.0a1 (2018-01-04) and it works fine.

	Iulia Cristescu, QA [:JuliaC] (away, please needinfo? cornel.ionce@softvision.ro) Reporter
	Comment 6 • 6 years ago

The issue is still reproducible on Microsoft Surface 4 and Dell XPS 12 devices, using 59.0a1 (2018-01-08). What devices did you use for verifying this? 
Also, maybe I didn't make myself understood well: the issue is triggered using touch and stylus, not using the mouse. 
Any thoughts about this?

	Ming-Chou Shih [:stone] Assignee
	Comment 7 • 6 years ago

(In reply to Iulia Cristescu, QA [:JuliaC] from comment #6)
> The issue is still reproducible on Microsoft Surface 4 and Dell XPS 12
> devices, using 59.0a1 (2018-01-08). What devices did you use for verifying
> this? 
> Also, maybe I didn't make myself understood well: the issue is triggered
> using touch and stylus, not using the mouse. 
> Any thoughts about this?

I thought this is reproduced with mouse (this is also reproducible with mouse before landing bug 1420589). Sorry about that and I'll check it.

	Ming-Chou Shih [:stone] Assignee
	Comment 8 • 6 years ago

(In reply to Ming-Chou Shih [:stone] from comment #7)
> (In reply to Iulia Cristescu, QA [:JuliaC] from comment #6)
> > The issue is still reproducible on Microsoft Surface 4 and Dell XPS 12
> > devices, using 59.0a1 (2018-01-08). What devices did you use for verifying
> > this? 
> > Also, maybe I didn't make myself understood well: the issue is triggered
> > using touch and stylus, not using the mouse. 
> > Any thoughts about this?
> 
> I thought this is reproduced with mouse (this is also reproducible with
> mouse before landing bug 1420589). Sorry about that and I'll check it.

Wondering that is this now only reproducible with touch? I can reproduce it with touch but can't with stylus (watcom intuos pen) on nightly 59.0a1 (2018-01-09 and 2018-01-10).

	Ming-Chou Shih [:stone] Assignee
	Comment 9 • 6 years ago

We fire pointercancel when the pointer is subsequently used to manipulate the page viewport. Cancel the event stops the default action and should stop firing pointercancel.

	Alexandru Simonca [:asimonca], Release Desktop QA
	Comment 11 • 6 years ago

I can't reproduce it with the Wacom tablet but it is reproducible on the Surface 4 machine. The menu doesn't open even if clicked.

	Ming-Chou Shih [:stone] Assignee
	Comment 13 • 6 years ago

(In reply to Alexandru Simonca, QA (:asimonca) from comment #11)
> I can't reproduce it with the Wacom tablet but it is reproducible on the
> Surface 4 machine. The menu doesn't open even if clicked.

Found that the events dispatched on Surface 4 with a stylus are touch events (which is the same as Chrome and Edge with touch enabled). So it should be fixed with my patch.

Also, found that the highlighted menu is incorrect and can be reproduced on Edge and Chrome (with touch enabled). Assume this problem is website specific.

I had verified locally, and it works as expected. Could you help me to confirm it, please? The test build is in https://queue.taskcluster.net/v1/task/c7cGOglrStmXff38fAsR1A/artifacts/public/build/target.zip (Windows10 x64)

	Ming-Chou Shih [:stone] Assignee
	Comment 14 • 6 years ago

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

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 15 • 6 years ago

Comment on attachment 8942070 [details] [diff] [review]
Don't fire pointercancel when content prevents default on touchstart

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

If it's not too hard it would be good to add a test for this.

	Ming-Chou Shih [:stone] Assignee
	Comment 16 • 6 years ago

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #15)
> Comment on attachment 8942070 [details] [diff] [review]
> Don't fire pointercancel when content prevents default on touchstart
> 
> Review of attachment 8942070 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> If it's not too hard it would be good to add a test for this.

It shouldn't be a problem to create a test for it. I'll follow up.

	Ming-Chou Shih [:stone] Assignee
	Comment 19 • 6 years ago

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

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 20 • 6 years ago

Comment on attachment 8942495 [details] [diff] [review]
Add a test case to make sure pointercancel isn't fired when content prevents default on touchstart

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

Looks fine to me. You may want to also add the android mochitest jobs to your try push to make sure it's passing there. Android often behaves differently from the desktop platforms.

::: dom/events/test/pointerevents/test_bug1414336.html
@@ +71,5 @@
> +
> +    target0_events.forEach((elem, index, arr) => {
> +      target0.addEventListener(elem, (event) => {
> +        is(event.type, target0_events[0], "receive " + event.type + " on target0");
> +        target0_events = target0_events.filter(item => item !== event.type);

This filter call seems unnecessarily complex. You should just be able to use target0_events.shift() to drop the first element.

@@ +77,5 @@
> +    });
> +
> +    target0.addEventListener("pointercancel", (event) => {
> +      ok(false, "Shouldn't receive pointercancel when content prevents default on touchstart");
> +      ok(target0_events.length == 0, " should receive " + target0_events + " on target0");

This second ok can be removed, since if this listener is triggered at all the first ok will fail the test.

	Ming-Chou Shih [:stone] Assignee
	Comment 21 • 6 years ago

Added try results on android
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c14897efe69287d71f8f3b9b62425682fc28d909

	Pulsebot
	Comment 22 • 6 years ago

Pushed by sshih@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f59d5e77eee0
Don't fire pointercancel when content prevents default on touchstart. r=kats.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f80a0db70284
Add a test case to make sure pointercancel isn't fired when content prevents default on touchstart. r=kats.

	Raul Gurzau (:RaulG)
	Comment 23 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/f59d5e77eee0
https://hg.mozilla.org/mozilla-central/rev/f80a0db70284

	Alexandru Simonca [:asimonca], Release Desktop QA
	Comment 24 • 6 years ago

Hi, 
I have just verified this fix on Windows 10 x64 on the Surface 4 and on the Wacom tablet linked to a desktop machine. Everything works as expected. Marking it verified fixed.

	Ming-Chou Shih [:stone] Assignee
	Comment 25 • 6 years ago

Thanks for your confirmation.

	Cosmin Sabou [:CosminS]
	Comment 26 • 6 years ago

This started to perma fail since it has been merge to central.

https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&noautoclassify&filter-searchStr=06772047c95a4dfe27c3e1de3555aae82783a40d&fromchange=04c0a07b8de21300856ec89b7d118d4be9b86250&selectedJob=156339901

[task 2018-01-16T10:50:49.757Z] 10:50:49     INFO - TEST-START | dom/events/test/pointerevents/test_bug1414336.html
[task 2018-01-16T10:50:49.819Z] 10:50:49     INFO - GECKO(2139) | Flushing APZ repaints was a no-op, triggering callback directly...
[task 2018-01-16T11:07:29.838Z] 11:07:29     INFO - Automation Error: mozprocess timed out after 1000 seconds running ['/builds/worker/workspace/build/venv/bin/python', '-u', '/builds/worker/workspace/build/tests/mochitest/runtests.py', '--disable-e10s', '--total-chunks', '10', '--this-chunk', '3', '--jscov-dir-prefix=/builds/worker/workspace/build/blobber_upload_dir', '--appname=/builds/worker/workspace/build/application/firefox/firefox', '--utility-path=tests/bin', '--extra-profile-file=tests/bin/plugins', '--symbols-path=https://queue.taskcluster.net/v1/task/XOLBomeRRVyl1EvKKdPsXQ/artifacts/public/build/target.crashreporter-symbols.zip', '--certificate-path=tests/certs', '--setpref=webgl.force-enabled=true', '--quiet', '--log-raw=/builds/worker/workspace/build/blobber_upload_dir/plain-chunked-coverage_raw.log', '--log-errorsummary=/builds/worker/workspace/build/blobber_upload_dir/plain-chunked-coverage_errorsummary.log', '--use-test-media-devices', '--screenshot-on-fail', '--cleanup-crashes', '--marionette-startup-timeout=180', '--sandbox-read-whitelist=/builds/worker/workspace/build', '--log-raw=-', '--chunk-by-dir=4', '--timeout=1200']
[task 2018-01-16T11:07:29.868Z] 11:07:29    ERROR - timed out after 1000 seconds of no output
[task 2018-01-16T11:07:29.869Z] 11:07:29    ERROR - Return code: -15
[task 2018-01-16T11:07:29.869Z] 11:07:29    ERROR - No suite end message was emitted by this harness.
[task 2018-01-16T11:07:29.869Z] 11:07:29     INFO - TinderboxPrint: mochitest-plain-chunked-coverage<br/>105/0/1
[task 2018-01-16T11:07:29.869Z] 11:07:29    ERROR - # TBPL FAILURE #
[task 2018-01-16T11:07:29.870Z] 11:07:29  WARNING - setting return code to 2
[task 2018-01-16T11:07:29.870Z] 11:07:29    ERROR - The mochitest suite: plain-chunked-coverage ran with return status: FAILURE
[task 2018-01-16T11:07:29.870Z] 11:07:29     INFO - Running post-action listener: _package_coverage_data
[task 2018-01-16T11:07:29.870Z] 11:07:29     INFO - Beginning compression of JSDCov artifacts...

	Ming-Chou Shih [:stone] Assignee
	Comment 27 • 6 years ago

Wondering where I can get more information about the build 'JSDCov'? Found the log shows '--disable-e10s'. I might have to check if apz is enabled before testing.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 28 • 6 years ago

That sounds reasonable. You can wrap the start of the test with isApzEnabled(), like at https://searchfox.org/mozilla-central/rev/48cbb200aa027a0a379b6004b6196a167344b865/gfx/layers/apz/test/mochitest/test_bug1253683.html#42

	Emilio Cobos Álvarez (:emilio)
	Comment 31 • 6 years ago

Comment on attachment 8958642 [details]
Make pointerevents/test_bug1414336.html more reliable.

Whoops, mozreview autodetected the bug and messed up because of the test-name. This is for bug 1445478.