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

Fix references to reserved tokenBinding member #2014

Merged
merged 1 commit into from
Feb 14, 2024
Merged

Conversation

emlun
Copy link
Member

@emlun emlun commented Jan 10, 2024

Commit 5f45d34 in PR #1661 changed the definition type of CollectedClientData/tokenBinding from "dict-member" to "dfn" to resolve Bikeshed errors about the member not existing in the CollectedClientData definition.

However, autolinks to the reserved tokenBinding member still use the {{foo}} syntax, which is only for IDL types, so the autolinks no longer resolve. This results in Bikeshed instead pulling these autolinks as external refs from the spec database instead of internal refs, which results in an entry being added to the "terms defined elsewhere" index: "[WEBAUTHN-3] defines the following terms: tokenBinding". The changed definition type also changes the generated link anchor from #dom-collectedclientdata-tokenbinding to #collectedclientdata-tokenbinding.

This fixes these issues by declaring the link anchor explicitly, for backwards compatibility, and by changing the autolinks to use the [=foo=] syntax for autolinks to "dfn"-type definitions.

This also fixes the following Bikeshed lint:

LINT: Unexported dfn that's not referenced locally - did you mean to export it?
<dfn data-dfn-type="dfn" data-dfn-for="CollectedClientData" id="collectedclientdata-tokenbinding" data-lt="tokenBinding" data-noexport="by-default">tokenBinding<a href="#collectedclientdata-tokenbinding" class="self-link"></a></dfn>
<dfn data-dfn-type="dfn" data-dfn-for="CollectedClientData" id="collectedclientdata-tokenbinding" data-lt="tokenBinding" data-noexport="by-default">tokenBinding<a href="#collectedclientdata-tokenbinding" class="self-link"></a></dfn>

Preview | Diff

Commit 5f45d34 in PR #1661 changed
the definition type of `CollectedClientData/tokenBinding` from
"dict-member" to "dfn" to resolve Bikeshed errors about the member not
existing in the `CollectedClientData` definition.

However, autolinks to the reserved `tokenBinding` member still use the
`{{foo}}` syntax, which is only for IDL types, so the autolinks no
longer resolve. This results in Bikeshed instead pulling these
autolinks as external refs from the spec database instead of internal
refs, which results in an entry being added to the "terms defined
elsewhere" index: "[WEBAUTHN-3] defines the following terms:
tokenBinding". The changed definition type also changes the generated
link anchor from "#dom-collectedclientdata-tokenbinding" to
"#collectedclientdata-tokenbinding".

This fixes these issues by declaring the link anchor explicitly, for
backwards compatibility, and by changing the autolinks to use the
`[=foo=]` syntax for autolinks to "dfn"-type definitions.

This also fixes the following Bikeshed lint:

```
LINT: Unexported dfn that's not referenced locally - did you mean to export it?
<dfn data-dfn-type="dfn" data-dfn-for="CollectedClientData" id="collectedclientdata-tokenbinding" data-lt="tokenBinding" data-noexport="by-default">tokenBinding<a href="#collectedclientdata-tokenbinding" class="self-link"></a></dfn>
<dfn data-dfn-type="dfn" data-dfn-for="CollectedClientData" id="collectedclientdata-tokenbinding" data-lt="tokenBinding" data-noexport="by-default">tokenBinding<a href="#collectedclientdata-tokenbinding" class="self-link"></a></dfn>
```
@emlun emlun self-assigned this Jan 10, 2024
@nadalin nadalin added this to the L3-WD-02 milestone Jan 10, 2024
@selfissued selfissued self-requested a review January 10, 2024 20:30
Copy link
Contributor

@ve7jtb ve7jtb left a comment

Choose a reason for hiding this comment

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

LGTM

@agl
Copy link
Contributor

agl commented Jan 31, 2024

Ping for @selfissued to review.

Copy link
Contributor

@selfissued selfissued left a comment

Choose a reason for hiding this comment

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

LGTM

@emlun emlun merged commit 99cbcfa into main Feb 14, 2024
2 checks passed
@emlun emlun deleted the tokenbinding-ref branch February 14, 2024 19:59
github-actions bot added a commit that referenced this pull request Feb 14, 2024
SHA: 99cbcfa
Reason: push, by emlun

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants