Add focusin and focusout events #10235
Conversation
These events are already implemented in browsers and are documented on MDN. This PR also adds onfocusin and onfocusout event handler content attributes. whatwg#3514 tracks this and other focus events. Fixes whatwg#10234
| @@ -79902,12 +79920,12 @@ dictionary <dfn dictionary>ToggleEventInit</dfn> : <span>EventInit</span> { | |||
| </ol> | |||
|
|
|||
| <p>To <dfn>fire a focus event</dfn> named <var>e</var> at an element <var>t</var> with a given | |||
There was a problem hiding this comment.
Let's make this more of a normal algorithm declaration:
To fire a focus event named event at an element target given an element relatedTarget and an optional boolean bubbles (default false), ...
And then update call sites to uniformly be either
[fire a focus event] named
fooat bar given baz
or
[fire a focus event] named
fooat bar given baz and true
| @@ -13135,6 +13135,8 @@ https://software.hixie.ch/utilities/js/live-dom-viewer/?%3C%21DOCTYPE%20HTML%3E% | |||
| <li><code data-x="handler-onended">onended</code></li> | |||
| <li><code data-x="handler-onerror">onerror</code>*</li> | |||
| <li><code data-x="handler-onfocus">onfocus</code>*</li> | |||
| <li><code data-x="handler-onfocusin">onfocusin</code>*</li> | |||
| <li><code data-x="handler-onfocusout">onfocusout</code>*</li> | |||
There was a problem hiding this comment.
This * means that they belong to the "Window-reflecting body element event handler set" instead of being the normal list at the top of https://whatpr.org/html/10235/webappapis.html#event-handlers-on-elements,-document-objects,-and-window-objects .
Can you confirm that difference is tested? (I'm honestly not sure exactly how you would test, this will take a bit of research. Or maybe change something in the implementation and see if any tests start failing.)
There was a problem hiding this comment.
Ok, I see that for example onclick is in the first table and onfocus is in the second one.
I made a quick test page, and it would seem that document.body doesn't do onfocus, but it does to onclick, and that was the only difference that I found.
Does that match up with your understanding?
There was a problem hiding this comment.
skimming the chromium code, I'm not sure what the difference is between onclick and onfocus.
They are both listed equally in all these files:
- https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/global_event_handlers.idl
- https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_attribute_names.json5
- https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/events/event_target.h;l=226-331;drc=82609dbef7832b13135ab4b9cd2f2a5770021677
There was a problem hiding this comment.
I think the difference in your test is due to the difference between click being bubbling and focus not.
The example below https://html.spec.whatwg.org/#the-body-element:window-reflecting-body-element-event-handler-set seems to have a possible test case at least for bubbling events.
I bet in Chromium there's some list hidden somewhere for the special ones.
There was a problem hiding this comment.
Ah I see, here it is: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_body_element.cc;l=115-216;drc=c0c69be50d661fd4952865e0aaf76952d9272f17
These are just for when they're set as HTML attributes rather than element.onfocusin which I am trying to implement in chromium to match safari.
It looks like chromium already implements onfocusin as an HTML attribute, but it is not included in the special body->window logic that onfocus has: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_element.cc;l=481-482;drc=040547c1a7d0987ca63e18ca010986113637bf51
So I guess that we should put onfocusin/onfocusout in the list that onfocus is not in? So I should remove that asterisk?
I also made a test page here, but the results only made me confused: https://jsfiddle.net/jarhar/84jLcptn/5/
There was a problem hiding this comment.
So I guess that we should put onfocusin/onfocusout in the list that onfocus is not in? So I should remove that asterisk?
Yes, I think that makes the most sense.
For testing, I figured out what we can do.
If you look at https://github.com/web-platform-tests/wpt/blob/master/html/webappapis/scripting/events/resources/event-handler-body.js , it pulls a list of all event handlers from the html.idl file. It then uses the windowReflectingBodyElementEventHandlerSet declaration to test how they should behave.
So, if we decide these should be in the "normal" list, we do no work. Updating the spec will eventually cause html.idl in the WPT repo to update, which will automatically make those tests start testing the right thing.
For your implementation, to verify everything is working correctly, you can modify the html.idl file locally, and make sure the tests still pass. When I do that right now, Chromium does not yet pass, I'd guess because the content attribute reflection isn't implemented. If you can confirm your patch makes such a local change pass, then I'll feel confident about our test coverage.
|
Browsing Blink source code I found these interesting if statements: I think these can have interesting impacts if you call Should we add that to the spec? Do we have tests for such cases? |
Nice, definitely!
I removed the logic in a chromium patch and am running all tests on it now to find out |
These events are already implemented in browsers and are documented on MDN. This PR also adds onfocusin and onfocusout event handler content attributes.
#3514 tracks this and other focus events.
Fixes #10234
(See WHATWG Working Mode: Changes for more details.)
/dom.html ( diff )
/indices.html ( diff )
/interaction.html ( diff )
/webappapis.html ( diff )