[webkit-reviews] review denied: [Bug 71123] Implement TextTrackList and the textTracks attribute of HTMLMediaElement : [Attachment 114278] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Nov 9 10:05:15 PST 2011
Sam Weinig <sam at webkit.org> has denied Eric Carlson <eric.carlson at apple.com>'s
request for review:
Bug 71123: Implement TextTrackList and the textTracks attribute of
HTMLMediaElement
https://bugs.webkit.org/show_bug.cgi?id=71123
Attachment 114278: Patch
https://bugs.webkit.org/attachment.cgi?id=114278&action=review
------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=114278&action=review
> Source/WebCore/html/HTMLMediaElement.cpp:2046
> +PassRefPtr<TextTrackList> HTMLMediaElement::textTracks() const
This probably wants to be a raw pointer return type.
> Source/WebCore/html/HTMLMediaElement.h:249
> + enum InvalidUrlAction { DoNothing, Complain };
Url should be spelled URL.
> Source/WebCore/html/HTMLTrackElement.cpp:157
> +PassRefPtr<TextTrack> HTMLTrackElement::track()
This should probably return a raw pointer.
> Source/WebCore/html/HTMLTrackElement.cpp:160
> + RefPtr<TextTrack> track = loadableTrack();
> + return track.release();
This can just be return loadableTrack();
> Source/WebCore/html/HTMLTrackElement.h:77
> + // TextTrackLoadingClient
I think this is just a TextTrackClient now.
> Source/WebCore/html/HTMLTrackElement.h:88
> + RefPtr<ScriptExecutionContext> m_context;
I am not sure you need to store this. The Document associate with the element
should be the ScriptExecutionContext to use.
> Source/WebCore/html/LoadableTextTrack.h:47
> + virtual void loadingCompleted(LoadableTextTrack*, bool /* loadingFailed
*/) { }
Maybe didCompleteLoad()?
> Source/WebCore/html/LoadableTextTrack.h:75
> + RefPtr<ScriptExecutionContext> m_context;
You should probably be able to get to this through the m_trackElement.
> Source/WebCore/html/track/TextTrackList.h:31
> +#include "ActiveDOMObject.h"
I don't think you use this anymore.
More information about the webkit-reviews
mailing list