Bug 124868 - Implement Render Media Controls for WinCairo
Summary: Implement Render Media Controls for WinCairo
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-25 15:29 PST by Alex Christensen
Modified: 2013-12-16 20:57 PST (History)
7 users (show)

See Also:


Attachments
Patch (18.70 KB, patch)
2013-11-25 15:36 PST, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Alex Christensen 2013-11-25 15:36:46 PST
Created attachment 217840 [details]
Patch
Comment 2 Eric Carlson 2013-11-25 16:07:31 PST
Have you seen Source/WebCore/Modules/mediacontrols/? The Apple ports use this to implement media controls with JavaScript.
Comment 3 Alex Christensen 2013-11-25 19:19:17 PST
I haven't, but it looks pretty platform independent.  I'll look into it and ping you on irc if I have any questions.
Comment 4 Alex Christensen 2013-11-25 20:37:53 PST
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
Comment 5 Jer Noble 2013-11-26 07:57:00 PST
(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.
Comment 6 Brent Fulgham 2013-11-26 19:13:03 PST
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!
Comment 7 Alex Christensen 2013-11-30 21:48:28 PST
(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!
Comment 8 Philippe Normand 2013-12-02 06:01:31 PST
(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 :)
Comment 9 Eric Carlson 2013-12-02 07:52:35 PST
Comment on attachment 217840 [details]
Patch

Clearing r? for now while Alex tries another approach.
Comment 10 Alex Christensen 2013-12-16 20:57:18 PST
I got it working without these files.  I'll post my changes in other bugs.