[webkit-reviews] review granted: [Bug 136550] Add TextTrack cueadd event : [Attachment 238605] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Brendan Long
<b.long at cablelabs.com>'s request for review:
Bug 136550: Add TextTrack cueadd event
https://bugs.webkit.org/show_bug.cgi?id=136550

Attachment 238605: Patch
https://bugs.webkit.org/attachment.cgi?id=238605&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=238605&action=review


> Source/WebCore/ChangeLog:13
> +	   * CMakeLists.txt:
> +	   * DerivedSources.cpp:
> +	   * bindings/js/JSDictionary.cpp:
> +	   (WebCore::JSDictionary::convertValue):

I know it is a pain but I really prefer to have per-method (or per-file)
comments in the ChangeLog. That makes it much easier for someone to understand
what changed when reading the ChangeLog later.

> Source/WebCore/html/track/CueEvent.cpp:39
> +

Nit: don't need this extra blank line.

> LayoutTests/media/track/text-track-oncueadd.html:11
> +	       var i = 0;

Nit: something like "cueNumber" would be clearer.