-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Update Speculation Rules data with specs and Webview support #27935
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
Conversation
|
Tip: Review these changes grouped by change (recommended for most PRs), or grouped by feature (for large PRs). |
…wser-compat-data into speculation-rules-updates
| "status": { | ||
| "experimental": true, | ||
| "standard_track": false, | ||
| "standard_track": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we point to a spec where inline-speculation-rules is explicitly mentioned? If not, let's set this back to false for now:
| "standard_track": true, | |
| "standard_track": false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So there's a PR open for that but not merged yet (hence why I didn't add the spec URL yet):
w3c/webappsec-csp#776
They were in the old prefetch spec (which has since mostly been moved to HTML except for this piece).
So it was standard track in WICG, but is now moving to the actual, non-WICG standards (so also standard track) but is kind of in an in between state for now, but don't think that makes it non-standard. WDYT?
Happy to update this with the spec url once it is merged to CSP spec btw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming that the PR will be merged soon, and knowing that you'll be following up, we can merge as is.
In the future, when we derive the standard_track automatically, a feature is not considered standard_track until it has landed in a spec (with good standing, as indicated by browser-specs).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. As i say it was in a WICG spec (which was included in browser-specs). But that spec has been closed off now as it’s being moved to the actual standards.
My read of “standards track” is it’s on track to become a standard (which this is) even if not quite a standard yet.
But I appreciate having a real spec is the only real evidence of that as a PR can be rejected.
Bad timing with this PR. I should have updated this setting when it was in the old spec really as always was on the standards track.
Anyway, thanks for the latitude!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Gotcha, we removed these spec_urls in #27755.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhhh and that’s what flipped this to non standard. I wondered as didn’t think this was marked as such in the past.
OK so all part of the fact we’re moving this!!
html/elements/script.json
Outdated
| "__compat": { | ||
| "description": "`expects_no_vary_search` key", | ||
| "mdn_url": "https://developer.mozilla.org/docs/Web/HTML/Reference/Elements/script/type/speculationrules#expects_no_vary_search", | ||
| "spec_url": "https://html.spec.whatwg.org/multipage/speculative-loading.html#valid-speculation-rule", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For all these keys pointing to https://html.spec.whatwg.org/multipage/speculative-loading.html#valid-speculation-rule, it would be preferable to add a text fragment, as this would allow us (in the future) to detect when a feature like this disappears from the spec.
But this is not a strict requirement for now, so we can merge as is.
/cc @Elchi3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess as long as it’s a fragment after the id I’m ok with that. Worse case it still links to the anchor o was linking to anyway. Let me add those back then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be preferable to add a text fragment
fwiw, I prefer real ids/anchors/definitions present in the specs. Text fragments are very unstable and I currently have no implementation for validating them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that I was actually suggesting text fragments combined with ID fragments:
For validation, you could just ignore the :~: part for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated now so think this is good to merge.
|
LGTM, thank you! |
|
Thanks @tunetheweb! |
Summary
Misc updates to the Speculation Rules BCD data, including:
Test results and supporting details
See these spec PRs:
Related issues