Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should "click", "dblclick" and "contextmenu" events be PointerEvents? #100

Closed
RByers opened this issue Jul 7, 2016 · 51 comments · Fixed by #317 or #404
Closed

Should "click", "dblclick" and "contextmenu" events be PointerEvents? #100

RByers opened this issue Jul 7, 2016 · 51 comments · Fixed by #317 or #404
Labels

Comments

@RByers
Copy link
Contributor

RByers commented Jul 7, 2016

@dtapuska just noticed that Edge sends click as an instance of PointerEvent, not just MouseEvent. I think we've agreed before that this was a good idea, but I don't think we've talked about speccing it.

I think we can just add a short section to the PE spec "upgrading" click to PointerEvent. Also dblclick and contextmenu. @teddink, that's it right? I didn't check drag events or anything.

@RByers RByers changed the title Specify that click is a PointerEvent? Specify that "click" is a PointerEvent? Jul 7, 2016
@RByers
Copy link
Contributor Author

RByers commented Jul 7, 2016

By the way, for context: The main reason we've said this is desirable it that sometimes it's useful to know the pointerType of a click / contextmenu. This is similar to, but subtly distinct from, wanting to know whether the source devices also fires touch events (sourceCapabilities.firesTouchEvents).

@dtapuska
Copy link

dtapuska commented Jul 7, 2016

In Edge I tested dragstart and the event fired has a drag prototype and the drag prototype is a mouse event prototype.

@NekR
Copy link

NekR commented Jul 7, 2016

I agree, it sounds good to have click as a PointerEvent. It'll provide only benefits to web devs.

@smaug----
Copy link
Contributor

contextmenu can be triggered also using keyboard, and click can be the default action for certain key events and click() dispatches MouseEvent click.
So need to define what pointerType should be in each of those cases. Default is "", I guess we could use that for now.

And HTML spec needs a change for click(), somewhere https://html.spec.whatwg.org/multipage/webappapis.html#fire-a-click-event or https://html.spec.whatwg.org/multipage/webappapis.html#fire-a-synthetic-mouse-event

@smaug----
Copy link
Contributor

Changing HTML spec might be a tiny bit controversial. Other option is to add a hook to the HTML spec where other specs could extends/re-define what kinds of events should be fired for click.

@patrickhlauke
Copy link
Member

Does Edge only "upgrade" click to a PointerEvent if it was the result of a pointer interaction? And if so, can it perhaps be defined in PE as an extension to the regular click event (leaving the HTML spec untouched)?

@smaug----
Copy link
Contributor

Would be rather odd to have platform dispatching 'click' as MouseEvent in some cases but as PointerEvent in some other cases. And error prone - event listeners couldn't trust the event to have the same properties each time.

@mustaqahmed mustaqahmed changed the title Specify that "click" is a PointerEvent? Specify that "click", "dblclick" and "contextmenu" events are PointerEvents? Jul 13, 2016
@RByers
Copy link
Contributor Author

RByers commented Jul 13, 2016

Looks like IE has had some version of this back to IE11. After some discussion it sounds like the design we think makes the most sense is:

  • All UA-generated click, dblclick, contextmenu events would be of type PointerEvent, with properties that match the corresponding pointer events (eg. pointerType, isPrimary).
  • click() and keyboard-initiated click/contextmenu events will have all PointerEvent properties set to their defaults (eg. pointerType="").
  • we'll treat DragEvent as a separate (low priority) issue (filed Should DragEvent be upgraded to a PointerEvent? #106).

Next step is to check to see to what extent Edge already matches this design. @teddink says he'll do this. As @smaug---- says, specing this properly will be hard. But perhaps we can just start with something partly hacky here in the PE spec and then try to get it integrated into the correct places.

@jacobrossi
Copy link
Member

I think this is roughly what we implemented yes. I think we also want PointerEvent for your "auxclick" proposal which currently uses MouseEvent (e.g. WICG/auxclick#6).

@jacobrossi
Copy link
Member

FYI - here's the old thread on this http://lists.w3.org/Archives/Public/public-pointer-events/2013JanMar/0081.html

:-)

@RByers
Copy link
Contributor Author

RByers commented Jul 14, 2016

Nice, thanks! I thought this was just something we chatted about. I assume you never took that step of proposing spec changes?

@mustaqahmed
Copy link
Member

Any chance click coordinates could be an issue if PointerEvent coordinates are all fractional (#107)? Any idea how frequent is coordinate-equality-checks with integers, like if (e.clientX == 20) ...?

@RByers
Copy link
Contributor Author

RByers commented Jul 25, 2016

Sure, there's a chance it could be an issue. What does Edge do? In the absence of any known issue, I'd just say we should try it.

@RByers
Copy link
Contributor Author

RByers commented Jul 26, 2016

Andrew (Edge team) says that they had to special case click for pointerType=mouse to work around an issue with a Microsoft Ajax framework. See discussion in hackathon notes. But happy to try and see if we can get away without this hack.

Repro from original Edge bug:

  • Open https://pptform.state.gov/ in Spartan
  • Check checkbox and click 'Submit'
  • On the next page, click on 'Submit' in 'Complete Form Online' section.

@RByers RByers changed the title Specify that "click", "dblclick" and "contextmenu" events are PointerEvents? Specify that "click", "dblclick" and "contextmenu" events are PointerEvents Jul 28, 2016
@RByers
Copy link
Contributor Author

RByers commented Dec 14, 2016

@teddink Can you look into what special cases / hacks you have for click etc? Rounding the co-ordinates to integers seems like a good idea (at least when pointerType=mouse). Is that it?

@NavidZ
Copy link
Member

NavidZ commented Jan 4, 2017

@teddink, did you get a chance to follow up on this? Does @RByers suggestion look reasonable and enough?

@smaug----
Copy link
Contributor

smaug---- commented Jan 4, 2017

Curious, if we need to add special cases for click being a pointer event, would it make sense to just keep click as mouse event?
(Adding more and more special cases to the platform just makes it weaker and harder to maintain.)

But perhaps having .pointerType especially (since that is probably rather useful) is strong enough reason to add special case for click.
One option is to add/move .pointerType to MouseEvent.

@RByers
Copy link
Contributor Author

RByers commented Jan 9, 2017

Moving pointerType up to MouseEvent would work for me. But I assumed that would be contentious / difficult since we'd have to move the definitions of different pointing devices up (perhaps evoking IP concerns around talking about touch screens in the WPWG). But perhaps it's no worse than trying to update specs to change the type of click and ultimately more useful?

Note that there'd be some compat risk with moving pointerType - eg. sites doing "feature detection" incorrectly as event.hasOwnProperty('pointerType') (I've seen this pattern cause real compat problems in other examples when we've moved a property up the prototype chain).

Alternately we could add a pointerType to InputDeviceCapabilities which just references the PointerEvents spec for the definition. I.e. the capability is "fires pointer events with the given pointerType value, or empty string if it doesn't fire pointer events at all".

But in general shipping code trumps spec purity. If we're going to try to convince Edge to change to something else we should have a compelling argument of why that's better for more than just spec authors ;-). Perhaps it's useful to have the pointerType on all MouseEvent instances, not just click etc?

@RByers
Copy link
Contributor Author

RByers commented Jan 11, 2017

On the call today @teddink said he'd get us details of what exactly Edge is doing for click (is it just truncating co-ordinates to integers).

More fundamentally, the question was raised of what specifically is the value of making click a PointerEvent. Ted said he'd see if he can find out why that was done - get a concrete use case. Is it just for pointerType? What would be the harm in Edge changing to match Chrome/Firefox in having click be just a MouseEvent?

Although I can see some value in having the pointerType on click perhaps it's going to be a ton of effort to get that spec'd properly for relatively little value?

@patrickkettner
Copy link

what are the advantages/rationales for upgrading click to be a pointerevent? any hints why Edge did it?

The thought process was that given touchscreens, a "click" event isn't a mouse event as much as it is an activation of interactivity event. Having {touch,mouse,pointer}tap events seems redundant, so we upgraded the click event to pointer with the additional metadata of device type

@mustaqahmed
Copy link
Member

I agree with @patrickkettner that click has evolved into a user activation (or interactivity) event, so it is no longer a strict mouse event. But from the same perspective, it is not a pointer event either, right?

Since almost the entire discussion revolved around the need for PointerEvents properties, let's summarize key points into a few buckets:

  • Fractional coordinates: The compat risk here is significant, and the majority (all?) of the suggestions suggest truncating to integers. So making click, dblclick and contextmenu PointerEvents adds no value to the current situation.

  • PointerEvent properties other than pointerType and pointerId: We concluded above that pressure should be zero for click since click follows pointerup. So we can only expect default/unknown values for tiltX, tiltY and twist too. The same decision goes to dblclick. We now have only the contextmenu event left but the event may be fired on pointerup (Windows) or pointerdown (non-Windows) or even through keyboard, so it is hard to infer anything from any particular value of these properties.

  • pointerType and pointerId: These seem to be the only properties we may need for for click, dblclick and contextmenu. I am yet to see a concrete use-case that requires these properties. Note that any app looking for these properties can already get them today by tracking pointerdown/pointerup events. Do we know about any such app today?

Since the compat risk of changing these old events is not negligible, I would suggest waiting until we have a deciding factor for the last point above. If needed, we may even mark this issue as non-v2 blocking.

@RByers
Copy link
Contributor Author

RByers commented Jun 14, 2017

Thanks for the summary @mustaqahmed. I agree that we don't yet have a compelling use case (certainly no concrete example of a real site that expects the Edge behavior). I'm OK removing the v2-blocking label and keeping this around as a bug that is a known interop difference between Edge and other implementations until at least one real-world site / scenario impacted by this materializes.

@RByers RByers changed the title Specify that "click", "dblclick" and "contextmenu" events are PointerEvents Should "click", "dblclick" and "contextmenu" events be PointerEvents? Jun 14, 2017
@patrickkettner
Copy link

Do we know about any such app today?

I have not been able to find any websites that require this behavior. When the change was made, it was more of the fact that it seemed more useful and there was no breakage when it was introduced.

@NavidZ NavidZ added the v3 label Mar 28, 2018
@NavidZ
Copy link
Member

NavidZ commented Mar 28, 2018

We talked about this in the call and Similar to #106 there are some advantages to this as well as possibilities of compat issues. Pushing this to V3 to further discuss this.

@BoCupp-Microsoft
Copy link

The open question on this thread seems to be about what use cases would require click, dblclick, and contextmenu to become a PointerEvent instead of a MouseEvent.

The use cases I know of involve detecting pointerType, but I don't think its fair to say that any of these cases actually "require" these events to be PointerEvents. A developer could remember the pointerType of the last primary pointerup and use that during the click event handler to differentiate behavior. So AFAICT the value of any change seems to be simplicity, i.e. developers can write less code to get differentiated behavior based on pointerType.

Two use cases to consider:
Video elements commonly show controls on the first touch so that the user can then tap the pause button, adjust volume, drag the scrubber, etc. Any move of the mouse can reveal the same controls so the click anywhere on the video can just directly toggle play/pause.

The select element of edgehtml-based Edge has touch-specific spacing for its select popup, i.e. opening the UI with your finger on a touchscreen device versus the mouse on the same device will lead to options that are spaced further apart for touch. While this UI is triggered on pointerdown I don't think its a stretch to say that any UI triggered on click or contextmenu could benefit from similar treatment.

Some (old) evidence I found that devs want this behavior: MSDN question

AFAICT YouTube has the differentiated behavior I describe for touch versus mouse when it comes to toggling play/pause for a video.

Lastly, this (also old) article talks about the same pattern of turning UI that requires hover and then click into a two-touch UI:

Make the UI take two single touch events to complete the click - the first click will show the hover information, the second will complete the action.

@patrickhlauke
Copy link
Member

patrickhlauke commented Jun 12, 2019

discussed on PEWG call today https://www.w3.org/2019/06/12-pointerevents-minutes.html

concerns about potential compat problems (including switch from integer to fractional coordinates in PE), and how to treat keyboard-generated events (invent new pointerType? just leave it empty/null?). historical issue of click etc being mouse event but triggered by keyboard too, more a generic activate event...

Chrome might make an experimental web feature where click etc are upgraded to PE and see what the potential compat impact actually is. at same time, discuss with UIEvents folks about best way forward.

@mustaqahmed
Copy link
Member

I just realized that Chrome Experimental has click, auxclick and dblclick as PointerEvents for about a year now, without any issue. @liviutinta is currently running a trail on Canary now, we will post the outcomes when available.

@NavidZ Wondering why the UI event change above didn't include context menu and dblclick. Did we plan to cover them later or not?

@NavidZ
Copy link
Member

NavidZ commented Aug 31, 2020

UI event doesn't define contextmenu event. As a matter of fact no spec defines it. WHATWG dom spec briefly mentions its existence with no actual definition. I believe I have a pull request to update that and left it unmerged until we are sure this change is okay and is here to remain.

As for the dblclick event it is already a legacy event. People should not rely on dblclick event and instead should look into detail field of the click event for double/triple/... clicks. So we thought it was not worth the effort going after that.

@liviutinta
Copy link
Contributor

Clarification for @mustaqahmed's comment on Aug 31 [4] above.

When using [1] for testing in Chrome with ClickPointerEvent flag enabled, contextmenu is a pointer event. This is corroborated by code [2] as well as by the pointerevents spec pull request [3].

Testing with [1] in Chrome with ClickPointerEvent flag enabled shows dblclick is a mouse event.

[1] https://rbyers.github.io/eventTest.html
[2] https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/core/input/mouse_event_manager.cc;drc=87a63ed093790f8c2d1751e5a3914dfb03352d21;l=307
[3] #317 (comment)
[4] #100 (comment)

@garykac
Copy link
Member

garykac commented Nov 4, 2020

Back in August, I started rewriting [1] parts of the UIEvent spec and included info about contextmenu along with hooks for PointerEvents in general.

[1] https://w3c.github.io/uievents/event-algo.html

pirxpilot pushed a commit to pirxpilot/mapbox-gl-js that referenced this issue Mar 6, 2021
looks like Chrome 89 is sending an instance of PointerEvent as a `click`
event

see: https://bugs.chromium.org/p/chromium/issues/detail?id=1182909

Pointer events have fractional mouse position which means that we cannot
compare directly with the position stored after `mousedown` event

for now use the configurable click tolerance for click events, which was
added to fix unrelated problem

in the longer term we should consider switching to handling
PointerEvents directly

see: https://developer.mozilla.org/en-US/docs/Web/API/PointerEvent
see: w3c/pointerevents#100
@mustaqahmed
Copy link
Member

In Chrome we encountered a few regressions in the wild in the last few weeks (while slowly ramping up our experiment on Stable channel since January this year). All of the regressions are with fractional coordinates in click where the event's clientX/Y are compared directly with those of the last mousedown event to detect/ignore "dragged clicks".

  • In some cases (like crbug.com/1182909) the developer was able to update their code with our suggested workaround.
  • But we need to avoid compat problems like crbug.com/1192449.

We decided in Chrome to ship the feature but without any change in coordinates. More precisely, click, auxclick and contextmenu events will be instances of PointerEvents but their coordinates will be integers (like MousEvents).

Reopening the issue because I think we need to update relevant specs accordingly.

@mustaqahmed mustaqahmed reopened this Apr 13, 2021
@patrickhlauke
Copy link
Member

@mustaqahmed suggest we tackle this as a priority for v3 - let's discuss at the next meeting.

@patrickhlauke
Copy link
Member

from a very basic check recently https://codepen.io/patrickhlauke/pen/XWxYRpv it seems that, at time of writing, only Chromium (Chrome/Win, Chrome/Android, Edge/Win, etc) has "upgraded" the click event to an actual PointerEvent.

Gecko (Firefox/Win, Firefox/Android) and WebKit (Safari/macOS, Safari/iOS) still haven't...which is a shame, because personally as a dev, I'd be excited to know "was this click caused by mouse, stylus, touch, or something else - most likely keyboard"

@mustaqahmed
Copy link
Member

We have added WPTs of the form pointerevents/.*is_a_pointerevent.* to test this: dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet