[spec] Make bikeshed check references to Private Aggregation API #219
Conversation
|
@alexmturner and @xyaoinum, PTAL! (Ignore the first commit, this PR is based on #216.) |
dmcardle
changed the title
dmcardle
force-pushed
the
fix-autolinks
branch
2 times, most recently
from
03555e8 to
32d26a5
Compare
dmcardle
changed the title
|
Rebased, this is now disentangled from #216, modulo the fact that it imports the |
| @@ -124,36 +147,6 @@ spec: permissions-policy; urlPrefix: https://www.w3.org/TR/permissions-policy/ | |||
| spec: attestation; urlPrefix: https://github.com/privacysandbox/attestation | |||
| type: dfn | |||
| text: enrolled | |||
| spec: private-aggregation-api; urlPrefix: https://patcg-individual-drafts.github.io/private-aggregation-api/ | |||
| type: dfn | |||
| text: Private Aggregation; url: | |||
There was a problem hiding this comment.
This little tidbit is what enabled us to link to the Private Aggregation spec with [=Private Aggregation=]. It's kind of a hack though -- it works by pretending there's a dfn with that name, then overriding the value of the URL hash as the empty string. Technically, it kind of got the job done, but I suppose it risks colliding with something that Private Aggregation actually wanted to export.
There was a problem hiding this comment.
Definitely a hack, but I don't think there's a real risk of collision here.
There was a problem hiding this comment.
Specifically, I was thinking that mixing and matching link-defaults and anchors might cause occurrences of [=Private Aggregation=] to incorrectly autolink to the permissions policy string, declared as <dfn>private-aggregation</dfn>, either now or in a future version of bikeshed.
dmcardle
force-pushed
the
fix-autolinks
branch
from
32d26a5 to
f88eeee
Compare
dmcardle
force-pushed
the
fix-autolinks
branch
from
f88eeee to
1c2e0ac
Compare
Currently, all references to the Private Aggregation spec are declared in the `anchors` block. It's not obvious from the name, but bikeshed interprets this block as "custom definitions"[1]. Consequently, it has not been checking our references to Private Aggregation, nor has it always been generating the correct links[2]. Since the Private Aggregation spec is now in the database[3], I think we can promote these to real references. This commit removes the Private Aggregation lines from the `anchors` block and makes the necessary adjustments to the `link-defaults` block. With this change, invalid references to the Private Aggregation spec become compilation errors. [1]: https://speced.github.io/bikeshed/#custom-dfns [2]: For instance, clicking "context ID" in step 10 of "obtain the pre-specified report parameters" links to a non-existent anchor on the Private Aggregation spec. [3]: https://github.com/tobie/specref/blob/3569d1ecfa298461f3de0586ed2d13ffb000fa29/refs/browser-specs.json#L1179
dmcardle
force-pushed
the
fix-autolinks
branch
from
1c2e0ac to
16a48be
Compare
DO_NOT_MERGE Until the parent PR has been merged (#216).Currently, all references to the private-aggregation-api spec are declared in the "anchors" block. According to the bikeshed docs, this is how you link to dfns in specs that aren't part in the autolinking database1. As a result, bikeshed hasn't actually been checking our references, nor has it always been generating the correct links2.
Private Aggregation spec is already in the database3, so I think we can promote these to actual references.
This commit makes the necessary adjustments to the "link-defaults" block. With this change, invalid references to the Private Aggregation spec are now compilation errors.
Preview | Diff
Footnotes
https://speced.github.io/bikeshed/#custom-dfns ↩
For instance, clicking "context ID" in step 10 of "obtain the pre-specified report parameters" links to a non-existent anchor on the Private Aggregation spec. ↩
https://github.com/tobie/specref/blob/3569d1ecfa298461f3de0586ed2d13ffb000fa29/refs/browser-specs.json#L1179 ↩