Closed Bug 1414336 Opened 2 years ago Closed 2 years ago

[Pointer Events] The main menu from calacademy.org is unresponsive if Pointer Events is preffed on

Categories

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

58 Branch
All
Windows
defect

Tracking

()

VERIFIED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- unaffected
firefox58 --- disabled
firefox59 --- verified

People

(Reporter: JuliaC, Assigned: stone)

References

(Blocks 1 open bug, )

Details

Attachments

(2 files, 4 obsolete files)

[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
Summary: [Pointer Events] The main menu from calacademy.org is unresponsive if if Pointer Events is preffed on → [Pointer Events] The main menu from calacademy.org is unresponsive if Pointer Events is preffed on
Assignee: nobody → sshih
Priority: -- → P2
Sounds like we should track this.
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.
Depends on: 1420589
[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.
This should be fixed after landing bug 1420589. Verified with nightly build 59.0a1 (2018-01-04) and it works fine.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
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?
Flags: needinfo?(sshih)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(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.
Flags: needinfo?(sshih)
(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).
Flags: needinfo?(iulia.cristescu)
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.
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.
Flags: needinfo?(iulia.cristescu)
Attachment #8942070 - Flags: review?(bugmail)
(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)
Flags: needinfo?(alexandru.simonca)
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.
Attachment #8942070 - Flags: review?(bugmail) → review+
(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.
Attachment #8942495 - Flags: review?(bugmail)
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.
Attachment #8942495 - Flags: review?(bugmail) → review+
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.
https://hg.mozilla.org/mozilla-central/rev/f59d5e77eee0
https://hg.mozilla.org/mozilla-central/rev/f80a0db70284
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
Flags: needinfo?(alexandru.simonca)
Thanks for your confirmation.
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...
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.
Flags: needinfo?(sshih)
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.
Attachment #8958642 - Attachment is obsolete: true
Attachment #8958642 - Flags: review?(bugmail)
You need to log in before you can comment on or make changes to this bug.