Bug 23210

Summary: Make it easier for ports to define custom UI for media controls
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
proposed patch
eric: review+
Change media controls theming to be overrides instead of complete ruleset
none
Updated patch with changelog and #ifdefs in RenderTheme hausmann: review+

Description Eric Carlson 2009-01-09 11:49:05 PST
It should be easier for ports to define their own media controls UI. Specifically we should not require the CSS to be in a single static variable, and we should assume that hit testing a control element's bounding box is sufficient.
Comment 1 Eric Carlson 2009-01-09 11:50:05 PST
Created attachment 26566 [details]
proposed patch
Comment 2 Eric Carlson 2009-01-09 12:05:04 PST
err, I meant to say "we should NOT assume that hit testing a control element's bounding box is
sufficient"
Comment 3 Eric Seidel (no email) 2009-01-09 13:13:05 PST
Comment on attachment 26566 [details]
proposed patch

I would expect this to be either:
const String& or String.  I'm not sure what const String would do for you.

The preferred webkit style is no else after return:
+    if (renderer() && renderer()->style()->hasAppearance())
+        return theme()->hitTestMediaControlPart(renderer(), absPoint);
+    else
+        return false;

No need for "o" here:
+    virtual bool hitTestMediaControlPart(RenderObject* o, const IntPoint& absPoint);

Otherwise it looks fine.  If you're a commiter, go ahead and fix the nits and land, no need to see another patch.
Comment 4 Eric Carlson 2009-01-11 17:11:44 PST
Committed revision 39782
Comment 5 Tor Arne Vestbø 2009-01-13 05:35:35 PST
Created attachment 26665 [details]
Change media controls theming to be overrides instead of complete ruleset
Comment 6 Tor Arne Vestbø 2009-01-13 05:35:48 PST
Re-opening as I have a patch to how this works. Talked to Eric about this on IRC.
Comment 7 Tor Arne Vestbø 2009-01-13 05:36:44 PST
Talked to Eric Carlson that is :) 
Comment 8 Tor Arne Vestbø 2009-01-13 06:21:04 PST
Created attachment 26666 [details]
Updated patch with changelog and #ifdefs in RenderTheme
Comment 9 Simon Hausmann 2009-01-13 06:31:38 PST
Comment on attachment 26666 [details]
Updated patch with changelog and #ifdefs in RenderTheme

Looks good to me!