Bug 18965

Summary: Allow platform specific adjustments to the default style sheet
Product: WebKit Reporter: Tor Arne Vestbø <vestbo>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: alp, hausmann, koivisto
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
Allow platform specific adjustments to the default style sheet
none
Allow platform specific adjustments to the default style sheet darin: review+

Description Tor Arne Vestbø 2008-05-09 04:46:35 PDT
Would be nice if platforms could style the media player controls using the render theme.
Comment 1 Tor Arne Vestbø 2008-05-09 04:48:24 PDT
Created attachment 21035 [details]
Patch

This patch adds callback to the render theme for styling the media player controls, and modifies the html4.css to allow controls to be placed in all parts of the media element.
Comment 2 Tor Arne Vestbø 2008-05-09 04:53:10 PDT
Antti, any comments?
Comment 3 Adele Peterson 2008-05-14 16:48:27 PDT
Comment on attachment 21035 [details]
Patch

do you know if the html4.css changes affect the mac or windows controls?

other than that concern, this looks pretty reasonable
Comment 4 Tor Arne Vestbø 2008-05-15 02:49:17 PDT
I tried the html4.css changes on Safari/Mac and the controls looked and behaved like before. 

But I have found an issue with adjustMediaSliderThumbStyle(), which does not get called. I'll look into that. Also, we should probably let the theme style the audio element, since it's now hard-coded to:

audio {
    width: 200px;
    height: 16px;
}

I'll get back to you with an updated patch.
Comment 5 Tor Arne Vestbø 2008-05-28 05:34:27 PDT
Created attachment 21386 [details]
Allow platform specific adjustments to the default style sheet

The new patch implements functionality to do platform specific adjustments to the default stylesheet.

This allows us to style the media control buttons by providing a default look before the UA and user styles are computed, which has the advantage of not overriding any user-document defined adjustments to the controls. It's also conceptually more in line with what we're doing, i.e. providing a default visual look, not fixing and overriding styles to ensure a given platform-look at all times.
Comment 6 Tor Arne Vestbø 2008-05-28 06:12:47 PDT
Created attachment 21387 [details]
Allow platform specific adjustments to the default style sheet

Updated patch to reflect comments from Simon about QString::fromUtf8() and actually adding the css-file to the qrc-file :)
Comment 7 Darin Adler 2008-06-08 14:02:16 PDT
Comment on attachment 21387 [details]
Allow platform specific adjustments to the default style sheet

+#include "CSSStyleSheet.h"

This include is not needed in RenderTheme.cpp.

+void RenderTheme::adjustDefaultStyleSheet(CSSStyleSheet* style) {}

For people compiling with unused argument warnings it would be better to omit the word "style".

I don't see any major benefit to using the "all in one line" format here. This could just be laid out in the normal way.

Seems OK, r=me
Comment 8 Tor Arne Vestbø 2008-06-09 01:17:34 PDT
Thanks for your comments Darin