[webkit-reviews] review granted: [Bug 105482] Implement :past pseudo class for the WebVTT ::cue pseudo element : [Attachment 184101] Proposed fix 0.1

[bugzilla-daemon at webkit.org]

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

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

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


r+ but I would prefer to have a bit more logging to help someone looking at the
test results figure out what is happening.

> LayoutTests/media/track/track-css-matching-timestamps-expected.txt:32
> +RUN(video.currentTime = 0.7)
> +EVENT(seeked)
> +EXPECTED (getComputedStyle(cueNode).color == 'rgb(0, 255, 0)') OK
> +EXPECTED (getComputedStyle(cueNode).color == 'rgb(0, 255, 0)') OK
> +EXPECTED (getComputedStyle(cueNode).color == 'rgb(0, 255, 0)') OK
> +EXPECTED (getComputedStyle(cueNode).color == 'rgb(128, 128, 128)') OK
> +EXPECTED (getComputedStyle(cueNode).color == 'rgb(0, 255, 0)') OK

I thought these results were wrong at first, and really had to scratch my head
before I understood the tree structure created by the <c>s in the cue. I think
it would be worth also logging the text of each cue node to make it more
obvious what is going on.