[webkit-reviews] review granted: [Bug 104043] Implement general ::cue pseudo element for the <video> : [Attachment 177621] Proposed fix 0.2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 4 17:49:23 PST 2012


Eric Carlson <eric.carlson at apple.com> has granted Dima Gorbik
<dgorbik at apple.com>'s request for review:
Bug 104043: Implement general ::cue pseudo element for the <video>
https://bugs.webkit.org/show_bug.cgi?id=104043

Attachment 177621: Proposed fix 0.2
https://bugs.webkit.org/attachment.cgi?id=177621&action=review

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


> LayoutTests/media/track/track-css-all-cues.html:14
> +		<style>
> +
> +		video::cue { color:red }
> +
> +		</style>

It looks like you have a mixture of spaces and tabs in this file. Spaces only
please.

> Source/WebCore/ChangeLog:13
> +	   (WebCore):

This doesn't add anything and can be removed.

> Source/WebCore/ChangeLog:15
> +	   (WebCore::MediaControlTextTrackContainerElement::createSubtrees):
> +	   (WebCore::MediaControlTextTrackContainerElement::updateDisplay):

I prefer to see a comment about what changed in each method. Not everyone does
this, but I think it makes it easier for people looking for information about
changes later.

> Source/WebCore/ChangeLog:19
> +	   (WebCore::MediaControlRootElement::createTextTrackDisplay):

Ditto.


More information about the webkit-reviews mailing list