1675847 - Update click/auxclick/contextmenu and click() to use PointerEvent

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, task, P2)

Description

[Domenic Denicola]

See spec changes in:

• https://github.com/w3c/uievents/pull/259
• https://github.com/whatwg/html/pull/5324
• https://github.com/whatwg/html/pull/6126

Web platform tests:

• https://github.com/web-platform-tests/wpt/pull/26374
• https://github.com/web-platform-tests/wpt/pull/26407

Comment 1

[raluca]

Could you please clarify what is the current status for this ticket? The use case affected by the issue is not being able to identify touch devices correctly in the 'click' handler as pointer type is undefined (expected 'touch').

Comment 2

[Kagami Rosylight [:saschanaz] (they/them) (inactive until 2025-11-14)]

Olli showed the interest in the relevant PRs and I guess that means we can do this 🙂

Comment 3

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

Assigning to Masayuki as he is currently working on this.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

It seems that almost all tests passed with click event change which is the most complicated one.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Due to need to update EventNameList, we cannot fix this behind a pref. Smaug, do you think that we should fix this only in the Nightly channel with using #ifdef?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

That might be reasonable, assuming it is not too hard.
(Other option is to land the patches very early in a cycle so that we get more testing - but I'm not super worried about webcompat issues here.)

Comment 7

[Kagami Rosylight [:saschanaz] (they/them) (inactive until 2025-11-14)]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #5)

Due to need to update EventNameList, we cannot fix this behind a pref. Smaug, do you think that we should fix this only in the Nightly channel with using #ifdef?

It's still a header file, can't it be StaticPrefs::pref() ? ePointerEventClass : eMouseEventClass or such?

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #6)

That might be reasonable, assuming it is not too hard.
(Other option is to land the patches very early in a cycle so that we get more testing - but I'm not super worried about webcompat issues here.)

Okay, I'll add new macros to make it switchable at build time.

(In reply to Kagami Rosylight [:saschanaz] (they/them) from comment #7)

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #5)

Due to need to update EventNameList, we cannot fix this behind a pref. Smaug, do you think that we should fix this only in the Nightly channel with using #ifdef?

It's still a header file, can't it be StaticPrefs::pref() ? ePointerEventClass : eMouseEventClass or such?

Although some header files include mozilla/StaticPrefs_*.h, but I don't like to include it from EventNameList.h for the build performance even if we could.

Comment 9

[Kagami Rosylight [:saschanaz] (they/them) (inactive until 2025-11-14)]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #8)

Although some header files include mozilla/StaticPrefs_*.h, but I don't like to include it from EventNameList.h for the build performance even if we could.

How many files need to include the StaticPref header if we did this? dom/html has #define EVENT /* nothing */ so maybe only the 4 files in dom/base? https://searchfox.org/mozilla-central/search?q=%23define+EVENT%28&path=&case=true&regexp=false (Two of them are headers again, which can have more effect though. The other two already are using StaticPrefs_dom.)

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Oh, the EventClassId looks like not used excpt nsContentUtils. So, including it in the user side could make the dependency simpler. I'll try.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Attachment: Bug 1675847 - part 1: Rename `eMouseClick` and `eMouseAuxClick` r=smaug!,#dom-core

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Attachment: Bug 1675847 - part 2: Rename some methods which handle "MouseClick" r=smaug!,#dom-core

Depends on D212999

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Attachment: Bug 1675847 - part 3: Make `ePointerClick` event dispatchers and handlers use `WidgetPointerEvent` r=smaug!,#dom-core

This patch makes the all ePointerClick event dispatcher in C++ code use
WidgetPointerEvent instead of WidgetMouseEvent.

Then, this patch also makes the all click event dispatcher in chrome code use
PointerEvent instead of MouseEvent. For detecting wrong trusted event
dispatching of click event, this patch adds assertion into MouseEvent.
Therefore, all chrome test dispatchers also changed to use PointerEvent.

Finally, this patch includes a change of a WPT. That checks the pointerId
caused by executing an access key. In this case, the value should be -1
rather than the default value 0 because Pointer Event spec defines so for
synthetic pointer events caused by non-pointing devices [1]. Chrome also
sets it to -1 and fails [2]. Therefore, the new assertion will pass on both
Firefox and Chrome.

1. https://w3c.github.io/pointerevents/#dom-pointerevent-pointerid
2. https://wpt.fyi/results/uievents/interface/keyboard-accesskey-click-event.html?run_id=5087897523060736&run_id=5136270464647168&run_id=5163620816388096&run_id=5201281304231936

Depends on D213000

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Attachment: Bug 1675847 - part 4: Make `ePointerAuxClick` event dispatchers use `WidgetPointerEvent` r=smaug!,#dom-core

Depends on D213001

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Attachment: Bug 1675847 - part 5: Make `contextmenu` event dispatchers use `WidgetPointerEvent` or `PointerEvent` r=smaug!,#dom-core

eContextMenu event may be fired from widget. Therefore, different from
ePointerClick and ePointerAuxClick, they may cross the process boundary,
may be handled by APZ and may be dispatched into the DOM after a delay.
Therefore, this patch is complicated than the previous patch. This adds

• New IPC message handlers for sending/receiving a WidgetPointerEvent
• New DelayedPointerEvent class and templated MouseInput::ToWidgetEvent
• PresShell::EventHandler handles eContextMenu as same as WidgetMouseEvent

Depends on D213002

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Posted an Intent to change email.

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

mconley: could you review the patch as @pip-reviwers or ping somebody else to do it?

Comment 18

[Mike Conley (:mconley) (:⚙️)]

Sure, I can do it - sorry for the delay.

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #18)

Sure, I can do it - sorry for the delay.

Thak you!

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1151acb648a3
part 1: Rename `eMouseClick` and `eMouseAuxClick` r=smaug
https://hg.mozilla.org/integration/autoland/rev/91d448af3993
part 2: Rename some methods which handle "MouseClick" r=smaug
https://hg.mozilla.org/integration/autoland/rev/9a2834bb1a8e
part 3: Make `ePointerClick` event dispatchers and handlers use `WidgetPointerEvent` r=smaug,search-reviewers,devtools-reviewers,nchevobbe,jteow
https://hg.mozilla.org/integration/autoland/rev/449d6e7af6a6
part 4: Make `ePointerAuxClick` event dispatchers use `WidgetPointerEvent` r=smaug
https://hg.mozilla.org/integration/autoland/rev/1c3c048669bd
part 5: Make `contextmenu` event dispatchers use `WidgetPointerEvent` or `PointerEvent` r=smaug,pip-reviewers,devtools-reviewers,nchevobbe,mconley

Comment 21

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46743 for changes under testing/web-platform/tests

Comment 22

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/1151acb648a3
https://hg.mozilla.org/mozilla-central/rev/91d448af3993
https://hg.mozilla.org/mozilla-central/rev/9a2834bb1a8e
https://hg.mozilla.org/mozilla-central/rev/449d6e7af6a6
https://hg.mozilla.org/mozilla-central/rev/1c3c048669bd

Comment 23

[Kagami Rosylight [:saschanaz] (they/them) (inactive until 2025-11-14)]

Woohoo, thank you for doing this!

Comment 24

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot