[Webkit-unassigned] [Bug 72171] <track>-related events cuechange, enter, and exit should be sorted and filtered before dispatching
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Feb 15 15:40:08 PST 2012
https://bugs.webkit.org/show_bug.cgi?id=72171
Eric Carlson <eric.carlson at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #126850| |review+
Flag| |
--- Comment #12 from Eric Carlson <eric.carlson at apple.com> 2012-02-15 15:40:08 PST ---
(From update of attachment 126850)
View in context: https://bugs.webkit.org/attachment.cgi?id=126850&action=review
Looks good with the minor suggestions noted, but I won't mark it cq+ because of the test failure.
> Source/WebCore/ChangeLog:848
> +>>>>>>> master
Is this left over from a bad merge?
> Source/WebCore/ChangeLog:9061
> +>>>>>>> master
Ditto.
> Source/WebCore/html/HTMLMediaElement.cpp:1139
> + static_cast<LoadableTextTrack*>(affectedTracks[i])->fireTrackElementCueChangeEvent();
I think it might be better if you left fireCueChangeEvent as a virtual method on the base class and ASSERT in the back class implementation instead. I don't have a really strong opinion about this, but please consider it.
> Source/WebCore/html/LoadableTextTrack.cpp:126
> +bool LoadableTextTrack::fireTrackElementCueChangeEvent()
Nit: I think the old name made more sense
> Source/WebCore/html/TextTrack.cpp:275
> + if (m_trackIndex == -1)
I would rather see a named const used here instead of "-1".
> Source/WebCore/html/TextTrack.cpp:283
> + m_trackIndex = -1;
Ditto.
> Source/WebCore/html/TextTrackCue.cpp:331
> + if (m_cueIndex == -1)
Ditto.
> Source/WebCore/html/TextTrackCue.cpp:339
> + m_cueIndex = -1;
Ditto.
> LayoutTests/ChangeLog:302
> +>>>>>>> master
??
> LayoutTests/ChangeLog:3899
> +>>>>>>> master
??
> LayoutTests/media/track/track-cues-sorted-before-dispatch.html:62
> + function logEndTest() {
This brace should go on the next line.
--
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