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

Remove vestigial reference to CollectedClientData/clientExtensions #835

Merged

Conversation

selfissued
Copy link
Contributor

@selfissued selfissued commented Mar 12, 2018

Fixes #828


Preview | Diff

@selfissued selfissued added this to the Last Working Draft milestone Mar 12, 2018
@selfissued selfissued self-assigned this Mar 12, 2018
@selfissued selfissued changed the title Remove vestigal reference to CollectedClientData/clientExtensions Remove vestigial reference to CollectedClientData/clientExtensions Mar 12, 2018
Copy link
Member

@emlun emlun left a comment

Choose a reason for hiding this comment

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

The text now reads:

The client extension input for the extension is used an input to this client processing. For each supported client extension, the client adds an entry to this dictionary with the extension identifier as the key, and the extension’s client extension input as the value.

However, "this dictionary" is now undefined. Actually, the whole second sentence is now outdated because the client no longer records the client extension inputs in the response. Perhaps this instead needs to say that the RP adds the input values to the extensions parameter?

By the way, there's also an unrelated pre-existing typo in the first sentence: "is used an input"

Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

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

agree w/@emlun

@selfissued selfissued modified the milestones: Last Working Draft, CR Mar 14, 2018
Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Mike!

Copy link
Member

@emlun emlun left a comment

Choose a reason for hiding this comment

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

👍

@rlin1
Copy link
Contributor

rlin1 commented Mar 14, 2018

Note to emlun: in the respective processing rules we say "Let clientExtensions be a new map and let authenticatorExtensions be a new map." etc.
So we are referring to that clientExtensions map (i.e. some "local" variable defined in the processing sections).

@selfissued selfissued merged commit f0a495b into w3c:master Mar 14, 2018
kpaulh pushed a commit to kpaulh/webauthn that referenced this pull request Apr 6, 2018
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.

4 participants