[Webkit-unassigned] [Bug 58346] Convert media controls hooks to a client interface
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Apr 18 11:22:05 PDT 2011
https://bugs.webkit.org/show_bug.cgi?id=58346
--- Comment #16 from Steve Lacey <sjl at chromium.org> 2011-04-18 11:22:05 PST ---
(In reply to comment #12)
> (From update of attachment 89917 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=89917&action=review
>
> > Source/WebCore/WebCore.xcodeproj/project.pbxproj:22965
> > + 1F0D6E0E13590DE700236592 /* MediaControlBase.h in Headers */,
>
> The XCode changes look like they haven't been sorted. Can you run sort-XCode-project-files on this?
Done. Though this resulted in a lot of changes! Am presuming this hasn't been run for a while and that the script did the right thing...
>
> > Source/WebCore/html/shadow/MediaControlBase.h:34
> > +#include <HTMLDivElement.h>
> > +#include <HTMLMediaElement.h>
> > +#include <HTMLNames.h>
>
> "", not <>
Done.
>
> > Source/WebCore/html/shadow/MediaControlBase.h:38
> > +class MediaControlBase : public HTMLDivElement {
>
> I am sorry about flip-flopping here, but looking at callsites, it's clear that they are working with an interface, not a base class. So we should call this new thing MediaControls, not MediaControlBase.
Done.
>
> > Source/WebCore/html/shadow/MediaControlBase.h:42
> > + // This function is to be implemented in your platform-specific media
>
> port-specific.
Done.
>
> > Source/WebCore/html/shadow/MediaControlBase.h:73
> > + MediaControlBase(HTMLMediaElement* mediaElement)
> > + : HTMLDivElement(HTMLNames::divTag, mediaElement->document()) { }
>
> I know you want to keep this change to only have a header file, but you could save a couple of header includes if you just have a source file with it.
Done.
>
> > Source/WebCore/html/shadow/MediaControlBase.h:81
> > +#endif
>
> extra break after first endif would be helpful.
Done. Was just following the style in other includes...
>
> > Source/WebCore/html/shadow/MediaControlRootElement.h:28
> > +#ifndef MediaControlRootElement_h
> > +#define MediaControlRootElement_h
>
> Ha! Thanks for noticing this one.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list