Bug 24846 - HTMLMediaElement should implement 'autobuffer' attribute
Summary: HTMLMediaElement should implement 'autobuffer' attribute
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-26 12:55 PDT by Eric Carlson
Modified: 2009-03-31 14:33 PDT (History)
0 users

See Also:


Attachments
proposed patch (8.08 KB, patch)
2009-03-28 11:19 PDT, Eric Carlson
darin: review-
Details | Formatted Diff | Diff
revised patch (10.03 KB, patch)
2009-03-30 10:35 PDT, Eric Carlson
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2009-03-26 12:55:14 PDT
HTMLMediaElement still doesn't implement the 'autobuffer' attribute, http://www.whatwg.org/specs/web-apps/current-work/multipage/video.html#attr-media-autobuffer.
Comment 1 Eric Carlson 2009-03-28 11:19:46 PDT
Created attachment 29038 [details]
proposed patch
Comment 2 Darin Adler 2009-03-28 17:36:28 PDT
Comment on attachment 29038 [details]
proposed patch

> +    setBooleanAttribute(autobufferAttr, b);
> +    if (m_player)
> +        m_player->setAutobuffer(b);

This is not the right way to bind an attribute to a back-end player property. I haven't looked at the other attributes to see if you're doing it that way. The correct place to do the mapping is in the parseMappedAttribute function. The setter should only set the attribute and the back end effect should be triggered by parseMappedAttribute.

Otherwise, setting the attribute via setAttribute or by setting the attribute in the initial document won't work.

> +void MediaPlayer::setAutobuffer(bool b)
> +{
> +    m_autobuffer = b;
> +    m_private->setAutobuffer(b);
> +}

Since we already know the old value here, I suggest we call the private function only if the value is changing. That way if the private version has to do some real work it won't get done if there's no change.

review- because of the first comment above
Comment 3 Eric Carlson 2009-03-30 10:35:25 PDT
Created attachment 29077 [details]
revised patch

Revised per review.
Comment 4 Darin Adler 2009-03-31 08:36:47 PDT
Comment on attachment 29077 [details]
revised patch

r=me
Comment 5 Eric Carlson 2009-03-31 14:33:02 PDT
Committed revision 42141.