816487 - WebIDL codegen doesn't support Enum values contain characters outside [a-z_]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Masatoshi Kimura [:emk]]

Is this a spec restriction? Or a limitation of the implementation?

Comment 1

[Boris Zbarsky [:bzbarsky]]

Of the implementation.  And it's not the parser but the codegen that has this restriction, no?

The reason for that is that we have to produce C++ enum values from the WebIDL enum values.  The current way of doing that is very simple, and we just throw on input that would not work with that method, so that we'll fix it if we ever have such input.  See the getEnumValueName in Codegen.py.

What values are you running into this problem with?

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Dashes, I bet.

Comment 3

[Masatoshi Kimura [:emk]]

Slash and plus sign.
http://domparsing.spec.whatwg.org/#the-domparser-interface

Comment 4

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

So would it be a good idea to just convert all non [a-z_] characters to _ as far as the enumeration is concerned, and throw an error if there is a collision in the same enumeration. The chances of that should be rare.

Comment 5

[Masatoshi Kimura [:emk]]

Attachment: Allow all characters for WebIDL enum

Like this? (obviously uppercase characters should not be replaced with '_', I think.)

Comment 6

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Umm... I am not sure if all Unicode characters should be converted to '_', maybe for now just convert ASCII punctuation?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 686842 [details] [diff] [review]
Allow all characters for WebIDL enum

Unless we think that the entire pipeline here (Python, compilers, editors, etc) handles non-ASCII, I think we should throw on any non-ASCII char (basically [^ -~] with a comment) and convert everything that's not [A-Za-z_] to '_'.  In particular, I have no faith in the number if '_' produced for non-ASCII chars being the same across systems, so would prefer to catch the issue as early as possible....

Sorry for the lag in replying to the part about '/' and '+'; I somehow failed to cc myself.

Comment 8

[Masatoshi Kimura [:emk]]

Attachment: Allow all ASCII characters for WebIDL enum

(In reply to Boris Zbarsky (:bz) from comment #7)
> Unless we think that the entire pipeline here (Python, compilers, editors,
> etc) handles non-ASCII, I think we should throw on any non-ASCII char
> (basically [^ -~] with a comment)
Done.

> and convert everything that's not
> [A-Za-z_] to '_'.
Also preserve [0-9], and throw if the value starts with [0-9].

> In particular, I have no faith in the number if '_'
> produced for non-ASCII chars being the same across systems, so would prefer
> to catch the issue as early as possible....
Throw for consecutive underscores. It's reserved by the C++ spec.

Comment 9

[Masatoshi Kimura [:emk]]

https://tbpl.mozilla.org/?tree=Try&rev=08e56c8150cb

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 687015 [details] [diff] [review]
Allow all ASCII characters for WebIDL enum

r=me.  Thanks!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4db5da82a09c

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4db5da82a09c