ReflectOnly may be represented incorrectly in IDL. |
||||
Issue descriptionThe problem: There are 11 IDL files in Chromium that are formatted incorrectly according to the Blink IDL Extended Attributes spec. The ReflectOnly section (https://chromium.googlesource.com/chromium/src/+/50.0.2661.2/third_party/WebKit/Source/bindings/idl-extended-attributes.md#reflectonly_list_a) shows that ReflectOnly with multiple values are supposed to be formatted like this: ReflectOnly="first"|"second"|"third"|"fourth" The 11 files in question have IDL that looks like this: ReflectOnly=("async", "sync", "auto") It may be that this is correct formatting, but it is not documented. Regardless, both formats throw this error in webidl2.parse(): "Got an error during or right after parsing `interface SVGImageElement`: Expected identifiers but none found, line 36 (tokens: "\"async\", \"sync\", \"auto\"")" I'll be happy to open a second ticket for webidl2, if that's what's called for. The 11 files are these: blink/renderer/core/html/html_element.idl blink/renderer/core/html/html_link_element.idl blink/renderer/core/html/html_table_cell_element.idl blink/renderer/core/html/html_anchor_element.idl blink/renderer/core/html/forms/html_form_element.idl blink/renderer/core/html/html_area_element.idl blink/renderer/core/html/html_script_element.idl blink/renderer/core/html/html_iframe_element.idl blink/renderer/core/html/html_image_element.idl blink/renderer/core/html/media/html_media_element.idl blink/renderer/core/dom/element.idl
,
Jan 10
,
Jan 11
ReflectOnly is a Blink specific extended attribute, and has no standard format. In addition, we don't accept A="a"|"b" format in our idl_parser.py, and none of extended attributes' formats that WebIDL spec defines[1] match it. So IDLExtendedAttributes.md should be updated. [1] https://heycam.github.io/webidl/#idl-extended-attributes I don't know much about webidl2, but as far as I read its README, it should parse A=("a","b") format successfully. Maybe tokens are not split?
,
Jan 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1820e3d3cfb2e8cb9eea5366995d35f7b7d93721 commit 1820e3d3cfb2e8cb9eea5366995d35f7b7d93721 Author: Hitoshi Yoshida <peria@chromium.org> Date: Fri Jan 11 02:36:08 2019 IDL document: Fix wrong descriptions around [Reflect*] This CL updates the document to correct followings; - Examples of [ReflectOnly] are written in a wrong format ReflectOnly="A"|"B"|"C" while WebIDL and our IDL parser allow ReflectOnly=("A","B","C") - Headings for [Reflect*] have right hand side, like [ReflectMissing="value"], while others don't. This CL drops their RHS. Bug: 920009 Change-Id: I53c4a06659df2e554bfe11a36c8621916e45c430 Reviewed-on: https://chromium-review.googlesource.com/c/1405155 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> Commit-Queue: Hitoshi Yoshida <peria@chromium.org> Cr-Commit-Position: refs/heads/master@{#621875} [modify] https://crrev.com/1820e3d3cfb2e8cb9eea5366995d35f7b7d93721/third_party/blink/renderer/bindings/IDLExtendedAttributes.md
,
Jan 11
Thanks for addressing this so quickly. Can this ticket re referred to webidl2 folks or do I need to open a second ticket on another system (like GitHub, for example)?
,
Jan 12
I recommend to file another issue for it. So let me close it as Fixed.
,
Jan 14
WebIDL2 issue is here, if anyone's interested. https://github.com/w3c/webidl2.js/issues/256 |
||||
►
Sign in to add a comment |
||||
Comment 1 by tkent@chromium.org
, Jan 9