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

stringifier as CSSOMString attribute is not allowed #254

Open
cdoublev opened this issue Dec 29, 2021 · 3 comments
Open

stringifier as CSSOMString attribute is not allowed #254

cdoublev opened this issue Dec 29, 2021 · 3 comments

Comments

@cdoublev
Copy link

When generating a wrapper class for CSS MediaList, I get the following error:

/home/vagrant/npm/css/node_modules/webidl2js/lib/constructs/interface.js:341
            throw new Error(`${msg}attribute can only be of type DOMString or USVString`);
                  ^

Error: Invalid stringifier "mediaText" on interface MediaList: attribute can only be of type DOMString or USVString
    at Interface._analyzeMembers (/home/vagrant/npm/css/node_modules/webidl2js/lib/constructs/interface.js:341:19)
    at Interface.toString (/home/vagrant/npm/css/node_modules/webidl2js/lib/constructs/interface.js:1718:12)
    at Transformer._writeFiles (/home/vagrant/npm/css/node_modules/webidl2js/lib/transformer.js:221:24)
    at async Transformer.generate (/home/vagrant/npm/css/node_modules/webidl2js/lib/transformer.js:286:5)
  ...
  stringifier attribute [LegacyNullToEmptyString] CSSOMString mediaText;
  ...

I'm not sure why CSSOMString attribute is not accepted or if it should be accepted, but obviously when appending ... && member.idlType.idlType !== "CSSOMString) ...", it works.

@domenic
Copy link
Member

domenic commented Dec 29, 2021

So, CSSOMString is basically a weird hack around the fact that it's hard for Rust code to implement DOMString APIs, and so the CSSWG decided to sacrifice edge-case interoperability (i.e. what happens to unpaired surrogates) in favor of letting Servo/Gecko write less code in their implementation. As such it's defined as either typedef USVString CSSOMString; or typedef DOMString CSSOMString;.

So the first question is, do your input IDL files properly include one of those typedefs? (I would suggest typedef DOMString CSSOMString;.) There's a chance that including that typedef will make this error go away.

But, there's also a chance that it won't. If so, this is probably ultimately an issue with how the Web IDL spec is not very good about "resolving" typedefs before it uses them. The stringifier spec says "The stringifier keyword must not be placed on an attribute unless it is declared to be of type DOMString or USVString" and it's not really defined whether "declared as" is post-typedef resolution or pre-typedef resolution. (Certainly the type that appears in the syntactic declaration is pre-typedef resolution...) A similar issue is, e.g., whatwg/webidl#649 about how it's not clear whether the "integer types" predicate applies through typedefs.

So anyway, in terms of a fix:

  • Maybe just adding the typedef to your input will work? If so that'd be ideal.
  • If not, then the best fix would be resolving typedefs before doing this comparison.
  • If that's too hard, we could do a temporary fix by just extending the predicate to include CSSOMString, like you say.

You could also just manually replace CSSOMString with DOMString in your input IDL files as a temporary workaround.

@cdoublev
Copy link
Author

The typedef DOMString CSSOMString; was missing in my IDL file, sorry about that. I added it (before the MediaList interface, of course) but the same error is produced, sadly, yes.

Thank you for the clarifications. I was somewhat familiar with the reasons why CSSOMString exists, but it was confusing to me because it also seemed to me that surrogates were replaced when parsing most (if not all) input values handled by the CSS API.

Filtered surrogates from the input stream, per WG resolution. Now the entire specification operates only on scalar values.

Replace any U+0000 NULL or surrogate code points in input with U+FFFD REPLACEMENT CHARACTER (�).

Both quotes are from CSS Syntax. MediaList is a list of CSS media queries that go through the above procedure when parsed. So I wonder why DOMString would have to be preferred instead of USVString (as a temporary workaround)?

@domenic
Copy link
Member

domenic commented Dec 29, 2021

Hmm, interesting. It does seem like a lot of the properties or method arguments that use CSSOMString eventually do call normalize into a token stream. So it might be the case that, at least for most APIs, using USVString at the IDL layer would allow you to skip the normalization step later.

On the other hand, it might be kind of messy since other entry points (like, saying, parsing the contents of <style>, not using any webidl2js-wrapped APIs) also need to normalize. So you couldn't always skip it. If you wanted the perf win of minimizing normalization, I think you'd need to track whether you'd normalized or not (i.e. whether the input to the parse algorithm is a USVString or DOMString). And that seems a bit annoying.

But you raise a good point that, apparently, DOMString and USVString have the same web-observable behavior for most Web IDL-generated CSS APIs, since the input usually goes through a parser. I'd never realized that before.

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

2 participants