Summary: | Implement default media UI for mac chromium. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Albert J. Wong <ajwong> | ||||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | brettw, eric, levin | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Attachments: |
|
Description
Albert J. Wong
2009-09-10 16:51:40 PDT
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. |