[Webkit-unassigned] [Bug 53556] Adding Feature Defines for <track>

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=53556





--- Comment #5 from Eric Carlson <eric.carlson at apple.com>  2011-03-31 23:28:43 PST ---
(From update of attachment 87825)
View in context: https://bugs.webkit.org/attachment.cgi?id=87825&action=review

> Source/WebCore/html/HTMLMediaElement.cpp:739
> +#if ENABLE(TRACK)
> +void HTMLMediaElement::loadMediaTracks()
> +{
> +    for (Node* node = firstChild(); node; node = node->nextSibling()) {
> +        if (node->hasTagName(trackTag)) {
> +            HTMLTrackElement* track = static_cast<HTMLTrackElement*>(node);
> +            // TODO(annacc): store in an ExclusiveTrackList (4.8.10.10.1)
> +            track->load();
> +        }
> +    }
> +}
> +#endif
> +

I would leave the HTMLMediaElement changes out of this patch. loadMediaTracks doesn't do anything yet, and it isn't clear who will call it or when, so it isn't possible to evaluate the design or code.


> Source/WebCore/html/HTMLTrackElement.cpp:79
> +void HTMLTrackElement::setSrc(const String& url)
> +{
> +    setAttribute(srcAttr, url);
> +}
> +

Isn't a src accessor method also necessary?


> Source/WebCore/html/HTMLTrackElement.cpp:128
> +void HTMLTrackElement::scheduleErrorEvent()
> +{
> +    LOG(Media, "HTMLTrackElement::scheduleErrorEvent - %p", this);
> +    if (m_errorEventTimer.isActive())
> +        return;
> +
> +    m_errorEventTimer.startOneShot(0);
> +}
> +

Who will call scheduleErrorEvent and why? It probably makes sense to only include the track methods that deal with IDL attributes in this patch because without more context it isn't really possible to evaluate this part.


> Source/WebCore/html/HTMLTrackElement.cpp:148
> +    // TODO(annacc): create a LoadableTextTrack and kick of loading

Type "of" -> "off"

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.