Explainer change for separate rate-limits for embedded site #1457
Conversation
|
@jolynyao Could you PTAL, thanks! |
linnan-github
force-pushed
the
embeddedBudget
branch
3 times, most recently
from
cbc7216
to
5144c38
Compare
linnan-github
force-pushed
the
embeddedBudget
branch
from
5144c38
to
f6c6a38
Compare
|
LGTM thanks @linnan-github! |
| @@ -980,6 +980,10 @@ were reported. | |||
| In order to achieve the privacy goals listed above the API has various rate limits, which can be found [here](https://github.com/WICG/attribution-reporting-api/blob/main/params/chromium-params.md) for Chromium. | |||
| Note: Browsers may choose to allow separate rate limits for the embedded site | |||
There was a problem hiding this comment.
Is embedded site standard terminology? If so, let's link to a definition of it. Otherwise, let's explain further. Same for the spec changes.
There was a problem hiding this comment.
Also, IIUC the spec changes aren't limited to rate limits. Rather, they effectively replace the context origin in all uses across the entire spec.
For sources, this includes "max pending sources per source origin" and "max destinations covered by unexpired sources", which aren't explicitly considered "rate limits."
It also includes the source site in verbose debug report bodies.
For aggregatable reports, it affects the effective attribution destination and therefore "max aggregatable attribution reports per attribution destination".
IMO we should make it explicit that the context origin is entirely replaced, not just for rate-limit purposes.
There was a problem hiding this comment.
I used “rate limits” to cover those limits as well. The destination limit is also discussed in this section, so I think it’s fine. I can mention storage limit explicitly if you feel strongly.
There was a problem hiding this comment.
The aforementioned limits aren't rate limits, as they do not expire after a specific window, and they are mentioned in different parts of the spec and explainer.
| @@ -1832,6 +1832,8 @@ To <dfn>check if cookie-based debugging is allowed</dfn> given a | |||
| To obtain the <dfn export for=node>context origin</dfn> of a [=node=] |node|, return |node|'s [=node navigable=]'s | |||
| [=navigable/top-level traversable=]'s [=navigable/active document=]'s [=origin=]. | |||
|
|
|||
| Note: The user agent MAY return the embedded origin for specific use cases. | |||
There was a problem hiding this comment.
I don't think this belongs in a note. We can instead phrase it as:
1. Optionally, return the embedded origin if it exists.
2. Return |node|'s ...
And then explain the reasoning or what an embedded origin is in the note.
| @@ -980,6 +980,10 @@ were reported. | |||
| In order to achieve the privacy goals listed above the API has various rate limits, which can be found [here](https://github.com/WICG/attribution-reporting-api/blob/main/params/chromium-params.md) for Chromium. | |||
| Note: Browsers may choose to allow separate rate limits for the embedded site | |||
There was a problem hiding this comment.
Is there a reason the explainer uses embedded site but the spec uses embedded origin?
There was a problem hiding this comment.
That’s just because the explainer mentions “top-level site” in that section, whereas the spec change is where we get the origin.
Fixes #1452
Preview | Diff