Attachment 136458 Details for Bug 82476 – Fixed ChangeLog entries

[patch] Fixed ChangeLog entries

bug-82476-20120410174341.patch (text/plain), 10.91 KB, created by Victor Carbune on 2012-04-10 07:43:43 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Victor Carbune

Created: 2012-04-10 07:43:43 PDT

Size: 10.91 KB

patch

obsolete

>Subversion Revision: 113315
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 31553d26d4054a8499da4cbb34b27d8aa366e4a7..bd678ab34698cb20e7877986dc519a561ea818b9 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2012-04-10  Victor Carbune  <vcarbune@adobe.com>
>+
>+        Extra display logic for the media control panel element
>+        https://bugs.webkit.org/show_bug.cgi?id=82476
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch fixes a bug which caused the controls to be displayed
>+        when they should remain hidden. Added an extra variable to the
>+        panel elements which properly keeps the state of the panel (visible or not).
>+
>+        Test: media/video-controls-toggling.html
>+
>+        * html/shadow/MediaControlElements.cpp:
>+        (WebCore::MediaControlPanelElement::MediaControlPanelElement): Added the
>+        variable m_isDisplayed to hold the state whether the panel is visible or not.
>+        (WebCore::MediaControlPanelElement::makeOpaque): Showing the panel only if it
>+        is visible.
>+        (WebCore::MediaControlPanelElement::makeTransparent): Enabled the transition
>+        timer which sets the display:none property on the controls.
>+        (WebCore::MediaControlPanelElement::setIsDisplayed): Setter for the state variable.
>+        (WebCore):
>+        * html/shadow/MediaControlElements.h:
>+        (MediaControlPanelElement):
>+        * html/shadow/MediaControlRootElement.cpp:
>+        (WebCore::MediaControlRootElement::show): Updated the panel visibility state.
>+        (WebCore::MediaControlRootElement::hide): Updated the panel visibility state.
>+        * html/shadow/MediaControlRootElementChromium.cpp:
>+        (WebCore::MediaControlRootElementChromium::show): Updated the panel visibility state.
>+        (WebCore::MediaControlRootElementChromium::hide): Updated the panel visibility state.
>+
> 2012-04-05  Pavel Feldman  <pfeldman@chromium.org>
> 
>         Web Inspector: add ability to copy resource URL from web inspector's resources page.
>diff --git a/Source/WebCore/html/shadow/MediaControlElements.cpp b/Source/WebCore/html/shadow/MediaControlElements.cpp
>index 0f34538808dbb7cdd549eec831219ef94c9a5058..c7580fa8a4f86fb60b2ff3f17bd9cb2df3320d16 100644
>--- a/Source/WebCore/html/shadow/MediaControlElements.cpp
>+++ b/Source/WebCore/html/shadow/MediaControlElements.cpp
>@@ -108,6 +108,7 @@ inline MediaControlPanelElement::MediaControlPanelElement(Document* document)
>     : MediaControlElement(document)
>     , m_canBeDragged(false)
>     , m_isBeingDragged(false)
>+    , m_isDisplayed(false)
>     , m_opaque(true)
>     , m_transitionTimer(this, &MediaControlPanelElement::transitionTimerFired)
> {
>@@ -242,9 +243,8 @@ void MediaControlPanelElement::makeOpaque()
> 
>     m_opaque = true;
> 
>-    // FIXME(BUG 79347): The display:none property should be toggled below only
>-    // when display logic is introduced.
>-    // show();
>+    if (m_isDisplayed)
>+        show();
> }
> 
> void MediaControlPanelElement::makeTransparent()
>@@ -258,9 +258,7 @@ void MediaControlPanelElement::makeTransparent()
> 
>     m_opaque = false;
> 
>-    // FIXME(BUG 79347): The display:none property should be toggled below
>-    // (through the timer start) when display logic is introduced.
>-    // startTimer();
>+    startTimer();
> }
> 
> void MediaControlPanelElement::defaultEventHandler(Event* event)
>@@ -293,6 +291,11 @@ void MediaControlPanelElement::setCanBeDragged(bool canBeDragged)
>         endDrag();
> }
> 
>+void MediaControlPanelElement::setIsDisplayed(bool isDisplayed)
>+{
>+    m_isDisplayed = isDisplayed;
>+}
>+
> // ----------------------------
> 
> inline MediaControlTimelineContainerElement::MediaControlTimelineContainerElement(Document* document)
>diff --git a/Source/WebCore/html/shadow/MediaControlElements.h b/Source/WebCore/html/shadow/MediaControlElements.h
>index efa69a58c5a6e2afc66bb15a23b338571eaee497..1bdb01dd8a4ef5014ca4a8e32074f9dc52fa704d 100644
>--- a/Source/WebCore/html/shadow/MediaControlElements.h
>+++ b/Source/WebCore/html/shadow/MediaControlElements.h
>@@ -109,6 +109,8 @@ public:
>     static PassRefPtr<MediaControlPanelElement> create(Document*);
> 
>     void setCanBeDragged(bool);
>+    void setIsDisplayed(bool);
>+
>     void resetPosition();
>     void makeOpaque();
>     void makeTransparent();
>@@ -131,6 +133,7 @@ private:
> 
>     bool m_canBeDragged;
>     bool m_isBeingDragged;
>+    bool m_isDisplayed;
>     bool m_opaque;
>     LayoutPoint m_dragStartPosition;
>     LayoutPoint m_dragStartEventLocation;
>diff --git a/Source/WebCore/html/shadow/MediaControlRootElement.cpp b/Source/WebCore/html/shadow/MediaControlRootElement.cpp
>index 4505fde80e94ab23af38a9118bd17f3c97621d34..a9ef86b24879b151d4f91250fd05413731e7b755 100644
>--- a/Source/WebCore/html/shadow/MediaControlRootElement.cpp
>+++ b/Source/WebCore/html/shadow/MediaControlRootElement.cpp
>@@ -282,11 +282,13 @@ void MediaControlRootElement::setMediaController(MediaControllerInterface* contr
> 
> void MediaControlRootElement::show()
> {
>+    m_panel->setIsDisplayed(true);
>     m_panel->show();
> }
> 
> void MediaControlRootElement::hide()
> {
>+    m_panel->setIsDisplayed(false);
>     m_panel->hide();
> }
> 
>diff --git a/Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp b/Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp
>index bed2abb8513de3f32977c3baffdc68f8e690ae8d..86ea489b5be8a4ada6a8c29eedd2a9eddc1f3a78 100644
>--- a/Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp
>+++ b/Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp
>@@ -164,11 +164,13 @@ void MediaControlRootElementChromium::setMediaController(MediaControllerInterfac
> 
> void MediaControlRootElementChromium::show()
> {
>+    m_panel->setIsDisplayed(true);
>     m_panel->show();
> }
> 
> void MediaControlRootElementChromium::hide()
> {
>+    m_panel->setIsDisplayed(false);
>     m_panel->hide();
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index f083c89b3b2a47ffbe42e26060edc99022a327ca..0336bc78b6575dbbbc1195280f33fd75e5905d57 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2012-04-10  Victor Carbune  <vcarbune@adobe.com>
>+
>+        Extra display logic for the media control panel element
>+        https://bugs.webkit.org/show_bug.cgi?id=82476
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added test to ensure that controls are not displayed when
>+        the controls attribute is not set.
>+
>+        * media/video-controls-toggling-expected.txt: Added.
>+        * media/video-controls-toggling.html: Added.
>+
> 2012-04-05  Zoltan Arvai  <zarvai@inf.u-szeged.hu>
> 
>         [Qt][WK2] Skip tests that still failing after implementing layoutTestController.setPageVisibility()
>diff --git a/LayoutTests/media/video-controls-toggling-expected.txt b/LayoutTests/media/video-controls-toggling-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..ec950f7e4bc4743068df8b01fcc2cc01d8ff13f0
>--- /dev/null
>+++ b/LayoutTests/media/video-controls-toggling-expected.txt
>@@ -0,0 +1,28 @@
>+Tests that showing / hiding video controls uses the sets the display:none property
>+
>+** Playing the video **
>+RUN(video.play())
>+
>+** Move mouse somewhere over the panel **
>+RUN(eventSender.mouseMoveTo(muteButtonCoordinates[0], muteButtonCoordinates[1]))
>+
>+** Test that controls are shown when controls attribute is present **
>+EXPECTED (panel.style['display'] != 'none') OK
>+
>+** Move mouse outside the video **
>+RUN(eventSender.mouseMoveTo(video.offsetLeft, video.offsetTop + 2 * video.offsetHeight))
>+
>+** The controls should have the display property set to none
>+EXPECTED (panel.style['display'] == 'none') OK
>+
>+** Remove controls attribute**
>+RUN(video.removeAttribute('controls'))
>+
>+** Move mouse back over the panel **
>+RUN(eventSender.mouseMoveTo(muteButtonCoordinates[0], muteButtonCoordinates[1]))
>+
>+** Video controls should not be shown **
>+EXPECTED (panel.style['display'] == 'none') OK
>+
>+END OF TEST
>+
>diff --git a/LayoutTests/media/video-controls-toggling.html b/LayoutTests/media/video-controls-toggling.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..3c22060082c235f881e8eaf0ef7d0a562dade13f
>--- /dev/null
>+++ b/LayoutTests/media/video-controls-toggling.html
>@@ -0,0 +1,82 @@
>+<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 fadeoutTime = 1000;
>+
>+        function init()
>+        {
>+            video = document.getElementsByTagName("video")[0];
>+            video.src = findMediaFile("video", "content/test");
>+
>+            consoleWrite("");
>+            consoleWrite("** Playing the video **");
>+            run("video.play()");
>+        }
>+
>+        function test()
>+        {
>+            panel = mediaControlsElement(internals.shadowRoot(video).firstChild, "-webkit-media-controls-panel");
>+
>+            if (window.eventSender) {
>+                try {
>+                    muteButtonCoordinates = mediaControlsButtonCoordinates(video, "mute-button");
>+                } catch (exception) {
>+                    layoutTestController.notifyDone();
>+                    return;
>+                }
>+
>+                consoleWrite("");
>+                consoleWrite("** Move mouse somewhere over the panel **");
>+                run("eventSender.mouseMoveTo(muteButtonCoordinates[0], muteButtonCoordinates[1])");
>+            }
>+
>+            consoleWrite("");
>+            consoleWrite("** Test that controls are shown when controls attribute is present **");
>+            testExpected("panel.style['display']", 'none', "!=");
>+
>+            if (window.eventSender) {
>+                consoleWrite("");
>+                consoleWrite("** Move mouse outside the video **");
>+                run("eventSender.mouseMoveTo(video.offsetLeft, video.offsetTop + 2 * video.offsetHeight)");
>+            }
>+
>+            setTimeout(continueTest, fadeoutTime);
>+        }
>+
>+        function continueTest()
>+        {
>+            consoleWrite("");
>+            consoleWrite("** The controls should have the display property set to none");
>+            testExpected("panel.style['display']", 'none', "==");
>+
>+            consoleWrite("");
>+            consoleWrite("** Remove controls attribute**");
>+            run("video.removeAttribute('controls')");
>+
>+            consoleWrite("");
>+            consoleWrite("** Move mouse back over the panel **");
>+            run("eventSender.mouseMoveTo(muteButtonCoordinates[0], muteButtonCoordinates[1])");
>+
>+            consoleWrite("");
>+            consoleWrite("** Video controls should not be shown **");
>+            testExpected("panel.style['display']", 'none', "==");
>+
>+            consoleWrite("");
>+
>+            endTest();
>+        }
>+
>+    </script>
>+</head>
>+<body onload="init()">
>+    Tests that showing / hiding video controls uses the sets the display:none property<br>
>+    <video onplay="test()" controls></video>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 82476: 136017 | 136291 | 136293 | 136458 | 136489 | 138540