-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Clarify that <details open> fires the toggle event even if the open attribute is set from the parser #4500
Comments
Good catch. Can you think of anything clearer than just adding "(including by the parser)" to that sentence? It feels a bit unsatisfying, but I don't have any better ideas. |
That sounds ok to me. web-platform-tests/wpt#16244 is the test for this. |
It seems bad to me that this would dispatch in an |
Closes #4500. Tests: web-platform-tests/wpt#16244 Also moves the task source inline, per #4506.
We'd probably need data to prove that people are not relying on that behavior... It's yet another XSS vector, but not sure if that's enough justification to change it (it's not more of an XSS vector than |
Oh re-reading my own comment, I guess Is there another precedent for something like this? |
There's no precedent, this is pretty bad. Having said that, I cannot get client = new XMLHttpRequest()
client.open("GET", 'data:text/xml,<html xmlns="http://www.w3.org/1999/xhtml"><details open="" ontoggle="alert(1)"/></html>', false);
client.send();
console.log(client.responseXML); to run script in browsers, so this will need more tests and more complicated behavior if we wanted to go down this route, right? |
Scripting is disabled in XML responseDocs. The event fires, but you have to add the handler from outside. See web-platform-tests/wpt#16244 for an example. |
I filed crbug.com/960252 for adding a new counter to Chrome. |
Result: less than 0.0004% https://chromestatus.com/metrics/feature/timeline/popularity/2978 I think we can change this behavior. |
I'd propose we add "is browsing-context connected" to the conditions for queuing the task to fire the event. |
At TPAC 2023 we discussed this and are generally OK with firing events from the parser. I'll work on driving #4574 and web-platform-tests/wpt#16244 to completion. |
Closes #4500. Tests: web-platform-tests/wpt#16244 Also moves the task source inline, per #4506.
… event even from the parser., a=testonly Automatic update from web-platform-tests HTML: Test that <details open> fires the event even from the parser. whatwg/html#4500 -- wpt-commits: 06854ec7854c9061b84a8d675ff6fb8f0dcd08fc wpt-pr: 16244
… event even from the parser., a=testonly Automatic update from web-platform-tests HTML: Test that <details open> fires the event even from the parser. whatwg/html#4500 -- wpt-commits: 06854ec7854c9061b84a8d675ff6fb8f0dcd08fc wpt-pr: 16244
… event even from the parser., a=testonly Automatic update from web-platform-tests HTML: Test that <details open> fires the event even from the parser. whatwg/html#4500 -- wpt-commits: 06854ec7854c9061b84a8d675ff6fb8f0dcd08fc wpt-pr: 16244 UltraBlame original commit: 93d1a7cfc9b50da825cb82bd72fabbd30218c41f
… event even from the parser., a=testonly Automatic update from web-platform-tests HTML: Test that <details open> fires the event even from the parser. whatwg/html#4500 -- wpt-commits: 06854ec7854c9061b84a8d675ff6fb8f0dcd08fc wpt-pr: 16244 UltraBlame original commit: 93d1a7cfc9b50da825cb82bd72fabbd30218c41f
… event even from the parser., a=testonly Automatic update from web-platform-tests HTML: Test that <details open> fires the event even from the parser. whatwg/html#4500 -- wpt-commits: 06854ec7854c9061b84a8d675ff6fb8f0dcd08fc wpt-pr: 16244 UltraBlame original commit: 93d1a7cfc9b50da825cb82bd72fabbd30218c41f
… event even from the parser., a=testonly Automatic update from web-platform-tests HTML: Test that <details open> fires the event even from the parser. whatwg/html#4500 -- wpt-commits: 06854ec7854c9061b84a8d675ff6fb8f0dcd08fc wpt-pr: 16244
… event even from the parser., a=testonly Automatic update from web-platform-tests HTML: Test that <details open> fires the event even from the parser. whatwg/html#4500 -- wpt-commits: 06854ec7854c9061b84a8d675ff6fb8f0dcd08fc wpt-pr: 16244
… event even from the parser., a=testonly Automatic update from web-platform-tests HTML: Test that <details open> fires the event even from the parser. whatwg/html#4500 -- wpt-commits: 06854ec7854c9061b84a8d675ff6fb8f0dcd08fc wpt-pr: 16244 UltraBlame original commit: 582cc4d01280d499d3511ee56ad2d302c64758af
… event even from the parser., a=testonly Automatic update from web-platform-tests HTML: Test that <details open> fires the event even from the parser. whatwg/html#4500 -- wpt-commits: 06854ec7854c9061b84a8d675ff6fb8f0dcd08fc wpt-pr: 16244 UltraBlame original commit: 582cc4d01280d499d3511ee56ad2d302c64758af
… event even from the parser., a=testonly Automatic update from web-platform-tests HTML: Test that <details open> fires the event even from the parser. whatwg/html#4500 -- wpt-commits: 06854ec7854c9061b84a8d675ff6fb8f0dcd08fc wpt-pr: 16244 UltraBlame original commit: 582cc4d01280d499d3511ee56ad2d302c64758af
https://html.spec.whatwg.org/#the-details-element says:
That doesn't say what happens when the parser sets the attribute, so e.g., you load
data:text/html,<details open ontoggle="alert(1)">
.Looks like all implementations are interoperable in this case, but it's not so obvious from the spec text, so should probably be clarified. I can write a test.
The text was updated successfully, but these errors were encountered: