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

Make PublicKeyCredentialRequestOptions.rpId a DOMString #2066

Open
zacknewman opened this issue May 3, 2024 · 3 comments · May be fixed by #2074
Open

Make PublicKeyCredentialRequestOptions.rpId a DOMString #2066

zacknewman opened this issue May 3, 2024 · 3 comments · May be fixed by #2074
Assignees
Milestone

Comments

@zacknewman
Copy link

zacknewman commented May 3, 2024

Both PublicKeyCredentialRpEntity.id and PublicKeyCredentialRequestOptions.rpId represent the same thing (i.e., RP ID); however the former is modeled as a DOMString while the latter is modeled as a USVString. These should be the same type. Specifically PublicKeyCredentialRequestOptions.rpId should be a DOMString as well or PublicKeyCredentialRpEntity.id should be a USVString. Should be noted that PublicKeyCredentialRequestOptionsJSON.rpId is already a DOMString.

@nadalin nadalin added this to the L3-WD-02 milestone May 15, 2024
@emlun
Copy link
Member

emlun commented May 15, 2024

Thanks for pointing this out!

2024-05-15 WG call: Agreed we should change PublicKeyCredentialRpEntity.id to USVString. Strictly speaking this is a breaking change (changing a type bound in input (contravariant) position to be more restrictive), but in practice this shouldn't be able to break any applications since then those credentials wouldn't have worked in get() anyway.

@jschanck Seeing as Firefox is the browser that makes the most actual use of WebIDL - would changing PublicKeyCredentialRpEntity.id to USVString be a problem for Firefox?

@jschanck
Copy link

No, that wouldn't be a problem.

@zacknewman
Copy link
Author

PublicKeyCredentialRequestOptionsJSON.rpId should also be changed to a USVString too.

@emlun emlun linked a pull request May 20, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants