Bug 54908 - 'load' and 'error' events fired for @poster
Summary: 'load' and 'error' events fired for @poster
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-21 14:38 PST by Eric Carlson
Modified: 2011-02-25 12:21 PST (History)
1 user (show)

See Also:


Attachments
proposed patch (3.11 KB, patch)
2011-02-22 17:15 PST, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2011-02-21 14:38:02 PST
The test attached to https://bugs.webkit.org/show_bug.cgi?id=54685 relies on 'load' and 'error' events being fired when a poster loads or fails to load. As per the spec a <video> element should not fire any events for poster loading or failing, WebKit does so  as a side effect of using HTMLImageLoader to load the poster.
Comment 1 Eric Carlson 2011-02-22 17:15:16 PST
Created attachment 83418 [details]
proposed patch
Comment 2 Eric Carlson 2011-02-22 17:36:02 PST
<rdar://problem/9039962>
Comment 3 Eric Seidel (no email) 2011-02-24 03:01:23 PST
Comment on attachment 83418 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=83418&action=review

> Source/WebCore/html/HTMLImageLoader.cpp:54
> +    // HTMLVideoElement uses this class to load the poster image, but it should not fire events for loading or failure.
> +    if (element()->hasTagName(HTMLNames::videoTag))
> +        return;

Should it just use a subclass of this class?  dispatchLoadEvent can't be too hot, so making it virtual should be easy.
Comment 4 Eric Seidel (no email) 2011-02-24 03:01:52 PST
I'm not sure what the correct design here.  I'm not sure it makes sense to bake this knowledge into HTMLImageLoader, but I also don't think it's that bad either.
Comment 5 Eric Carlson 2011-02-25 12:21:27 PST
http://trac.webkit.org/changeset/79718