[webkit-reviews] review denied: [Bug 24588] Update media element implementation to current spec : [Attachment 28645] updated patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Mar 17 00:23:46 PDT 2009
Adele Peterson <adele at apple.com> has denied Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 24588: Update media element implementation to current spec
https://bugs.webkit.org/show_bug.cgi?id=24588
Attachment 28645: updated patch
https://bugs.webkit.org/attachment.cgi?id=28645&action=review
------- Additional Comments from Adele Peterson <adele at apple.com>
Looks pretty good. Minor comments below. I found a problem with this patch
and the load event on Windows. Eric's going to diagnose tomorrow, so I'll r-
this for now, but I expect this will be ready to check in fairly soon once that
issue is figured out.
> Index: WebCore/ChangeLog
typo "obsolute"
typo "ticke"
> Index: WebCore/html/HTMLMediaElement.cpp
> ===================================================================
I think it would be cleaner here to remove the commented out code, and stick it
in a bug. Then you can just reference the bug here.
> + // FIXME: don't schedule timeupdate or progress events unless there are
registered listeners
> +/*
> + if (eventType == eventNames().progressEvent &&
!document()->hasListenerType(Document::xxxxxx_LISTENER))
> + return;
> + if (eventType == eventNames().timeupdateEvent &&
!document()->hasListenerType(Document::xxxxxx_LISTENER))
> + return;
> +*/
> +
More information about the webkit-reviews
mailing list