-
Notifications
You must be signed in to change notification settings - Fork 228
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
Specify support for cross-origin trusted signals #1197
Conversation
and the bundled queryFeatureSupport('*'). This also includes fixes to some bugs in parsing of these URLs (some of the checks were missing).
just a heads up, I may not have time to review it this week, but will find some time early next week |
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.
went through most of the PR. Will take a closer look at some places later.
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.
...Doing the easy stuff first.
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.
almost there
spec.bs
Outdated
@@ -4591,6 +4572,33 @@ The <dfn for="interest group">estimated size</dfn> of an [=interest group=] |ig| | |||
1. Return the [=URL serializer|serialization=] of |url|. | |||
</div> | |||
|
|||
<div algorithm> | |||
To <dfn>parse and verify a bidding code or update URL</dfn> given a [=string=] |input| and a |
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.
To <dfn>parse and verify a bidding code or update URL</dfn> given a [=string=] |input| and a | |
To <dfn>parse and verify a bidding script or update URL</dfn> given a [=string=] |input| and an |
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's using "code" since it includes WASM and not just javascript
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 see, it's not obvious what "code" means, but don't have a better suggestion.
there was another comment about the same line.
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 a lot for working through the reviews here. I'm happy with this now that morlovich#3 and morlovich#4 are pretty much in place and ready to fast-follow this work here.
Please make sure to reference those in any relevant shipping documents (i.e., Blink launch process artifacts) as fast-follow work that will close some of the gaps that this PR introduces, so relevant approvers (i.e., API OWNERs) can get a full sense of what is going on and what is left. But from a spec perspective, I am supportive of landing this now that those follow-ups are nearly in place!
SHA: 585f660 Reason: push, by michaelkleber Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
SHA: 585f660 Reason: push, by morlovich Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
and the bundled queryFeatureSupport('*'). This also includes fixes to some bugs in parsing of these URLs (some of the checks were missing).
See #813 for more context.
Preview | Diff