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

RESOLVED FIXED in mozilla20

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: emk, Assigned: emk)

Tracking

(Blocks: 1 bug)

unspecified
mozilla20
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Is this a spec restriction? Or a limitation of the implementation?
(Assignee)

Updated

5 years ago
Blocks: 580070
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?
Summary: WebIDL parser doesn't support Enum values contain characters outside [a-z_] → WebIDL codegen doesn't support Enum values contain characters outside [a-z_]
Dashes, I bet.
(Assignee)

Comment 3

5 years ago
Slash and plus sign.
http://domparsing.spec.whatwg.org/#the-domparser-interface
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.
(Assignee)

Comment 5

5 years ago
Created attachment 686842 [details] [diff] [review]
Allow all characters for WebIDL enum

Like this? (obviously uppercase characters should not be replaced with '_', I think.)
Attachment #686842 - Flags: review?(bzbarsky)
Umm... I am not sure if all Unicode characters should be converted to '_', maybe for now just convert ASCII punctuation?
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.
Attachment #686842 - Flags: review?(bzbarsky) → review-
(Assignee)

Comment 8

5 years ago
Created attachment 687015 [details] [diff] [review]
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.
Attachment #687015 - Flags: review?(bzbarsky)
(Assignee)

Updated

5 years ago
Attachment #686842 - Attachment is obsolete: true
(Assignee)

Comment 9

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=08e56c8150cb
Comment on attachment 687015 [details] [diff] [review]
Allow all ASCII characters for WebIDL enum

r=me.  Thanks!
Attachment #687015 - Flags: review?(bzbarsky) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/4db5da82a09c
Assignee: nobody → VYV03354
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4db5da82a09c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in before you can comment on or make changes to this bug.