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

[Impl nit] IDL string fields related to size declarations should be DOMString rather than USVString #1250

Open
gtanzer opened this issue Aug 14, 2024 · 6 comments

Comments

@gtanzer
Copy link
Contributor

gtanzer commented Aug 14, 2024

The following string fields related to size declarations in the joinAdInterestGroup API surface should be DOMStrings rather than USVStrings, because there is no specific reason for them to be USVStrings (they will not appear in a UI).

sizeGroup in AuctionAd
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/ad_auction/auction_ad.idl

width and height in AuctionAdInterestGroupSize
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/ad_auction/auction_ad_interest_group.idl;l=15

@gtanzer gtanzer changed the title IDL string fields related to size declarations should be DOMString rather than USVString [Impl bug] IDL string fields related to size declarations should be DOMString rather than USVString Aug 14, 2024
@gtanzer gtanzer changed the title [Impl bug] IDL string fields related to size declarations should be DOMString rather than USVString [Impl nit] IDL string fields related to size declarations should be DOMString rather than USVString Aug 14, 2024
@morlovich
Copy link
Collaborator

I believe that would be a mistake. Note that we have no good way of sending DOMString fields to devtools, and we would like to send accurate information to devtools about interest groups people join.

@gtanzer
Copy link
Contributor Author

gtanzer commented Aug 14, 2024

I believe that would be a mistake. Note that we have no good way of sending DOMString fields to devtools, and we would like to send accurate information to devtools about interest groups people join.

@domfarolino @JensenPaul

@domfarolino
Copy link
Collaborator

domfarolino commented Aug 14, 2024

Hmm, I have to imagine there are plenty of APIs and web platform features that allow DevTools introspection, but that use DOMStrings under the hood. What do those APIs do? Or can you point to the mechanism that only allows USVStrings to be passed to developer tools?

@morlovich
Copy link
Collaborator

We are sending a JSON object to devtools to represent complex stuff like this. Which means if it's got invalid utf-8, we check-fail.

@MattMenke2
Copy link
Contributor

Also worth noting that interest groups can be updated by a URL, that's specced to contain a JSON response body. Seems like it would be a bit awkward if you could set invalid utf-8 values via JS, but not via the update API.

@domfarolino
Copy link
Collaborator

Regarding this issue, the last I remember was discussing this with @JensenPaul and resolving to ask around in the Chromium community about the base::Value implementation's requirements for USVString-like semantics, and if there is a way or need for us to get around that. Has discussion of that kind taken place yet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants