Skip to content

Change the type of click, auxclick, and contextmenu to PointerEvent#317

Merged
patrickhlauke merged 5 commits into
gh-pagesfrom
click-auxclick-contextmenu-type
Dec 14, 2020
Merged

Change the type of click, auxclick, and contextmenu to PointerEvent#317
patrickhlauke merged 5 commits into
gh-pagesfrom
click-auxclick-contextmenu-type

Conversation

@NavidZ

@NavidZ NavidZ commented Mar 9, 2020

Copy link
Copy Markdown
Member

This is the followup change to WHATWG html spec and ui events spec pull requests to change the type of click, auxclick, and contextmenu events to pointerevents:

Closes #100


Preview | Diff

@NavidZ NavidZ requested a review from smaug---- March 9, 2020 17:42
@patrickhlauke

Copy link
Copy Markdown
Member

what's the status of this? just waiting for a review, or any more things happening upstream/in UIEvents?

@NavidZ

NavidZ commented Apr 20, 2020

Copy link
Copy Markdown
Member Author

The ui events part landed. The html spec part is sent. But Domenic suggested to wait until Chrome at least enables this and makes sure there is no regression. If so we will land the one in html spec. We can do either. Either merge now and if it turns out to cause some unrecoverable regressions then revert it. Or just wait until Chrome ships and confirms there is no bad regressions and then merge. I'll leave it up to you.

@patrickhlauke patrickhlauke self-requested a review July 13, 2020 22:46
Comment thread index.html Outdated
Comment thread index.html Outdated
@patrickhlauke

Copy link
Copy Markdown
Member

Just getting this ready but waiting for any tests/regression checks before merging.

@smaug----

Copy link
Copy Markdown
Contributor

@NavidZ Is this now enabled by default in Chrome?

Comment thread index.html Outdated
@NavidZ

NavidZ commented Oct 9, 2020

Copy link
Copy Markdown
Member Author

Looking at the code it seems it is still under experimental flag.

@mustaqahmed @liviutinta were running some experiments to make sure it is backward compatible and it doesn't cause any issues. I haven't heard any bad news out of that so far. Could you provide any update you might have?

@liviutinta

Copy link
Copy Markdown
Contributor

We are running an experiment, and gradually expanding its scope. There were no bugs logged yet. If there are no bugs logged by November we plan to start the shipping process. We believe the major risk will be around clientX/clientY being fractional. If there is any breakage, we plan to have a flag in the code to return clientX/clientY as long but keep click/auxclick/contextmenu events as PointerEvent.

- remove the requirement for positive pointerId which was not backwards-compatible (and I couldn't find reference to this change in minutes from PEWG meetings around that time)
- add mention of special case for zero pointerId (reusing some of the language from later on about PointerEvent stream)
@patrickhlauke patrickhlauke force-pushed the click-auxclick-contextmenu-type branch from 3775e86 to ad773db Compare November 23, 2020 11:00

@mustaqahmed mustaqahmed left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few more suggestions, sorry didn't spot them last time.

Comment thread index.html Outdated
Comment thread index.html Outdated
Comment thread index.html Outdated
@patrickhlauke

Copy link
Copy Markdown
Member

@smaug---- @mustaqahmed @NavidZ @liviutinta one final review ... we happy with the latest wording/changes on this?

@mustaqahmed mustaqahmed left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Patrick for addressing my comments, LGTM. I have one last suggestion; if you see a better way, please go ahead without waiting for my review again.

Comment thread index.html Outdated
@patrickhlauke patrickhlauke merged commit bb02e92 into gh-pages Dec 14, 2020
@patrickhlauke patrickhlauke deleted the click-auxclick-contextmenu-type branch December 14, 2020 00:45
Comment thread index.html
<dt><dfn>pointerId</dfn></dt>
<dd>
<p>A unique identifier for the pointer causing the event. This identifier MUST be unique from all other <a data-lt="active pointer">active pointers</a> in the <a>top-level browsing context</a> (as defined by [[HTML]]) at the time. A user agent MAY recycle previously retired values for <code>pointerId</code> from previous active pointers, if necessary.</p>
<p>A unique identifier for the pointer causing the event. This identifier MUST be unique from all other <a data-lt="active pointer">active pointers</a> in the <a>top-level browsing context</a> (as defined by [[HTML]]) at the time. The <code>pointerId</code> value of zero is reserved to indicate events that were generated by something other than a pointing device. A user agent MAY recycle previously retired values for <code>pointerId</code> from previous active pointers, if necessary.</p>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"value of zero is reserved to indicate events that were generated by something other than a pointing device." seems like backwards incompatible change.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. But the reason we chose that was no other implementation uses 0 that at this point. So we believe it should be safe to do so now. Otherwise, technically all the numbers are valid in pointerid space. Actually now that we are at it for breaking compat we can force all valid pointerid ones to be positive numbers (again something that the current implementations already do AFAIK) so maybe we can reserver those negative numbers for other future features or something.
Having said that, unless we add yet another field to the prototype to indicate non-pointing device I cannot imagine a way to be fully backward compatible.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point...looks like we assumed pointerId must start with 1. After your comment, I realized this behavior is mentioned only in the Note below. I will file an issue now.

@smaug----

Copy link
Copy Markdown
Contributor

There seem to be tests for this. _is_a_pointerevent...

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

Labels

None yet

5 participants