Bug 18965 - Allow platform specific adjustments to the default style sheet
Summary: Allow platform specific adjustments to the default style sheet
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P3 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-05-09 04:46 PDT by Tor Arne Vestbø
Modified: 2008-06-09 01:17 PDT (History)
3 users (show)

See Also:


Attachments
Patch (14.56 KB, text/plain)
2008-05-09 04:48 PDT, Tor Arne Vestbø
no flags Details
Allow platform specific adjustments to the default style sheet (6.83 KB, patch)
2008-05-28 05:34 PDT, Tor Arne Vestbø
no flags Details | Formatted Diff | Diff
Allow platform specific adjustments to the default style sheet (6.96 KB, patch)
2008-05-28 06:12 PDT, Tor Arne Vestbø
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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