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

Added a global event handler for beforeinput #4530

Closed
wants to merge 6 commits into from

Conversation

phistuck
Copy link

@phistuck phistuck commented Apr 12, 2019

Also imported the input event from the UI events
specifications, since that one defines it.

Fixed #4526


💥 Error: Wattsi server error 💥

PR Preview failed to build. (Last tried on Jan 15, 2021, 7:59 AM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 Wattsi Server - Wattsi Server is the web service used to build the WHATWG HTML spec.

🔗 Related URL

<html>
<head><title>504 Gateway Time-out</title></head>
<body bgcolor="white">
<center><h1>504 Gateway Time-out</h1></center>
<hr><center>nginx/1.10.3</center>
</body>
</html>

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

Also imported the input event from the UI events
specification, since that one defines it.

Fixed whatwg#4526
Copy link
Contributor

@sideshowbarker sideshowbarker left a comment

Choose a reason for hiding this comment

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

LGTM

@sideshowbarker sideshowbarker added the impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation label Apr 13, 2019
@sideshowbarker
Copy link
Contributor

heads-up @whatwg/documentation

source Show resolved Hide resolved
@chrisdavidmills
Copy link

Documentation potential need recorded at https://trello.com/c/aBs1o6Pg/86-dom-general-enhancements-fx-67

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Thanks, so what remains here:

  • Implementer interest if not implemented.
  • Implementer bugs if not implemented.
  • WPT coverage. (I guess we get some automatically through IDL, maybe enough?)

@phistuck
Copy link
Author

phistuck commented Apr 17, 2019

@annevk - and verifying my participation.

Looks like WebKit added it already, without adding it to the standard -
https://trac.webkit.org/changeset/206944/webkit#file12
They added it to Element and Document, rather than to GlobalEventHandlers -
https://github.com/WebKit/webkit/blob/master/Source/WebCore/dom/DocumentAndElementEventHandlers.idl
They also seem to have added support for it as a content attribute (should new events be added as content attributes? I assumed not) -
https://github.com/WebKit/webkit/blob/master/Source/WebCore/html/HTMLAttributeNames.in

Chromium issue - https://bugs.chromium.org/p/chromium/issues/detail?id=947408

An intention by Mozilla to add it when they implement beforeinput -
w3c/uievents#218 (comment)

@phistuck
Copy link
Author

@annevk - regarding tests, yes, I assumed IDLs are automatically tested.

@annevk
Copy link
Member

annevk commented Apr 18, 2019

Ah yeah, it should be a content attribute as well.

@phistuck
Copy link
Author

@annevk - I will add it in the UTC+2 evening. Anything else left other than the content attribute and verifying my participation?

@annevk
Copy link
Member

annevk commented Apr 18, 2019

No, I think we're good then, thanks! I'll verify you now.

@phistuck
Copy link
Author

@annevk -

Ah yeah, it should be a content attribute as well.

Unrelated, but why does DOMContentLoaded not have an IDL/content attribute, too?

I added the content attribute.

One more thing, is adding my name to the acknowledgement section mandatory? I assumed it was, so I added, but I generally rather not if it is possible.

@domenic
Copy link
Member

domenic commented Apr 23, 2019

I'm not sure this should be verified, as I don't believe signing a contract using a pseudonym is viable legally...

@phistuck
Copy link
Author

phistuck commented Apr 23, 2019

@domenic - Google was fine with it, jQuery was fine with it (they did consult first) and maybe a few more I signed (RavenDB was not fine but only because they required an exact address and phone number which I am not willing to provide). But if you have an issue with that, I hereby release all rights for my change and you or anyone else is welcome to take it as their own, no credit needed.

@annevk
Copy link
Member

annevk commented Apr 23, 2019

@domenic I've verified other pseudonym-like names. Also, without ID verification of some kind many others could be pseudonyms too?

@annevk
Copy link
Member

annevk commented Apr 23, 2019

@phistuck adding your name to the Acknowledgments section is optional, feel free to remove it. DOMContentLoaded could maybe have an event handler attribute too. I think historically we might have tried to steer folks away from event handler attributes, but there's no real good reason to do so. One problem there is that the event name is not lowercase, but the event handler needs to be. That'd be a first and might be problematic for some implementations. Best discussed separately.

@phistuck
Copy link
Author

@annevk - done.

Thank you for the review, everyone.

@phistuck
Copy link
Author

@annevk and @domenic - is there anything else that is needed for this to be merged?

Like I mentioned, if there is a legal issue here, feel free to take the change as your own, credit-attribution-acknowledgement free, I am waiving all rights.

@phistuck
Copy link
Author

@annevk and @domenic - from the agreement status page -

After an agreement is submitted, it is verified by the WHATWG. This is a manual process, but is usually completed quickly if it's preventing a pull request from being accepted.

This is preventing a pull request from being accepted for three months. It would be great if you expedited the process.

@domenic
Copy link
Member

domenic commented Jul 15, 2019

We are blocked on confirmation from the SG as to whether pseudonyms are acceptable. This is out of our hands. We can close the PR if that's preferable to you.

Base automatically changed from master to main January 15, 2021 07:57
@mfreed7
Copy link
Contributor

mfreed7 commented Apr 19, 2021

Any update on this and #4526? Seems a shame to not be able to land the PRs over names vs. pseudonyms. Can we land this under a different name, or does a fresh PR need to be made?

@domenic
Copy link
Member

domenic commented Apr 19, 2021

A fresh PR is probably the best way forward.

@phistuck
Copy link
Author

@mfreed7 - sure, go for it, let me know when you have and I will close this one.

mfreed7 added a commit to mfreed7/html that referenced this pull request Jun 7, 2021
@mfreed7
Copy link
Contributor

mfreed7 commented Jun 7, 2021

@mfreed7 - sure, go for it, let me know when you have and I will close this one.

#6743

@annevk annevk closed this Jun 17, 2021
mfreed7 added a commit to mfreed7/html that referenced this pull request Jun 7, 2022
Also imported the input event from the UI events specifications, since that one defines it.

Fixes whatwg#4526

(This PR is directly pulled from whatwg#4530, which can't land due to IP issues.)
(This PR is a replacement for whatwg#6743, which I neglected for too long.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation
Development

Successfully merging this pull request may close these issues.

Add onbeforeinput to GlobalEventHandlers
6 participants