The render theme code for mac chromium uses native mac widgets for media controls. This is inconsistent with the chromium UI for other platforms. Redo the render theme code in the style of the Windows and Linux ports.
Created attachment 39393 [details] Reimplement default media UI for Mac Chromium to match the style of the Windows and Linux versions. Also breaks the dependency on the internal wk* functions that were previously used to render the media controller widgets. https://bugs.webkit.org/show_bug.cgi?id=29161 Patch by Albert J. Wong <ajwong@chromium.org> on 2009-09-10 Reviewed by NOBODY (OOPS!). No media layout tests are currently enabled in Mac Chromium, so nothing needs rebaselineing, etc. * css/mediaControlsChromium.css: * rendering/RenderThemeChromiumMac.h: * rendering/RenderThemeChromiumMac.mm: (WebCore::mediaElementParent): (WebCore::RenderThemeChromiumMac::extraMediaControlsStyleSheet): (WebCore::mediaSliderThumbImage): (WebCore::mediaVolumeSliderThumbImage): (WebCore::RenderThemeChromiumMac::paintSliderTrack): (WebCore::RenderThemeChromiumMac::adjustSliderThumbSize): (WebCore::RenderThemeChromiumMac::paintMediaButtonInternal): (WebCore::RenderThemeChromiumMac::paintMediaPlayButton): (WebCore::RenderThemeChromiumMac::paintMediaMuteButton): (WebCore::RenderThemeChromiumMac::paintMediaSliderTrack): (WebCore::RenderThemeChromiumMac::paintMediaVolumeSliderTrack): (WebCore::RenderThemeChromiumMac::paintMediaSliderThumb): (WebCore::RenderThemeChromiumMac::paintMediaVolumeSliderThumb): (WebCore::RenderThemeChromiumMac::paintMediaControlsBackground): * rendering/RenderThemeChromiumSkia.cpp: (WebCore::RenderThemeChromiumSkia::adjustSliderThumbSize): --- 5 files changed, 256 insertions(+), 99 deletions(-)
Created attachment 39394 [details] Reimplement default media UI for Mac Chromium to match the style of the Windows and Linux versions. Also breaks the dependency on the internal wk* functions that were previously used to render the media controller widgets. https://bugs.webkit.org/show_bug.cgi?id=29161 Patch by Albert J. Wong <ajwong@chromium.org> on 2009-09-10 Reviewed by NOBODY (OOPS!). No media layout tests are currently enabled in Mac Chromium, so nothing needs rebaselineing, etc. * css/mediaControlsChromium.css: * rendering/RenderThemeChromiumMac.h: * rendering/RenderThemeChromiumMac.mm: (WebCore::mediaElementParent): (WebCore::RenderThemeChromiumMac::extraMediaControlsStyleSheet): (WebCore::mediaSliderThumbImage): (WebCore::mediaVolumeSliderThumbImage): (WebCore::RenderThemeChromiumMac::paintSliderTrack): (WebCore::RenderThemeChromiumMac::adjustSliderThumbSize): (WebCore::RenderThemeChromiumMac::paintMediaButtonInternal): (WebCore::RenderThemeChromiumMac::paintMediaPlayButton): (WebCore::RenderThemeChromiumMac::paintMediaMuteButton): (WebCore::RenderThemeChromiumMac::paintMediaSliderTrack): (WebCore::RenderThemeChromiumMac::paintMediaVolumeSliderTrack): (WebCore::RenderThemeChromiumMac::paintMediaSliderThumb): (WebCore::RenderThemeChromiumMac::paintMediaVolumeSliderThumb): (WebCore::RenderThemeChromiumMac::paintMediaControlsBackground): * rendering/RenderThemeChromiumSkia.cpp: (WebCore::RenderThemeChromiumSkia::adjustSliderThumbSize): --- 5 files changed, 255 insertions(+), 98 deletions(-)
Created attachment 39395 [details] Screenshot o new UI on white background.
If this looks good, please also commit-queue+.
Created attachment 39401 [details] Reimplement default media UI for Mac Chromium to match the style of the Windows and Linux versions. Also breaks the dependency on the internal wk* functions that were previously used to render the media controller widgets. https://bugs.webkit.org/show_bug.cgi?id=29161 Patch by Albert J. Wong <ajwong@chromium.org> on 2009-09-10 Reviewed by NOBODY (OOPS!). No media layout tests are currently enabled in Mac Chromium, so nothing needs rebaselineing, etc. * css/mediaControlsChromium.css: * rendering/RenderThemeChromiumMac.h: * rendering/RenderThemeChromiumMac.mm: (WebCore::mediaElementParent): (WebCore::RenderThemeChromiumMac::extraMediaControlsStyleSheet): (WebCore::mediaSliderThumbImage): (WebCore::mediaVolumeSliderThumbImage): (WebCore::RenderThemeChromiumMac::paintSliderTrack): (WebCore::RenderThemeChromiumMac::adjustSliderThumbSize): (WebCore::RenderThemeChromiumMac::paintMediaButtonInternal): (WebCore::RenderThemeChromiumMac::paintMediaPlayButton): (WebCore::RenderThemeChromiumMac::paintMediaMuteButton): (WebCore::RenderThemeChromiumMac::paintMediaSliderTrack): (WebCore::RenderThemeChromiumMac::paintMediaVolumeSliderTrack): (WebCore::RenderThemeChromiumMac::paintMediaSliderThumb): (WebCore::RenderThemeChromiumMac::paintMediaVolumeSliderThumb): (WebCore::RenderThemeChromiumMac::paintMediaControlsBackground): * rendering/RenderThemeChromiumSkia.cpp: (WebCore::RenderThemeChromiumSkia::adjustSliderThumbSize): --- 5 files changed, 252 insertions(+), 98 deletions(-)
FYI, hclam@chromium.org also looked this over and thinks it is sane. He did default UI implementation for Windows/Linux.
You can always set cq=? if you want folks to know to cq+ when reviewing.
Ah, didn't realize that. cq=? set.
Comment on attachment 39401 [details] Reimplement default media UI for Mac Chromium to match the style Just a few nits to fix up and then commit. > diff --git a/WebCore/rendering/RenderThemeChromiumMac.mm b/WebCore/rendering/RenderThemeChromiumMac.mm > +#if ENABLE(VIDEO) > +// Attempt to retrieve a HTMLMediaElement from a Node. Returns NULL if one cannot be found. Use 0 instead of NULL. check-webkit-style would have flagged this :) > + static Image* soundFull = Image::loadPlatformResource("mediaSoundFull").releaseRef(); > + static Image* soundNone = Image::loadPlatformResource("mediaSoundNone").releaseRef(); > + static Image* soundDisabled = Image::loadPlatformResource("mediaSoundDisabled").releaseRef(); > + > + if (mediaElement->networkState() == HTMLMediaElement::NETWORK_NO_SOURCE || !mediaElement->hasAudio()) > + return paintMediaButtonInternal(paintInfo.context, rect, soundDisabled); > + > + return paintMediaButtonInternal(paintInfo.context, rect, mediaElement->muted() ? soundNone: soundFull); Add a space before the : > + if (bufferedRect.width() > 0 && bufferedRect.height() > 0) { > + FloatPoint p0 = bufferedRect.location(); > + FloatPoint p1 = p0; Can you think of more meaningful names than p0 and p1? (sliderTop, sliderBottom?)
Committed r48438: <http://trac.webkit.org/changeset/48438>
I rolled back this change because it did not compile. WebCore/rendering/RenderThemeChromiumMac.mm: In member function 'virtual bool WebCore::RenderThemeChromiumMac::paintMediaSliderTrack(WebCore::RenderObject*, const WebCore::RenderObject::PaintInfo&, const WebCore::IntRect&)': /b/slave/webkit-rel-mac-webkit-org/build/src/webkit/../third_party/WebKit/WebCore/rendering/RenderThemeChromiumMac.mm:1966: error: 'p1' was not declared in this scope
Comment on attachment 39401 [details] Reimplement default media UI for Mac Chromium to match the style Marking this change as r- since it was rolled out.
Resolving bug since it has been committed again.