Implement load notifications.
Created attachment 112736 [details] Patch
Comment on attachment 112736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112736&action=review > LayoutTests/media/track/track-load-error-readyState.html:10 > + endTest(); These constant will change to CAPS (e.g. TextTrack.ERROR) after this patch lands: https://bugs.webkit.org/show_bug.cgi?id=70951
Comment on attachment 112736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112736&action=review > Source/WebCore/html/HTMLMediaElement.cpp:850 > +void HTMLMediaElement::loadNextTextTrack(HTMLTrackElement* track) > +{ > + track->load(ActiveDOMObject::scriptExecutionContext(), this); > +} Nit: It is probably worth including a FIXME here noting the further setup that will be required to make this work correctly. > Source/WebCore/html/HTMLTrackElement.cpp:64 > + if (parent && parent->isMediaElement()) > + static_cast<HTMLMediaElement*>(parentNode())->trackWasAdded(this); This callback will result in an immediate call to HTMLTrackElement::load. We probably shouldn't trigger loading synchronously from here, but a FIXME to investigate is probably fine for now. > LayoutTests/media/track/track-load-error-readyState.html:6 > +<script src=../media-file.js></script> > +<script src=../video-test.js></script> > +<script> > + > + function trackError() > + { Nit: would rather that we use fully formed HTML files for media tests, eg. include an HTML5 DOCTYPE, <html>, <head>, <body>, etc. Obviously fragments work just fine and there isn't an official WebKit convention, but I think the structure makes files easier to understand. > LayoutTests/media/track/track-load-from-element-readyState-expected.txt:1 > +Tests the load event on HTMLTrackElement for src="..." and LOADED readyState on TextTrack. Nit: this comment is confusing, I expected to it was literally "<track src='...'>". > LayoutTests/media/track/track-load-from-src-readyState-expected.txt:1 > +Tests the load event on HTMLTrackElement for src defined in JavaScript and LOADED readyState on TextTrack. Nit: the src attribute is *set* from JavaScript, not defined.
Comment on attachment 112736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112736&action=review Thanks Eric! >> Source/WebCore/html/HTMLMediaElement.cpp:850 >> +} > > Nit: It is probably worth including a FIXME here noting the further setup that will be required to make this work correctly. Done. >> Source/WebCore/html/HTMLTrackElement.cpp:64 >> + static_cast<HTMLMediaElement*>(parentNode())->trackWasAdded(this); > > This callback will result in an immediate call to HTMLTrackElement::load. We probably shouldn't trigger loading synchronously from here, but a FIXME to investigate is probably fine for now. Done. I added a FIXME in HTMLMediaElement::loadNextTextTrack() because text tracks should always be loaded asynchronously. >> LayoutTests/media/track/track-load-error-readyState.html:6 >> + { > > Nit: would rather that we use fully formed HTML files for media tests, eg. include an HTML5 DOCTYPE, <html>, <head>, <body>, etc. Obviously fragments work just fine and there isn't an official WebKit convention, but I think the structure makes files easier to understand. Done. >> LayoutTests/media/track/track-load-from-element-readyState-expected.txt:1 >> +Tests the load event on HTMLTrackElement for src="..." and LOADED readyState on TextTrack. > > Nit: this comment is confusing, I expected to it was literally "<track src='...'>". Done. >> LayoutTests/media/track/track-load-from-src-readyState-expected.txt:1 >> +Tests the load event on HTMLTrackElement for src defined in JavaScript and LOADED readyState on TextTrack. > > Nit: the src attribute is *set* from JavaScript, not defined. Done.
http://trac.webkit.org/changeset/98736
Created attachment 112979 [details] Patch for landing
Created attachment 112981 [details] updating to ToT Looks like this patch just needed an update to apply, but I'll need another r+. Thanks.
Created attachment 112982 [details] updating to ToT One more try, let's see if this will apply.
Created attachment 113047 [details] Patch for landing
Committed r98860: <http://trac.webkit.org/changeset/98860>