-
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
Meta element and DOM post-insertion steps #10241
Comments
cc @smaug----, for the deviation from WebKit and Blink that we're observing with Gecko (regarding |
See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. FWIW, I did try and change the processing of these kinds of meta tags in Chromium to use the insertion steps [1], and without any extra work at least, this caused a bunch of crashes [2]. So this change (changing the test expectations) is easier, and aligns with more browsers. [1]: https://crrev.com/c/5399060 [2]: https://crbug.com/330694762 [email protected] Bug: 40150299 Change-Id: I60f8ba556984bc1e8df4ba6e1dc375b1dfb7823d
See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. FWIW, I did try and change the processing of these kinds of meta tags in Chromium to use the insertion steps [1], and without any extra work at least, this caused a bunch of crashes [2]. So this change (changing the test expectations) is easier, and aligns with more browsers. [1]: https://crrev.com/c/5399060 [2]: https://crbug.com/330694762 [email protected] Bug: 40150299, 330694762 Change-Id: I60f8ba556984bc1e8df4ba6e1dc375b1dfb7823d
Do you know why browsers don't use the normal insertion steps for these? It seems very strange. |
I looked into this a bit thanks to your comment. Apparently HTMLMetaElement can (or at least once could) run script synchronously, which is absolutely not allowed during the normal insertion steps, so its entire processing phase got relegated to the post-insertion steps. Assuming it still can run script synchronously for some meta types, I kind of imagine we could technically pull the non-script-running types into the "insertion steps", and keep only the script-running-types in the "post-insertion steps" but I'm not sure that buys us very much and it would require Blink & WebKit to change their longstanding behavior with probably very little upside for the vendors, so I'm not sure it's worth it. |
It's pretty hard to imagine how any meta elements could run scripts. I agree probably implementers won't want to invest any effort in changing this, but I wish we would, because this is bizarre. |
Yeah the only thing I can imagine would be unload or pagehide in a meta refresh, but those are actually unconditionally scheduled asynchronously anyways, so the script wouldn't run right away. I'll try and sniff around for other cases that motivated the original change, to see if they're still relevant. |
Are you sure default-style is processed at all in that testcase? |
Yep, in the post-insertion steps. |
See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. FWIW, I did try and change the processing of these kinds of meta tags in Chromium to use the insertion steps [1], and without any extra work at least, this caused a bunch of crashes [2]. So this change (changing the test expectations) is easier, and aligns with more browsers. [1]: https://crrev.com/c/5399060 [2]: https://crbug.com/330694762 [email protected] Bug: 40150299, 330694762 Change-Id: I60f8ba556984bc1e8df4ba6e1dc375b1dfb7823d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5410497 Reviewed-by: Noam Rosenthal <[email protected]> Commit-Queue: Dominic Farolino <[email protected]> Cr-Commit-Position: refs/heads/main@{#1281241}
See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. FWIW, I did try and change the processing of these kinds of meta tags in Chromium to use the insertion steps [1], and without any extra work at least, this caused a bunch of crashes [2]. So this change (changing the test expectations) is easier, and aligns with more browsers. [1]: https://crrev.com/c/5399060 [2]: https://crbug.com/330694762 [email protected] Bug: 40150299, 330694762 Change-Id: I60f8ba556984bc1e8df4ba6e1dc375b1dfb7823d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5410497 Reviewed-by: Noam Rosenthal <[email protected]> Commit-Queue: Dominic Farolino <[email protected]> Cr-Commit-Position: refs/heads/main@{#1281241}
See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. [email protected] Bug: 40150299 Change-Id: I3cabc92c16a4cfadc6675bd156aca04c2ade64aa
See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. [email protected] Bug: 40150299 Change-Id: I3cabc92c16a4cfadc6675bd156aca04c2ade64aa
See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. [email protected] Bug: 40150299 Change-Id: I3cabc92c16a4cfadc6675bd156aca04c2ade64aa
See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. [email protected] Bug: 40150299 Change-Id: I3cabc92c16a4cfadc6675bd156aca04c2ade64aa Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5420410 Commit-Queue: Dominic Farolino <[email protected]> Reviewed-by: Noam Rosenthal <[email protected]> Cr-Commit-Position: refs/heads/main@{#1284444}
See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. [email protected] Bug: 40150299 Change-Id: I3cabc92c16a4cfadc6675bd156aca04c2ade64aa Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5420410 Commit-Queue: Dominic Farolino <[email protected]> Reviewed-by: Noam Rosenthal <[email protected]> Cr-Commit-Position: refs/heads/main@{#1284444}
See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. [email protected] Bug: 40150299 Change-Id: I3cabc92c16a4cfadc6675bd156aca04c2ade64aa Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5420410 Commit-Queue: Dominic Farolino <[email protected]> Reviewed-by: Noam Rosenthal <[email protected]> Cr-Commit-Position: refs/heads/main@{#1284444}
…ations, a=testonly Automatic update from web-platform-tests Fix `<meta>` post-insertion steps expectations See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. FWIW, I did try and change the processing of these kinds of meta tags in Chromium to use the insertion steps [1], and without any extra work at least, this caused a bunch of crashes [2]. So this change (changing the test expectations) is easier, and aligns with more browsers. [1]: https://crrev.com/c/5399060 [2]: https://crbug.com/330694762 [email protected] Bug: 40150299, 330694762 Change-Id: I60f8ba556984bc1e8df4ba6e1dc375b1dfb7823d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5410497 Reviewed-by: Noam Rosenthal <[email protected]> Commit-Queue: Dominic Farolino <[email protected]> Cr-Commit-Position: refs/heads/main@{#1281241} -- wpt-commits: 38c6d6793e349f48b44950428bccd0f96d113b4e wpt-pr: 45457
…ps expectations, a=testonly Automatic update from web-platform-tests Fix `<meta>` referrer post-insertion steps expectations See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. [email protected] Bug: 40150299 Change-Id: I3cabc92c16a4cfadc6675bd156aca04c2ade64aa Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5420410 Commit-Queue: Dominic Farolino <[email protected]> Reviewed-by: Noam Rosenthal <[email protected]> Cr-Commit-Position: refs/heads/main@{#1284444} -- wpt-commits: 499f941b0fa6ed59e4a29c98a603813c358f17a9 wpt-pr: 45611
…ations, a=testonly Automatic update from web-platform-tests Fix `<meta>` post-insertion steps expectations See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. FWIW, I did try and change the processing of these kinds of meta tags in Chromium to use the insertion steps [1], and without any extra work at least, this caused a bunch of crashes [2]. So this change (changing the test expectations) is easier, and aligns with more browsers. [1]: https://crrev.com/c/5399060 [2]: https://crbug.com/330694762 [email protected] Bug: 40150299, 330694762 Change-Id: I60f8ba556984bc1e8df4ba6e1dc375b1dfb7823d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5410497 Reviewed-by: Noam Rosenthal <[email protected]> Commit-Queue: Dominic Farolino <[email protected]> Cr-Commit-Position: refs/heads/main@{#1281241} -- wpt-commits: 38c6d6793e349f48b44950428bccd0f96d113b4e wpt-pr: 45457
…ps expectations, a=testonly Automatic update from web-platform-tests Fix `<meta>` referrer post-insertion steps expectations See whatwg/html#10241 for the recommendation to change these test expectations to align with Chromium and WebKit's behavior. [email protected] Bug: 40150299 Change-Id: I3cabc92c16a4cfadc6675bd156aca04c2ade64aa Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5420410 Commit-Queue: Dominic Farolino <[email protected]> Reviewed-by: Noam Rosenthal <[email protected]> Cr-Commit-Position: refs/heads/main@{#1284444} -- wpt-commits: 499f941b0fa6ed59e4a29c98a603813c358f17a9 wpt-pr: 45611
For any given insert operation, these steps run for each inserted node synchronously after all node insertions are complete. This closes #732. The goal here is to separate the following: 1. Script-observable but not-script-executing insertion side effects - This includes synchronously applying styles to the document, etc... 2. Script-executing insertion side effects - This includes creating an iframe's document and synchronously firing its load event For any given call to insert, the above model allows us to keep all of (1) running synchronously after each node's insertion (as part of its insertion steps), while pushing all script-executing (or DOM tree-modifying or frame tree-modifying etc.) side effects to the new set of post-connection steps, which run synchronously during insertion, but _after all_ nodes finish their insertion. This essentially makes insertions "atomic" from the perspective of script, since script will not run until a given batch of DOM insertions are complete. This two-stage approach aligns the spec with a model most similar to Blink & Gecko's implementation, and fixes #808. This PR also helps out with whatwg/html#1127 and #575 (per #732 (comment)). To accomplish, this we audited all insertion side effects on the web platform in https://docs.google.com/document/d/1Fu_pgSBziVIBG4MLjorpfkLTpPD6-XI3dTVrx4CZoqY/edit#heading=h.q06t2gg4vpw, and catalogued whether they have script-observable side-effects, whether they invoke script, whether we have tests for them, and how each implementation handles them. This gave us a list of present "insertion steps" that should move to the "post-connection steps", because they invoke script and therefore prevent insertions from being "atomic". This PR is powerless without counterpart changes to HTML, which will actually _use_ the post-connection steps for all current insertion steps that invoke script or modify the frame tree. The follow-up HTML work is tracked here: - whatwg/html#10188 - whatwg/html#10241
For any given insert operation, these steps run for each inserted node synchronously after all node insertions are complete. This closes whatwg#732. The goal here is to separate the following: 1. Script-observable but not-script-executing insertion side effects - This includes synchronously applying styles to the document, etc... 2. Script-executing insertion side effects - This includes creating an iframe's document and synchronously firing its load event For any given call to insert, the above model allows us to keep all of (1) running synchronously after each node's insertion (as part of its insertion steps), while pushing all script-executing (or DOM tree-modifying or frame tree-modifying etc.) side effects to the new set of post-connection steps, which run synchronously during insertion, but _after all_ nodes finish their insertion. This essentially makes insertions "atomic" from the perspective of script, since script will not run until a given batch of DOM insertions are complete. This two-stage approach aligns the spec with a model most similar to Blink & Gecko's implementation, and fixes whatwg#808. This PR also helps out with whatwg/html#1127 and whatwg#575 (per whatwg#732 (comment)). To accomplish, this we audited all insertion side effects on the web platform in https://docs.google.com/document/d/1Fu_pgSBziVIBG4MLjorpfkLTpPD6-XI3dTVrx4CZoqY/edit#heading=h.q06t2gg4vpw, and catalogued whether they have script-observable side-effects, whether they invoke script, whether we have tests for them, and how each implementation handles them. This gave us a list of present "insertion steps" that should move to the "post-connection steps", because they invoke script and therefore prevent insertions from being "atomic". This PR is powerless without counterpart changes to HTML, which will actually _use_ the post-connection steps for all current insertion steps that invoke script or modify the frame tree. The follow-up HTML work is tracked here: - whatwg/html#10188 - whatwg/html#10241
What is the issue with the HTML Standard?
The processing of
http-equiv
meta tags is currently done in the insertion steps (see this usage and this usage), meaning it happens synchronously upon insertion, before the post-insertion steps (which all browsers seem to have some form of) run.But implementations don't seem to do this. See this test: https://wpt.fyi/results/dom/nodes/insertion-removing-steps/Node-appendChild-script-and-default-style-meta-from-fragment.tentative.html?label=experimental&label=master&aligned. All browsers currently fail it (except for Edge, mysteriously), because Blink, WebKit, and Gecko (though I can't find the exact source code for this) all process
http-equiv
in the post-insertion steps.Once whatwg/dom#1261 lands (🤞), we should update the spec to use the post-insertion steps for this kind of meta tag, to match implementations. In the meantime, I will update the test expectations.
There's an open question about what we should do with
name
/content
meta elements. Both Chromium and WebKit also process these kinds of meta tags in the post-insertion steps, so I suppose we should update the following instances:... to use the post-insertion steps too, to at least match 2/3 browser implementations. See https://wpt.fyi/results/dom/nodes/insertion-removing-steps/Node-append-meta-referrer-and-script-from-fragment.tentative.html?label=experimental&label=master&aligned. Given this, Gecko would be the odd one out and we'd file a bug against Gecko to start processing these kinds of meta elements in the post-insertion steps.
If anyone disagrees with this analysis and my recommendations above, let me know! I'll prepare spec PRs once the post-insertion steps primitive is a thing.
The text was updated successfully, but these errors were encountered: