Source/WebCore: Removed particular rendering for the volume slider and used
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Mar 2012 01:07:20 +0000 (01:07 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Mar 2012 01:07:20 +0000 (01:07 +0000)
css positioning instead (simpler and less prone to errors when
changes occur).
https://bugs.webkit.org/show_bug.cgi?id=82150

Patch by Victor Carbune <vcarbune@adobe.com> on 2012-03-28
Reviewed by Eric Carlson.

Test: media/video-controls-rendering-toggle-display-none.html

* css/mediaControls.css: Updated css to correctly render controls.
(audio::-webkit-media-controls-panel, video::-webkit-media-controls-panel):
* css/mediaControlsChromium.css: Updated css to correctly render controls.
(audio::-webkit-media-controls-panel, video::-webkit-media-controls-panel):
(audio::-webkit-media-controls-mute-button, video::-webkit-media-controls-mute-button):
(audio::-webkit-media-controls-volume-slider-container, video::-webkit-media-controls-volume-slider-container):
* html/shadow/MediaControlElements.cpp: Removed particular renderer.
(WebCore):
* html/shadow/MediaControlElements.h: Removed particular renderer.
(MediaControlVolumeSliderContainerElement):
* html/shadow/MediaControlRootElementChromium.cpp:
(WebCore::MediaControlRootElementChromium::create): Added an anonymous div container
for the mute button and volume slider (to be displayed on top of each other)

LayoutTests: Added relevant test to support video controls display:none toggling.
https://bugs.webkit.org/show_bug.cgi?id=82150

Patch by Victor Carbune <vcarbune@adobe.com> on 2012-03-28
Reviewed by Eric Carlson.

* media/video-controls-rendering-toggle-display-none.html: Added.
* platform/chromium-linux/media/video-controls-rendering-toggle-display-none-expected.png: Added.
* platform/chromium-linux/media/video-controls-rendering-toggle-display-none-expected.txt: Added.
* platform/chromium/test_expectations.txt: Updated expectations, more rebaselining is needed.

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

LayoutTests/ChangeLog
LayoutTests/media/video-controls-rendering-toggle-display-none-expected.txt [new file with mode: 0644]
LayoutTests/media/video-controls-rendering-toggle-display-none.html [new file with mode: 0644]
LayoutTests/platform/chromium/test_expectations.txt
Source/WebCore/ChangeLog
Source/WebCore/css/mediaControls.css
Source/WebCore/css/mediaControlsChromium.css
Source/WebCore/html/shadow/MediaControlElements.cpp
Source/WebCore/html/shadow/MediaControlElements.h
Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp

index 684553c..a31b27c 100644 (file)
@@ -1,3 +1,15 @@
+2012-03-28  Victor Carbune  <vcarbune@adobe.com>
+
+        Added relevant test to support video controls display:none toggling.
+        https://bugs.webkit.org/show_bug.cgi?id=82150
+
+        Reviewed by Eric Carlson.
+
+        * media/video-controls-rendering-toggle-display-none.html: Added.
+        * platform/chromium-linux/media/video-controls-rendering-toggle-display-none-expected.png: Added.
+        * platform/chromium-linux/media/video-controls-rendering-toggle-display-none-expected.txt: Added.
+        * platform/chromium/test_expectations.txt: Updated expectations, more rebaselining is needed.
+
 2012-03-28  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r110064.
diff --git a/LayoutTests/media/video-controls-rendering-toggle-display-none-expected.txt b/LayoutTests/media/video-controls-rendering-toggle-display-none-expected.txt
new file mode 100644 (file)
index 0000000..78d3b6f
--- /dev/null
@@ -0,0 +1,11 @@
+Tests that the video controls are properly rendered when the display none is set and unset.
+
+** The volume slider has the same left offset as the mute button
+EXPECTED (volumeSliderElement.offsetLeft == muteButtonElement.offsetLeft == 'true') OK
+
+** The volume slider should be on top of the mute button **
+EXPECTED (volumeSliderElement.offsetTop < muteButtonElement.offsetTop == 'true') OK
+
+** There should be no empty space between the two controls **
+EXPECTED (volumeSliderElement.offsetTop + volumeSliderElement.offsetHeight == muteButtonElement.offsetTop == 'true') OK
+
diff --git a/LayoutTests/media/video-controls-rendering-toggle-display-none.html b/LayoutTests/media/video-controls-rendering-toggle-display-none.html
new file mode 100644 (file)
index 0000000..6d82bab
--- /dev/null
@@ -0,0 +1,72 @@
+<html>
+<head>
+    <title>Test rendering of volume slider of video tag</title>
+    <script src=media-file.js></script>
+    <script src=media-controls.js></script>
+    <script src=video-test.js></script>
+    <script>
+        var video;
+        var panel;
+
+        var muteButtonCoordinates;
+        var volumeSliderCoordinates;
+
+        var volumeSliderElement;
+        var muteButtonElement;
+
+        function init()
+        {
+            video = document.getElementsByTagName("video")[0];
+            video.src = findMediaFile("video", "content/test");
+        }
+
+        function test()
+        {
+            if (window.eventSender) {
+                try {
+                    muteButtonCoordinates = mediaControlsButtonCoordinates(video, "mute-button");
+                    volumeSliderCoordinates = mediaControlsButtonCoordinates(video, "volume-slider-container");
+                } catch (exception) {
+                    layoutTestController.notifyDone();
+                    return;
+                }
+
+                eventSender.mouseMoveTo(muteButtonCoordinates[0], muteButtonCoordinates[1]);
+            }
+
+            panel = mediaControlsElement(internals.shadowRoot(video).firstChild, "-webkit-media-controls-panel");
+            volumeSliderElement = mediaControlsElement(internals.shadowRoot(video).firstChild, "-webkit-media-controls-volume-slider-container");
+            muteButtonElement = mediaControlsElement(internals.shadowRoot(video).firstChild, "-webkit-media-controls-mute-button");
+
+
+            // Ensure paint with display property set to "none".
+            panel.style.display = "none";
+            document.body.offsetTop;
+
+            // Ensure (re)paint with default display property.
+            panel.style.removeProperty("display");
+            document.body.offsetTop;
+
+            // Test that the left offset of both controls is equal.
+            consoleWrite("");
+            consoleWrite("** The volume slider has the same left offset as the mute button");
+            testExpected("volumeSliderElement.offsetLeft == muteButtonElement.offsetLeft", true);
+
+            consoleWrite("");
+            consoleWrite("** The volume slider should be on top of the mute button **");
+            testExpected("volumeSliderElement.offsetTop < muteButtonElement.offsetTop", true);
+
+            consoleWrite("");
+            consoleWrite("** There should be no empty space between the two controls **");
+            testExpected("volumeSliderElement.offsetTop + volumeSliderElement.offsetHeight == muteButtonElement.offsetTop", true);
+
+            layoutTestController.notifyDone();
+        }
+    </script>
+</head>
+<body onload="init()">
+    Tests that the video controls are properly rendered when the display none is set and unset.<br>
+
+    <video oncanplaythrough="test()" controls preload="true"></video>
+</body>
+</html>
index 53f52d2..e1f54dc 100644 (file)
@@ -4079,6 +4079,24 @@ BUGWK74147 : compositing/reflections/nested-reflection-on-overflow.html = IMAGE
 BUGWK37244 : tables/mozilla/bugs/bug27038-1.html = IMAGE+TEXT
 BUGWK37244 : tables/mozilla/bugs/bug27038-2.html = IMAGE+TEXT
 
+// Need rebaselining.
+BUGWK82150 : media/media-document-audio-repaint.html = IMAGE+TEXT
+BUGWK82150 : media/video-no-audio.html = IMAGE+TEXT
+BUGWK82150 : media/controls-strict.html = IMAGE+TEXT
+BUGWK82150 : media/video-volume-slider.html = IMAGE+TEXT
+BUGWK82150 : media/controls-styling.html = IMAGE+TEXT
+BUGWK82150 : media/video-display-toggle.html = IMAGE+TEXT
+BUGWK82150 : media/audio-repaint.html = IMAGE+TEXT
+BUGWK82150 : media/audio-controls-rendering.html = IMAGE+TEXT
+BUGWK82150 : media/video-zoom-controls.html = IMAGE+TEXT
+BUGWK82150 : media/video-controls-rendering.html = IMAGE+TEXT
+BUGWK82150 : media/controls-without-preload.html = IMAGE+TEXT
+BUGWK82150 : media/media-controls-clone.html = IMAGE+TEXT
+BUGWK82150 : fast/layers/video-layer.html = IMAGE+TEXT
+BUGWK82150 : media/video-empty-source.html = IMAGE+TEXT
+BUGWK82150 : media/video-playing-and-pause.html = IMAGE+TEXT
+BUGWK82150 : media/controls-after-reload.html = IMAGE+TEXT
+
 // New test, flaky since added in r110965.
 BUGWK82097 : editing/selection/move-by-word-visually-crash-test-5.html = PASS TIMEOUT
 
index bd98d42..32d6c37 100644 (file)
@@ -1,3 +1,28 @@
+2012-03-28  Victor Carbune  <vcarbune@adobe.com>
+
+        Removed particular rendering for the volume slider and used
+        css positioning instead (simpler and less prone to errors when
+        changes occur).
+        https://bugs.webkit.org/show_bug.cgi?id=82150
+
+        Reviewed by Eric Carlson.
+
+        Test: media/video-controls-rendering-toggle-display-none.html
+
+        * css/mediaControls.css: Updated css to correctly render controls.
+        (audio::-webkit-media-controls-panel, video::-webkit-media-controls-panel):
+        * css/mediaControlsChromium.css: Updated css to correctly render controls.
+        (audio::-webkit-media-controls-panel, video::-webkit-media-controls-panel): 
+        (audio::-webkit-media-controls-mute-button, video::-webkit-media-controls-mute-button):
+        (audio::-webkit-media-controls-volume-slider-container, video::-webkit-media-controls-volume-slider-container):
+        * html/shadow/MediaControlElements.cpp: Removed particular renderer.
+        (WebCore):
+        * html/shadow/MediaControlElements.h: Removed particular renderer.
+        (MediaControlVolumeSliderContainerElement):
+        * html/shadow/MediaControlRootElementChromium.cpp:
+        (WebCore::MediaControlRootElementChromium::create): Added an anonymous div container
+        for the mute button and volume slider (to be displayed on top of each other)
+
 2012-03-28  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r110064.
index b81332f..1be6da4 100644 (file)
@@ -47,7 +47,8 @@ audio {
 audio::-webkit-media-controls-panel, video::-webkit-media-controls-panel {
     display: -webkit-box;
     -webkit-box-orient: horizontal;
-    -webkit-box-align: center;
+    -webkit-box-align: end;
+    -webkit-box-pack: end;
     -webkit-user-select: none;
     position: relative;
     bottom: 0;
index 5fb62cc..3930e16 100644 (file)
@@ -41,6 +41,7 @@ audio:-webkit-full-page-media, video:-webkit-full-page-media {
 
 audio::-webkit-media-controls-panel, video::-webkit-media-controls-panel {
     -webkit-user-select: none;
+    -webkit-box-pack: end;
     position: relative;
     overflow: visible;
     bottom: 0;
@@ -56,7 +57,7 @@ video:-webkit-full-page-media::-webkit-media-controls-panel {
 
 audio::-webkit-media-controls-mute-button, video::-webkit-media-controls-mute-button {
     -webkit-appearance: media-mute-button;
-    position: absolute;
+    position: relative;
     top: auto;
     bottom: 0;
     right: 0;
@@ -149,7 +150,7 @@ audio::-webkit-media-controls-timeline, video::-webkit-media-controls-timeline {
 
 audio::-webkit-media-controls-volume-slider-container, video::-webkit-media-controls-volume-slider-container {
     -webkit-appearance: media-volume-slider-container;
-    position: absolute;
+    position: relative;
 
     width: 34px;
     height: 100px;
index 0f34538..5e4af9d 100644 (file)
@@ -320,34 +320,6 @@ const AtomicString& MediaControlTimelineContainerElement::shadowPseudoId() const
 
 // ----------------------------
 
-class RenderMediaVolumeSliderContainer : public RenderBlock {
-public:
-    RenderMediaVolumeSliderContainer(Node*);
-
-private:
-    virtual void layout();
-};
-
-RenderMediaVolumeSliderContainer::RenderMediaVolumeSliderContainer(Node* node)
-    : RenderBlock(node)
-{
-}
-
-void RenderMediaVolumeSliderContainer::layout()
-{
-    RenderBlock::layout();
-    if (style()->display() == NONE || !previousSibling() || !previousSibling()->isBox())
-        return;
-
-    RenderBox* buttonBox = toRenderBox(previousSibling());
-
-    LayoutStateDisabler layoutStateDisabler(view());
-
-    LayoutPoint offset = theme()->volumeSliderOffsetFromMuteButton(buttonBox, size());
-    setX(offset.x() + buttonBox->offsetLeft());
-    setY(offset.y() + buttonBox->offsetTop());
-}
-
 inline MediaControlVolumeSliderContainerElement::MediaControlVolumeSliderContainerElement(Document* document)
     : MediaControlElement(document)
 {
@@ -360,11 +332,6 @@ PassRefPtr<MediaControlVolumeSliderContainerElement> MediaControlVolumeSliderCon
     return element.release();
 }
 
-RenderObject* MediaControlVolumeSliderContainerElement::createRenderer(RenderArena* arena, RenderStyle*)
-{
-    return new (arena) RenderMediaVolumeSliderContainer(this);
-}
-
 void MediaControlVolumeSliderContainerElement::defaultEventHandler(Event* event)
 {
     if (!event->isMouseEvent() || event->type() != eventNames().mouseoutEvent)
index efa69a5..2c7dfd8 100644 (file)
@@ -159,7 +159,6 @@ public:
 
 private:
     MediaControlVolumeSliderContainerElement(Document*);
-    virtual RenderObject* createRenderer(RenderArena*, RenderStyle*);
     virtual void defaultEventHandler(Event*);
     virtual MediaControlElementType displayType() const;
     virtual const AtomicString& shadowPseudoId() const;
index bed2abb..dc498b8 100644 (file)
@@ -106,11 +106,7 @@ PassRefPtr<MediaControlRootElementChromium> MediaControlRootElementChromium::cre
     if (ec)
         return 0;
 
-    RefPtr<MediaControlPanelMuteButtonElement> panelMuteButton = MediaControlPanelMuteButtonElement::create(document, controls.get());
-    controls->m_panelMuteButton = panelMuteButton.get();
-    panel->appendChild(panelMuteButton.release(), ec, true);
-    if (ec)
-        return 0;
+    RefPtr<HTMLDivElement> panelVolumeControlContainer = HTMLDivElement::create(document);
 
     RefPtr<MediaControlVolumeSliderContainerElement> volumeSliderContainer = MediaControlVolumeSliderContainerElement::create(document);
 
@@ -121,7 +117,17 @@ PassRefPtr<MediaControlRootElementChromium> MediaControlRootElementChromium::cre
         return 0;
 
     controls->m_volumeSliderContainer = volumeSliderContainer.get();
-    panel->appendChild(volumeSliderContainer.release(), ec, true);
+    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);
+    if (ec)
+        return 0;
+
+    panel->appendChild(panelVolumeControlContainer, ec, true);
     if (ec)
         return 0;