Bug 83858 – The cuechange event on track element is fired synchronously rather than queued

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Anna Cavender]

4.8.10.8 Playing the media resource: http://www.whatwg.org/specs/web-apps/current-work/#playing-the-media-resource "When the current playback position of a media element changes..." specifies how events are to be sorted and filtered before dispatch.

Currently, the cuechange event on track elements is fired synchronously rather than queued along with the other events in the queueing algorithm.

[Anna Cavender]

Created attachment 137037 [details]
Patch

[Eric Carlson]

Comment on attachment 137037 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137037&action=review

Nice fix, thanks!

> Source/WebCore/html/HTMLMediaElement.cpp:-1197
> -            affectedTracks[i]->fireCueChangeEvent();

It looks like fireCueChangeEvent will not be used after this change. If so, it should be removed.

> Source/WebCore/html/HTMLMediaElement.cpp:1197
> +        if (affectedTracks[i]->trackType() == TextTrack::TrackElement) {
> +            RefPtr<Event> event = Event::create(eventNames().cuechangeEvent, false, false);
> +            event->setTarget(static_cast<LoadableTextTrack*>(affectedTracks[i])->trackElement());

I think it would be safer to put the HTMLTrackElement in a local so you can ASSERT it value before setting the event target.

[Radar WebKit Bug Importer]

<rdar://problem/11246028>

[Anna Cavender]

Comment on attachment 137037 [details]
Patch

Thanks Eric!

View in context: https://bugs.webkit.org/attachment.cgi?id=137037&action=review

>> Source/WebCore/html/HTMLMediaElement.cpp:-1197
>> -            affectedTracks[i]->fireCueChangeEvent();
> 
> It looks like fireCueChangeEvent will not be used after this change. If so, it should be removed.

Good call.

>> Source/WebCore/html/HTMLMediaElement.cpp:1197
>> +            event->setTarget(static_cast<LoadableTextTrack*>(affectedTracks[i])->trackElement());
> 
> I think it would be safer to put the HTMLTrackElement in a local so you can ASSERT it value before setting the event target.

Another good call.  Thanks.

[Anna Cavender]

Created attachment 137139 [details]
assert track element and remove fireCueChangeEvent()

[Anna Cavender]

Committed as http://trac.webkit.org/changeset/114171

[Alexey Proskuryakov]

*** Bug 83878 has been marked as a duplicate of this bug. ***