Change the volume slider to horizontal rendering for Chrome video controls.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Jun 2012 14:31:59 +0000 (14:31 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Jun 2012 14:31:59 +0000 (14:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=87835

Patch by Silvia Pfeiffer <silviapf@chromium.org> on 2012-06-12
Reviewed by Eric Carlson.

Source/WebCore:

No new tests - final patch in the Chrome controls update series will contain rebaselined tests.

The Chrome video controls are receiving a visual update. The volume slider is moved into
the controls with horizontal rendering, the volume slider container is removed. The visual
update itself is in a separate patch.

* css/mediaControlsChromium.css:
(audio::-webkit-media-controls-mute-button, video::-webkit-media-controls-mute-button):
Removed relative positioning to render all controls elements equally in DOM order.
(audio::-webkit-media-controls-play-button, video::-webkit-media-controls-play-button):
Removed absolute positioning to render all controls elements equally in DOM order.
(audio::-webkit-media-controls-timeline-container, video::-webkit-media-controls-timeline-container):
Removed absolute positioning to render all controls elements equally in DOM order.
(audio::-webkit-media-controls-volume-slider-container, video::-webkit-media-controls-volume-slider-container):
Removed volume slider container - it's not necessary any more.
(audio::-webkit-media-controls-current-time-display, video::-webkit-media-controls-current-time-display):
Remove flexbox and introduce inline display to always display this field.
(audio::-webkit-media-controls-volume-slider, video::-webkit-media-controls-volume-slider):
Removed absolute positioning to render all controls elements equally in DOM order.
* html/shadow/MediaControlRootElementChromium.cpp:
(WebCore::MediaControlRootElementChromium::MediaControlRootElementChromium):
Removed volume slider container - it's not necessary any more.
(WebCore::MediaControlRootElementChromium::create):
Appended the volume slider and the mute button directly to the panel, removed volume slider container.
(WebCore::MediaControlRootElementChromium::setMediaController):
Removed volume slider container.
(WebCore::MediaControlRootElementChromium::reportedError):
Removed volume slider container.
(WebCore::MediaControlRootElementChromium::showVolumeSlider):
Removed volume slider container.
* html/shadow/MediaControlRootElementChromium.h:
(WebCore):
(MediaControlRootElementChromium):
Removed volume slider container.
* html/shadow/SliderThumbElement.cpp:
(WebCore::hasVerticalAppearance):
Allow use of horizontal media volume slider.
(WebCore::RenderSliderThumb::layout):
Reuse hasVerticalAppearance function.
* rendering/RenderMediaControlsChromium.cpp:
(WebCore::RenderMediaControlsChromium::paintMediaVolumeSlider):
Change line drawing from vertical to horizontal.
* rendering/RenderTheme.h:
(WebCore::RenderTheme::usesVerticalVolumeSlider):
Add usesVerticalVolumeSlider function to allow horizontal volume sliders.
* rendering/RenderThemeChromiumMac.h:
(WebCore::RenderThemeChromiumMac::usesVerticalVolumeSlider):
Add usesVerticalVolumeSlider function to allow horizontal volume sliders.
Add usesVerticalVolumeSlider function to allow horizontal volume sliders.
* rendering/RenderThemeChromiumSkia.h:
(WebCore::RenderThemeChromiumSkia::usesVerticalVolumeSlider):
Add usesVerticalVolumeSlider function to allow horizontal volume sliders.

LayoutTests:

The Chrome video controls are receiving a visual update. The volume slider is included into
the controls with horizontal rendering, the volume slider container is removed. The visual
update itself is in a separate patch.

* platform/chromium/TestExpectations:
Temporarily skip tests related to volume controls rendering for Chromium.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@120072 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/chromium/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/css/mediaControlsChromium.css
Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp
Source/WebCore/html/shadow/MediaControlRootElementChromium.h
Source/WebCore/html/shadow/SliderThumbElement.cpp
Source/WebCore/rendering/RenderMediaControlsChromium.cpp
Source/WebCore/rendering/RenderTheme.h
Source/WebCore/rendering/RenderThemeChromiumMac.h
Source/WebCore/rendering/RenderThemeChromiumSkia.h

index 416e093..047233c 100644 (file)
@@ -1,3 +1,17 @@
+2012-06-12  Silvia Pfeiffer  <silviapf@chromium.org>
+
+        Change the volume slider to horizontal rendering for Chrome video controls.
+        https://bugs.webkit.org/show_bug.cgi?id=87835
+
+        Reviewed by Eric Carlson.
+
+        The Chrome video controls are receiving a visual update. The volume slider is included into
+        the controls with horizontal rendering, the volume slider container is removed. The visual
+        update itself is in a separate patch.
+
+        * platform/chromium/TestExpectations:
+        Temporarily skip tests related to volume controls rendering for Chromium.
+
 2012-06-12  Andrei Bucur  <abucur@adobe.com>
 
         [CSSRegions]NamedFlow::overset should return true when there's no region chain attached
index fbf2fc7..0e271fe 100644 (file)
@@ -774,6 +774,11 @@ WONTFIX SKIP : media/video-size-intrinsic-scale.html = PASS
 // Doesn't apply to Chromium (QuickTime-specific behavior)
 WONTFIX SKIP : media/video-does-not-loop.html = TIMEOUT
 
+// Doesn't apply to Chromium any longer - volume slider is inline with video controls.
+WONTFIX SKIP : media/video-volume-slider.html = FAIL
+WONTFIX SKIP : media/media-volume-slider-rendered-below.html = FAIL
+WONTFIX SKIP : media/media-volume-slider-rendered-normal.html = FAIL
+
 // QuickTime reference movies not supported.
 WONTFIX SKIP : http/tests/media/video-cross-site.html = PASS
 
@@ -2972,9 +2977,11 @@ BUGWK83882 : media/track/track-mode.html = PASS TEXT TIMEOUT
 
 // Need rebaseline.
 BUGWK87683 : fast/layers/video-layer.html = FAIL
+BUGWK87683 : fullscreen/full-screen-stacking-context.html = FAIL
 BUGWK87683 : media/audio-controls-rendering.html = FAIL
 BUGWK87683 : media/audio-repaint.html = FAIL
 BUGWK87683 : media/controls-after-reload.html = FAIL
+BUGWK87683 : media/controls-layout-direction.html = FAIL
 BUGWK87683 : media/controls-strict.html = FAIL
 BUGWK87683 : media/controls-styling.html = FAIL
 BUGWK87683 : media/controls-without-preload.html = FAIL
@@ -2986,7 +2993,7 @@ BUGWK87683 : media/video-empty-source.html = FAIL
 BUGWK87683 : media/video-no-audio.html = FAIL
 BUGWK87683 : media/video-playing-and-pause.html = FAIL
 BUGWK87683 : media/video-zoom-controls.html = FAIL
-BUGWK87683 : media/video-volume-slider.html = FAIL
+BUGWK87835 : http/tests/media/video-buffered-range-contains-currentTime.html = FAIL
 
 BUGWK72271 SNOWLEOPARD DEBUG : fast/dom/node-iterator-reference-node-moved-crash.html = PASS CRASH
 
index 85eab5d..7c351cb 100644 (file)
@@ -1,3 +1,63 @@
+2012-06-12  Silvia Pfeiffer  <silviapf@chromium.org>
+
+        Change the volume slider to horizontal rendering for Chrome video controls.
+        https://bugs.webkit.org/show_bug.cgi?id=87835
+
+        Reviewed by Eric Carlson.
+
+        No new tests - final patch in the Chrome controls update series will contain rebaselined tests.
+
+        The Chrome video controls are receiving a visual update. The volume slider is moved into
+        the controls with horizontal rendering, the volume slider container is removed. The visual
+        update itself is in a separate patch.
+
+        * css/mediaControlsChromium.css:
+        (audio::-webkit-media-controls-mute-button, video::-webkit-media-controls-mute-button):
+        Removed relative positioning to render all controls elements equally in DOM order.
+        (audio::-webkit-media-controls-play-button, video::-webkit-media-controls-play-button):
+        Removed absolute positioning to render all controls elements equally in DOM order.
+        (audio::-webkit-media-controls-timeline-container, video::-webkit-media-controls-timeline-container):
+        Removed absolute positioning to render all controls elements equally in DOM order.
+        (audio::-webkit-media-controls-volume-slider-container, video::-webkit-media-controls-volume-slider-container):
+        Removed volume slider container - it's not necessary any more.
+        (audio::-webkit-media-controls-current-time-display, video::-webkit-media-controls-current-time-display):
+        Remove flexbox and introduce inline display to always display this field.
+        (audio::-webkit-media-controls-volume-slider, video::-webkit-media-controls-volume-slider):
+        Removed absolute positioning to render all controls elements equally in DOM order.
+        * html/shadow/MediaControlRootElementChromium.cpp:
+        (WebCore::MediaControlRootElementChromium::MediaControlRootElementChromium):
+        Removed volume slider container - it's not necessary any more.
+        (WebCore::MediaControlRootElementChromium::create):
+        Appended the volume slider and the mute button directly to the panel, removed volume slider container.
+        (WebCore::MediaControlRootElementChromium::setMediaController):
+        Removed volume slider container.
+        (WebCore::MediaControlRootElementChromium::reportedError):
+        Removed volume slider container.
+        (WebCore::MediaControlRootElementChromium::showVolumeSlider):
+        Removed volume slider container.
+        * html/shadow/MediaControlRootElementChromium.h:
+        (WebCore):
+        (MediaControlRootElementChromium):
+        Removed volume slider container.
+        * html/shadow/SliderThumbElement.cpp:
+        (WebCore::hasVerticalAppearance):
+        Allow use of horizontal media volume slider.
+        (WebCore::RenderSliderThumb::layout):
+        Reuse hasVerticalAppearance function.
+        * rendering/RenderMediaControlsChromium.cpp:
+        (WebCore::RenderMediaControlsChromium::paintMediaVolumeSlider):
+        Change line drawing from vertical to horizontal.
+        * rendering/RenderTheme.h:
+        (WebCore::RenderTheme::usesVerticalVolumeSlider):
+        Add usesVerticalVolumeSlider function to allow horizontal volume sliders.
+        * rendering/RenderThemeChromiumMac.h:
+        (WebCore::RenderThemeChromiumMac::usesVerticalVolumeSlider):
+        Add usesVerticalVolumeSlider function to allow horizontal volume sliders.
+        Add usesVerticalVolumeSlider function to allow horizontal volume sliders.
+        * rendering/RenderThemeChromiumSkia.h:
+        (WebCore::RenderThemeChromiumSkia::usesVerticalVolumeSlider):
+        Add usesVerticalVolumeSlider function to allow horizontal volume sliders.
+
 2012-06-12  Andrei Bucur  <abucur@adobe.com>
 
         [CSSRegions]NamedFlow::overset should return true when there's no region chain attached
index e249562..65bf787 100644 (file)
@@ -67,25 +67,13 @@ audio::-webkit-media-controls-panel, video::-webkit-media-controls-panel {
 
 audio::-webkit-media-controls-mute-button, video::-webkit-media-controls-mute-button {
     -webkit-appearance: media-mute-button;
-    position: relative;
-    top: auto;
-    bottom: 0;
-    right: 0;
-    left: auto;
-
     width: 34px;
     height: 32px;
 }
 
 audio::-webkit-media-controls-play-button, video::-webkit-media-controls-play-button {
     -webkit-appearance: media-play-button;
-
-    position: absolute;
-    top: auto;
-    bottom: 7px;
-    left: 7px;
-    right: 6px;
-
+    margin: 7px 8px 6px 8px;
     width: 18px;
     height: 19px;
 }
@@ -97,13 +85,6 @@ audio::-webkit-media-controls-timeline-container, video::-webkit-media-controls-
     -webkit-box-align: center;
     -webkit-box-pack: center;
     -webkit-box-flex: 1;
-
-    position: absolute;
-    top: auto;
-    bottom: 0;
-    left: 30px;
-    right: 34px;
-
     width: auto;
     height: 32px;
 
@@ -114,10 +95,7 @@ audio::-webkit-media-controls-timeline-container, video::-webkit-media-controls-
 audio::-webkit-media-controls-current-time-display, video::-webkit-media-controls-current-time-display {
     -webkit-appearance: media-current-time-display;
     -webkit-user-select: none;
-    display: -webkit-box;
-    -webkit-box-flex: 0;
-    -webkit-box-pack: center;
-    -webkit-box-align: center;
+    display: inline;
 
     overflow: hidden;
     cursor: default;
@@ -158,25 +136,11 @@ audio::-webkit-media-controls-timeline, video::-webkit-media-controls-timeline {
     color: rgb(50, 140, 223);
 }
 
-audio::-webkit-media-controls-volume-slider-container, video::-webkit-media-controls-volume-slider-container {
-    -webkit-appearance: media-volume-slider-container;
-    position: absolute;
-    bottom: 32px;
-
-    width: 34px;
-    height: 100px;
-
-    background-color: rgba(0, 0, 0, 0.6);
-}
-
 audio::-webkit-media-controls-volume-slider, video::-webkit-media-controls-volume-slider {
     -webkit-appearance: media-volume-slider;
-    display: inline;
-    position: absolute;
-
-    top: 10px;
-    left: 12px;
-
-    width: 10px;
-    height: 80px;
+    display: -webkit-box;
+    -webkit-box-flex: 1;
+    margin-right: 5px;
+    max-width: 100px;
+    height: 32px;
 }
index cb8a3a8..6245149 100644 (file)
@@ -72,7 +72,6 @@ MediaControlRootElementChromium::MediaControlRootElementChromium(Document* docum
     , m_timelineContainer(0)
     , m_panelMuteButton(0)
     , m_volumeSlider(0)
-    , m_volumeSliderContainer(0)
 #if ENABLE(FULLSCREEN_MEDIA_CONTROLS)
     , m_fullscreenButton(0)
 #endif
@@ -130,28 +129,15 @@ PassRefPtr<MediaControlRootElementChromium> MediaControlRootElementChromium::cre
     if (ec)
         return 0;
 
-    RefPtr<HTMLDivElement> panelVolumeControlContainer = HTMLDivElement::create(document);
-
-    RefPtr<MediaControlVolumeSliderContainerElement> volumeSliderContainer = MediaControlVolumeSliderContainerElement::create(document);
-
-    RefPtr<MediaControlVolumeSliderElement> slider = MediaControlVolumeSliderElement::create(document);
-    controls->m_volumeSlider = slider.get();
-    volumeSliderContainer->appendChild(slider.release(), ec, true);
-    if (ec)
-        return 0;
-
-    controls->m_volumeSliderContainer = volumeSliderContainer.get();
-    panelVolumeControlContainer->appendChild(volumeSliderContainer.release(), ec, true);
-    if (ec)
-        return 0;
-
     RefPtr<MediaControlPanelMuteButtonElement> panelMuteButton = MediaControlPanelMuteButtonElement::create(document, controls.get());
     controls->m_panelMuteButton = panelMuteButton.get();
-    panelVolumeControlContainer->appendChild(panelMuteButton.release(), ec, true);
+    panel->appendChild(panelMuteButton.release(), ec, true);
     if (ec)
         return 0;
 
-    panel->appendChild(panelVolumeControlContainer, ec, true);
+    RefPtr<MediaControlVolumeSliderElement> slider = MediaControlVolumeSliderElement::create(document);
+    controls->m_volumeSlider = slider.get();
+    panel->appendChild(slider.release(), ec, true);
     if (ec)
         return 0;
 
@@ -194,8 +180,6 @@ void MediaControlRootElementChromium::setMediaController(MediaControllerInterfac
         m_panelMuteButton->setMediaController(controller);
     if (m_volumeSlider)
         m_volumeSlider->setMediaController(controller);
-    if (m_volumeSliderContainer)
-        m_volumeSliderContainer->setMediaController(controller);
 #if ENABLE(FULLSCREEN_MEDIA_CONTROLS)
     if (m_fullscreenButton)
         m_fullscreenButton->setMediaController(controller);
@@ -221,7 +205,6 @@ void MediaControlRootElementChromium::hide()
 {
     m_panel->setIsDisplayed(false);
     m_panel->hide();
-    m_volumeSliderContainer->hide();
 }
 
 void MediaControlRootElementChromium::makeOpaque()
@@ -232,7 +215,6 @@ void MediaControlRootElementChromium::makeOpaque()
 void MediaControlRootElementChromium::makeTransparent()
 {
     m_panel->makeTransparent();
-    m_volumeSliderContainer->hide();
 }
 
 void MediaControlRootElementChromium::reset()
@@ -307,10 +289,10 @@ void MediaControlRootElementChromium::reportedError()
 
     m_timelineContainer->hide();
     m_panelMuteButton->hide();
-    m_volumeSliderContainer->hide();
 #if ENABLE(FULLSCREEN_MEDIA_CONTROLS)
     m_fullscreenButton->hide();
 #endif
+    m_volumeSlider->hide();
 }
 
 void MediaControlRootElementChromium::updateStatusDisplay()
@@ -380,7 +362,7 @@ void MediaControlRootElementChromium::showVolumeSlider()
     if (!m_mediaController->hasAudio())
         return;
 
-    m_volumeSliderContainer->show();
+    m_volumeSlider->show();
 }
 
 #if ENABLE(VIDEO_TRACK)
index 6ad32dd..f4c3921 100644 (file)
@@ -47,7 +47,6 @@ class MediaControlTimeDisplayElement;
 class MediaControlTimelineContainerElement;
 class MediaControlMuteButtonElement;
 class MediaControlVolumeSliderElement;
-class MediaControlVolumeSliderContainerElement;
 class MediaControlPanelElement;
 class MediaControlChromiumEnclosureElement;
 class MediaControllerInterface;
@@ -139,7 +138,6 @@ private:
     MediaControlTimelineContainerElement* m_timelineContainer;
     MediaControlPanelMuteButtonElement* m_panelMuteButton;
     MediaControlVolumeSliderElement* m_volumeSlider;
-    MediaControlVolumeSliderContainerElement* m_volumeSliderContainer;
 #if ENABLE(FULLSCREEN_MEDIA_CONTROLS)
     MediaControlFullscreenButtonElement* m_fullscreenButton;
 #endif
index 4e714a0..aa750ca 100644 (file)
@@ -62,7 +62,10 @@ inline static bool hasVerticalAppearance(HTMLInputElement* input)
 {
     ASSERT(input->renderer());
     RenderStyle* sliderStyle = input->renderer()->style();
-    return sliderStyle->appearance() == SliderVerticalPart || sliderStyle->appearance() == MediaVolumeSliderPart;
+    RenderTheme* sliderTheme = input->renderer()->theme();
+
+    return sliderStyle->appearance() == SliderVerticalPart
+        || (sliderStyle->appearance() == MediaVolumeSliderPart && sliderTheme->usesVerticalVolumeSlider());
 }
 
 SliderThumbElement* sliderThumbElementOf(Node* node)
@@ -108,7 +111,7 @@ void RenderSliderThumb::layout()
     // Do not cast node() to SliderThumbElement. This renderer is used for
     // TrackLimitElement too.
     HTMLInputElement* input = node()->shadowAncestorNode()->toInputElement();
-    bool isVertical = style()->appearance() == SliderThumbVerticalPart || style()->appearance() == MediaVolumeSliderThumbPart;
+    bool isVertical = hasVerticalAppearance(input);
 
     double fraction = convertInputNumberToDouble(sliderPosition(input) * 100);
     if (isVertical)
index 72e12c8..b0c308f 100644 (file)
@@ -200,8 +200,8 @@ static bool paintMediaVolumeSlider(RenderObject* object, const PaintInfo& paintI
     if (originalColor != Color::white)
         context->setStrokeColor(Color::white, ColorSpaceDeviceRGB);
 
-    int x = rect.x() + rect.width() / 2;
-    context->drawLine(IntPoint(x, rect.y()),  IntPoint(x, rect.y() + rect.height()));
+    int y = rect.y() + rect.height() / 2;
+    context->drawLine(IntPoint(rect.x(), y),  IntPoint(rect.x() + rect.width(), y));
 
     if (originalColor != Color::white)
         context->setStrokeColor(originalColor, ColorSpaceDeviceRGB);
index 8167ba8..5e56b6d 100644 (file)
@@ -198,6 +198,7 @@ public:
     virtual bool hasOwnDisabledStateHandlingFor(ControlPart) const { return false; }
     virtual bool usesMediaControlStatusDisplay() { return false; }
     virtual bool usesMediaControlVolumeSlider() const { return true; }
+    virtual bool usesVerticalVolumeSlider() const { return true; }
     virtual double mediaControlsFadeInDuration() { return 0.1; }
     virtual double mediaControlsFadeOutDuration() { return 0.3; }
     virtual String formatMediaControlsTime(float time) const;
index 2b4c8c6..65bfab2 100644 (file)
@@ -55,6 +55,7 @@ protected:
     virtual IntPoint volumeSliderOffsetFromMuteButton(RenderBox*, const IntSize&) const OVERRIDE;
     virtual bool usesMediaControlStatusDisplay() { return false; }
     virtual bool hasOwnDisabledStateHandlingFor(ControlPart) const { return true; }
+    virtual bool usesVerticalVolumeSlider() const { return false; }
 #endif
 
     virtual bool usesTestModeFocusRingColor() const;
index 0028ebb..1ea06fe 100644 (file)
@@ -127,6 +127,7 @@ class RenderThemeChromiumSkia : public RenderTheme {
 #if ENABLE(VIDEO)
         // Media controls
         virtual bool hasOwnDisabledStateHandlingFor(ControlPart) const { return true; }
+        virtual bool usesVerticalVolumeSlider() const { return false; }
 #endif
 
         // Provide a way to pass the default font size from the Settings object