WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
24846
HTMLMediaElement should implement 'autobuffer' attribute
https://bugs.webkit.org/show_bug.cgi?id=24846
Summary
HTMLMediaElement should implement 'autobuffer' attribute
Eric Carlson
Reported
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
.
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
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Eric Carlson
Comment 1
2009-03-28 11:19:46 PDT
Created
attachment 29038
[details]
proposed patch
Darin Adler
Comment 2
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
Eric Carlson
Comment 3
2009-03-30 10:35:25 PDT
Created
attachment 29077
[details]
revised patch Revised per review.
Darin Adler
Comment 4
2009-03-31 08:36:47 PDT
Comment on
attachment 29077
[details]
revised patch r=me
Eric Carlson
Comment 5
2009-03-31 14:33:02 PDT
Committed revision 42141.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug