Skip to content
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

Finish a todo of setting browserSignal's data-version field. #657

Merged
merged 3 commits into from
Jun 22, 2023

Conversation

qingxinwu
Copy link
Collaborator

@qingxinwu qingxinwu commented Jun 22, 2023

Also provides "list" as a parameter to get structured head list as it
needs.

Also changed header's "true" to "?1" since the latter is the correct
true value for headers.


Preview | Diff

Qingxin Wu added 3 commits June 21, 2023 16:06
Also provides "list" as a parameter to get structured head list as it
needs.

Also changed header's "true" to "?1" since the latter is the correct
true value for headers.
@qingxinwu qingxinwu requested a review from miketaylr June 22, 2023 15:31
@qingxinwu
Copy link
Collaborator Author

@miketaylr Mind taking a look at the small change? Thanks.

@qingxinwu qingxinwu added the spec Relates to the spec label Jun 22, 2023
Copy link
Collaborator

@miketaylr miketaylr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@miketaylr miketaylr merged commit 77df320 into WICG:main Jun 22, 2023
github-actions bot added a commit that referenced this pull request Jun 22, 2023
SHA: 77df320
Reason: push, by miketaylr

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@dmdabbs
Copy link
Contributor

dmdabbs commented Jun 22, 2023

    1. Set |dataVersion| to the result of [=header list/getting a structured field value=]
      given [:Data-Version:] and "`item`" from |headers|.
    1. If |dataVersion| is not null:
      1. If |dataVersion| is not an integer, or is less than 0 or more than 4294967295, set
        |signals| to failure and return.
      1. TODO: Check whether version is consistent for all keys requested by this interest group.

Was the data version at one time (or will be) an attribute within the response? Now it is a header value, so in what case might it differ across keys? Is there a possibility an IG's keys could be split across different signalsURL fetches?

BTW,

<dfn>fetch trusted signals</dfn>

is referenced elsewhere as [=fetching trusted signals=].

@qingxinwu
Copy link
Collaborator Author

Good question. Sent a PR to remove the TODO.
It was there because the explainer (browserSignals part) mentioned this. The explainer mentioned that just as an open option for a possible optimization of using multiple fetches for a generateBid's trustedBiddingSignals, but it's not what's implemented, and there's no plan of implementing it. So in the spec, going to remove the reference to that.

About [=fetching trusted signals=], it's a feature of bikeshed. "Bikeshed can automatically handle a wide range of English conjugations and pluralizations"

@dmdabbs
Copy link
Contributor

dmdabbs commented Jun 22, 2023

About [=fetching trusted signals=], it's a feature of bikeshed. "Bikeshed can automatically handle a wide range of English conjugations and pluralizations"

谢谢 and 🤯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec Relates to the spec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants