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

Replace USVString with DOMString #2203

Open
zacknewman opened this issue Nov 14, 2024 · 3 comments · May be fixed by #2217
Open

Replace USVString with DOMString #2203

zacknewman opened this issue Nov 14, 2024 · 3 comments · May be fixed by #2217

Comments

@zacknewman
Copy link
Contributor

zacknewman commented Nov 14, 2024

It would appear that the same justification that was used for #2115 applies to the remaining areas where USVString is used:

As recommended by the Web IDL spec:

Specifications should only use USVString for APIs that perform text processing and need a string of scalar values to operate on. Most APIs that use strings should instead be using DOMString, which does not make any interpretations of the code units in the string. When in doubt, use DOMString.

Currently the only places where USVString is used are the following extensions:

Should these all be changed to DOMString?

@agl
Copy link
Contributor

agl commented Nov 20, 2024

These extensions do, however, process their inputs.

The prf extension needs to perform base64url decoding of the values, so we need a valid input string. The AppId extensions need to convert the input to UTF-8 and hash it. In both cases, the input needs to be valid strings and invalid UTF-16 would cause an error.

So perhaps these are correctly USVString?

@zacknewman
Copy link
Contributor Author

zacknewman commented Nov 20, 2024

That argument applies to many of the DOMStrings in the spec (i.e., many areas where you see DOMString have to be valid Unicode). Making these lone 3 USVStrings into DOMStrings is a much smaller change, makes the spec consistent, and does not make it more fragile since there are more requirements than just valid Unicode. base64url decoding is defined on arbitrary bytes. If the input is not valid UTF-8, then the decoding will either fail or decode into something invalid which is possible even when the input is valid UTF-8.

The prf extension is also inconsistent with how the base64URL encoding of a Credential ID is typed elsewhere. For example, RegistrationResponseJSON.id is a DOMString.

It would be nice to pick one approach and stick with it:

  • Pick the most strict type possible
  • Define all strings as DOMString per the recommendation

The first approach will cause a lot of DOMStrings to be changed to USVString—including some of which revert previous changes from USVString to DOMString—and I don't think it appreciably makes the spec that much more type safe since many things require additional properties anyway (i.e.,USVString won't make it infallible anyway).

The second approach is much easier, aligns with the recommendation, and aligns with previous PRs that changed USVStrings to DOMStrings.

@emlun
Copy link
Member

emlun commented Nov 27, 2024

2024-11-27 WG call: Those are fair arguments. @zacknewman would you like to open a PR for this?

@zacknewman zacknewman linked a pull request Nov 27, 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