[Webkit-unassigned] [Bug 106553] [V8] Add IDL Enum support WIP
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jan 24 22:11:14 PST 2013
https://bugs.webkit.org/show_bug.cgi?id=106553
--- Comment #11 from Kentaro Hara <haraken at chromium.org> 2013-01-24 22:13:09 PST ---
(From update of attachment 184415)
View in context: https://bugs.webkit.org/attachment.cgi?id=184415&action=review
Almost looks OK.
> LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-responseType.html:38
> +req.responseType = "ZdBrlAwOzlYbJII";
> +shouldBeEmptyString("req.responseType");
> +req.responseType = "U0q1V8kGQ";
> +shouldBeEmptyString("req.responseType");
> +req.responseType = "2T";
> +shouldBeEmptyString("req.responseType");
> +req.responseType = "XJaC9ON1mTIPIYEUNv";
> +shouldBeEmptyString("req.responseType");
> +req.responseType = "X02eXXO4J44r6Y";
> +shouldBeEmptyString("req.responseType");
Rather than testing various kind of strings, you can test various types ({foo: 1}, 123, 123.4, function(){}, document, null, undefined).
> Source/WebCore/bindings/scripts/IDLParser.pm:677
> + my $enumValueToken = $self->getToken();
> + $self->assertTokenType($enumValueToken, StringToken);
> + my $quotedValue = $enumValueToken->value();
> + my $unquotedValue;
> + ($unquotedValue) = $quotedValue =~ /^"([^"]*)"$/;
> + push(@values, $unquotedValue);
> + push(@values, @{$self->parseEnumValues()});
> + return \@values;
I understand some change like this is needed. Instead of using ad-hoc regular expressions, would you write it by strictly following the BNF of the Web IDL grammer? (http://www.w3.org/TR/WebIDL/#prod-Enum) Maybe you can use parseString() or something.
> Source/WebCore/bindings/scripts/IDLParser.pm:697
> + my $enumValueToken = $self->getToken();
> + $self->assertTokenType($enumValueToken, StringToken);
> + my $quotedValue = $enumValueToken->value();
> + my $unquotedValue;
> + ($unquotedValue) = $quotedValue =~ /^"([^"]*)"$/;
> + push(@values, $unquotedValue);
> + push(@values, @{$self->parseEnumValues()});
> + return \@values;
Ditto.
> Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:41
> +#include "JSTestEnumType.h"
For JSC, you can fix it in a follow-up patch.
> Source/WebCore/bindings/scripts/test/TestObj.idl:33
> +#if defined(TESTING_JS) || defined(TESTING_V8)
Remove this. Alternatively, add a logic to CodeGenerator{ObjC,GObject,CPP}.pm to skip 'enum'. For now it's OK to keep the macro. Please fix it in a follow-up patch.
> Source/WebCore/bindings/scripts/test/TestObj.idl:34
> +enum TestEnumType { "", "foo", "bar", " foo, bar, or zork", "ãã²" };
Remove multi-byte characters. Here we are not intending to test various strings. Rather, readability is important. So the test case could be:
enum TestEnumType { "Enum1", "Enum2", "Enum3" };
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list