Attachment 134328 Details for Bug 82150 – Fixed nits

[patch] Fixed nits

bug-82150-20120328203101.patch (text/plain), 14.53 KB, created by Victor Carbune on 2012-03-28 10:31:03 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Victor Carbune

Created: 2012-03-28 10:31:03 PDT

Size: 14.53 KB

patch

obsolete

>Subversion Revision: 112401
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 77a690b89f596e53531c37d573982e1517c6295d..f316d02269be7cb554ac7f4835452b9f5e8c230d 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2012-03-27  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 NOBODY (OOPS!).
>+
>+        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  Carlos Garcia Campos  <cgarcia@igalia.com>
> 
>         [GTK] Implement PlatformStrategies
>diff --git a/Source/WebCore/css/mediaControls.css b/Source/WebCore/css/mediaControls.css
>index b81332fc7613a259f97e611d4793f4a421d60460..1be6da40f8f89852679ea9b292c7f9eca875cfef 100644
>--- a/Source/WebCore/css/mediaControls.css
>+++ b/Source/WebCore/css/mediaControls.css
>@@ -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;
>diff --git a/Source/WebCore/css/mediaControlsChromium.css b/Source/WebCore/css/mediaControlsChromium.css
>index 5fb62cc34db11e670eb10e1b5256fc26ee14d8d9..3930e162bb9175d42286d3900b4582cb475f8020 100644
>--- a/Source/WebCore/css/mediaControlsChromium.css
>+++ b/Source/WebCore/css/mediaControlsChromium.css
>@@ -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;
>diff --git a/Source/WebCore/html/shadow/MediaControlElements.cpp b/Source/WebCore/html/shadow/MediaControlElements.cpp
>index 0f34538808dbb7cdd549eec831219ef94c9a5058..5e4af9d670290790cf203e647c408b0b4024e637 100644
>--- a/Source/WebCore/html/shadow/MediaControlElements.cpp
>+++ b/Source/WebCore/html/shadow/MediaControlElements.cpp
>@@ -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)
>diff --git a/Source/WebCore/html/shadow/MediaControlElements.h b/Source/WebCore/html/shadow/MediaControlElements.h
>index efa69a58c5a6e2afc66bb15a23b338571eaee497..2c7dfd810cc7cd9ee08202e91e921157035eec0b 100644
>--- a/Source/WebCore/html/shadow/MediaControlElements.h
>+++ b/Source/WebCore/html/shadow/MediaControlElements.h
>@@ -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;
>diff --git a/Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp b/Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp
>index bed2abb8513de3f32977c3baffdc68f8e690ae8d..dc498b86d80f54ecbc770dc0f9239591f9df1213 100644
>--- a/Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp
>+++ b/Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp
>@@ -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;
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 23f81891a7caf8ab0ddfb2d725532c818b3f0e9d..50657a006dbb321bb43ccc7c811d280b3fd1ec62 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2012-03-27  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 NOBODY (OOPS!).
>+
>+        * 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  Yi Shen  <yi.4.shen@nokia.com>
> 
>         An extra line break is inserted when pasting into a font element.
>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
>index 0000000000000000000000000000000000000000..78d3b6f83e5aad9d0515b05f6f04b1396823f615
>--- /dev/null
>+++ b/LayoutTests/media/video-controls-rendering-toggle-display-none-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..6d82babdceb30671bab95428e9166781277a088e
>--- /dev/null
>+++ b/LayoutTests/media/video-controls-rendering-toggle-display-none.html
>@@ -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>
>diff --git a/LayoutTests/platform/chromium/test_expectations.txt b/LayoutTests/platform/chromium/test_expectations.txt
>index 37fcb2b00dbf9f19db6f510d5aefd91744d35580..826b0f7c461e18dcf66e4ef970c69426bed1cca0 100644
>--- a/LayoutTests/platform/chromium/test_expectations.txt
>+++ b/LayoutTests/platform/chromium/test_expectations.txt
>@@ -4127,6 +4127,24 @@ BUGWK78412 MAC WIN : fast/table/cell-pref-width-invalidation.html = TEXT
> 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
>

Actions: View | Formatted Diff | Diff

Attachments on bug 82150: 133700 | 133702 | 134039 | 134048 | 134315 | 134328 | 135820 | 136015 | 136449 | 138011 | 138121 | 138337