Drop WebIDL enums

[christianliebel]

Closes #633

This change (choose one):

• Breaks existing normative behavior (please add label "breaking")
• Adds new normative requirements
• Adds new normative recommendations or optional items
• Makes only editorial changes (only changes informative sections, or
  changes normative sections without changing behavior)
• Is a "chore" (metadata, formatting, fixing warnings, etc).

Commit message:

Drop WebIDL enums, because they throw for invalid values.

---

Preview | Diff

[christianliebel]

@marcoscaceres @kenchris I’m not quite sure about:

• DOMString vs. USVString
• Is it okay to pass an enum/type definition as a set?
• What’s the default value for orientation?

[mgiuca]

Is Marcos on board with this? I know he has been meaning to move the spec away from WebIDL to a different schema language for some time, due to the noted issues with WebIDL.

I do think we should be moving to some kind of schema language that allows us to specify the structure of the file declaratively, and not back towards the algorithmic parsing that we used to have. This is essentially a partial revert of the work Ken and I did a couple of years ago (converting the spec into WebIDL), making it much more verbose and harder to read.

I like that you've provided a "processing an enumeration member" abstraction so that at least when we look at the algorithm for, say, processing "display", we can see that it's based on the DisplayModeType enumeration. But it's much easier to read when the WebIDL clearly specifies that the type of display is DisplayModeType, not DOMString. I would prefer to hold off on this and do an all-at-once transition to either a different schema language that parses the way we want, or (as I've suggested doing in the past), define slightly different parsing rules for WebIDL but keep the same representation.

Ultimately, a specification needs to be readable and understandable without having to follow through threads of program code, so I'm opposed to doing any work that reduces readability, even if it technically improves the guidance on how to handle edge cases. I think there are other ways to be unambiguous about how to handle edge cases.

• DOMString vs. USVString

Enum values and other "internal" (not user-facing) strings should be DOMString (since they will be ASCII-only anyway).

• Is it okay to pass an enum/type definition as a set?
• What’s the default value for orientation?

Good catch, there is none specified at the moment. My reading of the [SCREEN-ORIENTATION] spec says it should be "any".

[mgiuca]

TextDirectionType and DisplayModeType are referenced several times throughout the spec, and are no longer formally defined. Your new structure implies that these are now "sets of DOMStrings" (as opposed to WebIDL enum types), so I think they need to be formally declared as such.

[marcoscaceres]

Yeah, I'm not on board on this. My plan is to switch to infra types, but once we do the other more important stuff...

[christianliebel]

@mgiuca First of all, thanks for your feedback! I’ve seen your changes in #750 after I created this PR, are they related somehow?

Yeah, I'm not on board on this. My plan is to switch to infra types, but once we do the other more important stuff...

@marcoscaceres Alright, let's talk about this on Monday. 😇

[mgiuca]

@mgiuca First of all, thanks for your feedback! I’ve seen your changes in #750 after I created this PR, are they related somehow?

Yeah, #750 (thanks for finding this) is what I had in mind when I said "define slightly different parsing rules for WebIDL but keep the same representation". As I recall, that was rejected. @annevk said on whatwg/webidl#597 that we could use https://infra.spec.whatwg.org/#parse-json-into-infra-values, but I don't see how that addresses the error problem.

What I was trying to capture with that "[CatchTypeError]" annotation was a declarative way of defining the "limit" of errors. Because we don't just want to say "any manifest field that doesn't have the correct type should just be dropped", because for example if an ImageResource has a bad URL, the whole ImageResource should be dropped, not just the URL field. So we need to manually define on a case-by-case basis the "scope" where failure stops.

It looks silly in my patch that every single element has [CatchTypeError] on it. But there's a reason to this. For example, look at icons:

[CatchTypeError] sequence<[CatchTypeError] ImageResource> icons;

That says if any icon is invalid, the failure bubbles up to the whole ImageResource and we drop that item from the list. But also, if icons itself is the wrong type (say, an int), then we drop the icons object, rather than erroring out the whole manifest.

I still like this approach. I think it's better than scrapping the whole WebIDL for hand-written algorithms, or another schema language. We have WebIDL, a beautifully defined way to express a typed data structure that everyone working on the web platform can read and write. I'd rather embrace it than abandon it, simply because we don't have a good way of describing how errors should propagate.

[annevk]

See whatwg/infra#159 (comment) for the latest on a schema-language. I'm not entirely convinced it's needed as it seems pretty easy to reject/accept things in prose. Perhaps a list of use cases or scenarios would help there.

[marcoscaceres]

@annevk, the use case is to take fetch some JSON -> convert into a neutral set of types (e.g., parsed in C++, Rust, JS, whatever) -> process the data into some canonical data structure while performing error handling and assigning defaults/fallback values, then allow the browser to operate on it.

In my mind, Infra types are ideal for this because they meet the use case of being programming language neutral.

Just to reiterate the problem: this spec defines things using WebIDL, but no one actually sends the JSON through a WebIDL processor (so the spec doesn't match reality, so would be pointless to add more error handling or pretend this is WebIDL). Chrome processes the JSON using C++, while Gecko does it in JS.

[annevk]

@marcoscaceres well, https://infra.spec.whatwg.org/#convert-a-json-derived-javascript-value-to-an-infra-value addresses that. But I think @mgiuca wants something more and I was asking about that.

[mgiuca]

Hi @annevk . The Infra convert a JSON-derived JavaScript value to an Infra value is necessary but not sufficient, as it's dynamically typed. It will take any valid JSON and convert it into an Infra value, but as far as I can tell, has no schema and no way of checking that the input JSON corresponds to a particular "type signature" (i.e., well-defined data structure). I guess that's what you're trying to cover with whatwg/infra#159. In the interum, I have to check all that myself with prose.

I'm not entirely convinced it's needed as it seems pretty easy to reject/accept things in prose.

It's "easy" to do in small quantities. It makes it harder to read the overall spec, though, especially when you have a JSON structure as large and complex as that of the Manifest format. I'm really happy with being able to look at this WebIDL and tell at a glance what type is required for each of those fields. It would be much less readable if I had to navigate to the parsing algorithm for each field and then inspect the prose text within to find out what assertions are being made about the data in there, to infer what type is being expected for that field.

I'm not wedded to WebIDL. I just would like some declarative schema language as opposed to manual parsing code.

@marcoscaceres yes, Chromium and Firefox both parse the structure with code, but that isn't particularly readable either, and ideally those implementations would be refactored to parse them using a declarative structure. Either way, it should not stop the specification from being more readable than the code that implements it.

Perhaps a list of use cases or scenarios would help there.

The use case is simply the need to express the type requirements of the Web App Manifest JSON structure: https://www.w3.org/TR/appmanifest/#webappmanifest-dictionary

Currently it is expressed in WebIDL, which clearly communicates the type of everything, but doesn't communicate what should happen in the event of a type error. Ideally we would be able to fix that problem without making the text less readable and maintainable.

[marcoscaceres]

Closed via 32b497c