Bug 124868 – Implement Render Media Controls for WinCairo

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Layout and Rendering

[Alex Christensen]

I updated and would like to add http://git.igalia.com/cgi-bin/gitweb.cgi?p=webkit.git;a=commitdiff;h=117428fca8a141af22ad017ebe976b5624d574e6

[Alex Christensen]

Created attachment 217840 [details]
Patch

[Eric Carlson]

Have you seen Source/WebCore/Modules/mediacontrols/? The Apple ports use this to implement media controls with JavaScript.

[Alex Christensen]

I haven't, but it looks pretty platform independent.  I'll look into it and ping you on irc if I have any questions.

[Alex Christensen]

Nobody's on irc.  I can compile everything except mediaControlsAppleJavaScript and mediaControlsAppleUserAgentStyleSheet are not defined ing RenderThemeWin.cpp and I cannot figure out how to define them.  Would it be ok to use those?  How are they generated?  It seems like it's related to http://trac.webkit.org/changeset/156546

[Jer Noble]

(In reply to comment #4)
> Nobody's on irc.  I can compile everything except mediaControlsAppleJavaScript and mediaControlsAppleUserAgentStyleSheet are not defined ing RenderThemeWin.cpp and I cannot figure out how to define them.  Would it be ok to use those?  How are they generated?  It seems like it's related to http://trac.webkit.org/changeset/156546

They're generated by DerivedSources.make and will be defined in UserAgentStyleSheets.h and UserAgentScripts.h, if memory serves.

[Brent Fulgham]

Comment on attachment 217840 [details]
Patch

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

I have a few questions about the implementation! Can you take a look and answer?

> Source/WebCore/rendering/RenderMediaControlsWinCairo.cpp:101
> +    // TODO: fix image...

Please provide a bug number for this image fix!

> Source/WebCore/rendering/RenderMediaControlsWinCairo.cpp:108
> +    static Image* mediaSliderThumb = platformResource("mediaSliderThumb");

Why do we have this static function here, but simply declare static inline functions elsewhere? Is this used in a number of places?

> Source/WebCore/rendering/RenderMediaControlsWinCairo.cpp:122
> +    // FIXME: this should be a rounded rect but need to fix GraphicsContextSkia first.

I don't think this comment is useful anymore; we no longer have a Skia-based port.

> Source/WebCore/rendering/RenderMediaControlsWinCairo.cpp:134
> +    // FIXME: Draw multiple ranges if there are multiple buffered ranges.

Bug number!

[Alex Christensen]

(In reply to comment #6)
> I have a few questions about the implementation! Can you take a look and answer?
This was just me blindly updating Philippe's code to get it to compile.  I think I'll take Eric's advice and use the mediacontrols module instead.  It would be cool if we didn't have to add more code.  I've almost got it compiling without using #if(0) anywhere!

[Philippe Normand]

(In reply to comment #7)
> (In reply to comment #6)
> > I have a few questions about the implementation! Can you take a look and answer?
> This was just me blindly updating Philippe's code to get it to compile.  I think I'll take Eric's advice and use the mediacontrols module instead.  It would be cool if we didn't have to add more code.  I've almost got it compiling without using #if(0) anywhere!

I worked on this much before the nice mediacontrols module indeed :)

[Eric Carlson]

Comment on attachment 217840 [details]
Patch

Clearing r? for now while Alex tries another approach.

[Alex Christensen]

I got it working without these files.  I'll post my changes in other bugs.