[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