[webkit-reviews] review denied: [Bug 62887] JavaScript access to text tracks from HTMLMediaElement : [Attachment 110220] updating with EnabledAtRuntime

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Anna Cavender <annacc at chromium.org>'s
request for review:
Bug 62887: JavaScript access to text tracks from HTMLMediaElement
https://bugs.webkit.org/show_bug.cgi?id=62887

Attachment 110220: updating with EnabledAtRuntime
https://bugs.webkit.org/attachment.cgi?id=110220&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=110220&action=review


> Source/WebCore/bindings/js/JSTextTrackCueCustom.cpp:39
> +EncodedJSValue JSC_HOST_CALL
JSTextTrackCueConstructor::constructJSTextTrackCue(ExecState* exec)
> +{

Do we really need a custom constructor?  We can create fancy constructors in
IDL now.

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:245
> -		    "unsigned short", "ushort");
> +		    "unsigned short", "uint");

Really?

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2874
>      return 1 if $type eq "IDBTransaction";
>      return 1 if $type eq "FileReader";
>      return 1 if $type eq "FileWriter";
> +    return 1 if $type eq "TextTrackCue";

We really shouldn't be adding more things to this list.  We should get this
information from the ActiveDOMObject IDL attribute.

> Source/WebCore/html/TextTrackCue.idl:39
> +	   Conditional=VIDEO_TRACK,
> +	   EnabledAtRuntime=webkitVideoTrack,
> +	   CanBeConstructed,
> +	   CustomConstructFunction,
> +	   ConstructorParameters=6,
> +	   V8CustomConstructor

This doesn't have the ActiveDOMObject attribute even though you hard coded it
into the code generator.  Are you sure this object needs to be an
activeDOMobject?