Attachment 136017 Details for Bug 82476 – Fixed display bug and removed timer

Fixed display bug and removed timer

bug-82476-20120406170231.patch (text/plain), 13.94 KB, created by Victor Carbune on 2012-04-06 07:02:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Victor Carbune

Created: 2012-04-06 07:02:33 PDT

Size: 13.94 KB

patch

obsolete

>Subversion Revision: 113315
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 31553d26d4054a8499da4cbb34b27d8aa366e4a7..1139fa3455d5dcf71dcba9e2722e71b57f316355 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,38 @@
>+2012-04-06  Victor Carbune  <vcarbune@adobe.com>
>+
>+        The media controls panel needs should toggle the display:none property
>+        correctly and efficiently when the fade transition ends.
>+
>+        The timer has been removed and the dispatchEvent method is used
>+        in order to catch the transition end event.
>+
>+        https://bugs.webkit.org/show_bug.cgi?id=82476
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: media/video-controls-toggling.html
>+
>+        * html/shadow/MediaControlElements.cpp:
>+        (WebCore::MediaControlPanelElement::MediaControlPanelElement):
>+        Removed timer.
>+        (WebCore::MediaControlPanelElement::makeOpaque): Used boolean variable
>+        before calling show on the panel element.
>+        (WebCore::MediaControlPanelElement::makeTransparent): Removed the use of
>+        a timer.
>+        (WebCore):
>+        (WebCore::MediaControlPanelElement::setDisplayed): Method used to
>+        inform the panel element whether it should ever be displayed or not.
>+        (WebCore::MediaControlPanelElement::dispatchEvent): Overriding dispatchEvent
>+        method in order to hide the panel element when transition end event is fired.
>+        * html/shadow/MediaControlElements.h:
>+        (MediaControlPanelElement): Removed timer functions.
>+        * html/shadow/MediaControlRootElement.cpp:
>+        (WebCore::MediaControlRootElement::show): Inform the panel that it is visible.
>+        (WebCore::MediaControlRootElement::hide): Inform the panel that it is not visible.
>+        * html/shadow/MediaControlRootElementChromium.cpp:
>+        (WebCore::MediaControlRootElementChromium::show): Same.
>+        (WebCore::MediaControlRootElementChromium::hide): Same.
>+
> 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..c4f2d2d613b56c93d0fb100fd3a24e10f25c443d 100644
>--- a/Source/WebCore/html/shadow/MediaControlElements.cpp
>+++ b/Source/WebCore/html/shadow/MediaControlElements.cpp
>@@ -54,6 +54,7 @@
> #include "ScriptController.h"
> #include "Settings.h"
> #include "Text.h"
>+#include "WebKitTransitionEvent.h"
> 
> namespace WebCore {
> 
>@@ -109,7 +110,7 @@ inline MediaControlPanelElement::MediaControlPanelElement(Document* document)
>     , m_canBeDragged(false)
>     , m_isBeingDragged(false)
>     , m_opaque(true)
>-    , m_transitionTimer(this, &MediaControlPanelElement::transitionTimerFired)
>+    , m_displayed(false)
> {
> }
> 
>@@ -176,32 +177,6 @@ void MediaControlPanelElement::endDrag()
>     frame->eventHandler()->setCapturingMouseEventsNode(0);
> }
> 
>-void MediaControlPanelElement::startTimer()
>-{
>-    stopTimer();
>-
>-    // The timer is required to set the property display:'none' on the panel,
>-    // such that captions are correctly displayed at the bottom of the video
>-    // at the end of the fadeout transition.
>-    double duration = document()->page() ? document()->page()->theme()->mediaControlsFadeOutDuration() : 0;
>-    m_transitionTimer.startOneShot(duration);
>-}
>-
>-void MediaControlPanelElement::stopTimer()
>-{
>-    if (m_transitionTimer.isActive())
>-        m_transitionTimer.stop();
>-}
>-
>-
>-void MediaControlPanelElement::transitionTimerFired(Timer<MediaControlPanelElement>*)
>-{
>-    if (!m_opaque)
>-        hide();
>-
>-    stopTimer();
>-}
>-
> void MediaControlPanelElement::setPosition(const LayoutPoint& position)
> {
>     double left = position.x();
>@@ -242,9 +217,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_displayed)
>+        show();
> }
> 
> void MediaControlPanelElement::makeTransparent()
>@@ -257,10 +231,23 @@ void MediaControlPanelElement::makeTransparent()
>     setInlineStyleProperty(CSSPropertyOpacity, 0.0, CSSPrimitiveValue::CSS_NUMBER);
> 
>     m_opaque = false;
>+}
>+
>+void MediaControlPanelElement::setDisplayed(bool displayed)
>+{
>+    m_displayed = displayed;
>+}
>+
>+bool MediaControlPanelElement::dispatchEvent(PassRefPtr<Event> event)
>+{
>+    if (event->type() != eventNames().webkitTransitionEndEvent)
>+        return HTMLDivElement::dispatchEvent(event);
>+
>+    WebKitTransitionEvent* transitionEvent = static_cast<WebKitTransitionEvent*>(event.get());
>+    if (!m_opaque && transitionEvent->propertyName() == getPropertyName(CSSPropertyOpacity))
>+        hide();
> 
>-    // FIXME(BUG 79347): The display:none property should be toggled below
>-    // (through the timer start) when display logic is introduced.
>-    // startTimer();
>+    return HTMLDivElement::dispatchEvent(event);
> }
> 
> void MediaControlPanelElement::defaultEventHandler(Event* event)
>diff --git a/Source/WebCore/html/shadow/MediaControlElements.h b/Source/WebCore/html/shadow/MediaControlElements.h
>index efa69a58c5a6e2afc66bb15a23b338571eaee497..c5d988f4ae47680299d4474218f40ed1953cc3fc 100644
>--- a/Source/WebCore/html/shadow/MediaControlElements.h
>+++ b/Source/WebCore/html/shadow/MediaControlElements.h
>@@ -112,6 +112,9 @@ public:
>     void resetPosition();
>     void makeOpaque();
>     void makeTransparent();
>+    void setDisplayed(bool);
>+
>+    virtual bool dispatchEvent(PassRefPtr<Event>);
> 
> private:
>     MediaControlPanelElement(Document*);
>@@ -123,19 +126,14 @@ private:
>     void continueDrag(const LayoutPoint& eventLocation);
>     void endDrag();
> 
>-    void startTimer();
>-    void stopTimer();
>-    void transitionTimerFired(Timer<MediaControlPanelElement>*);
>-
>     void setPosition(const LayoutPoint&);
> 
>     bool m_canBeDragged;
>     bool m_isBeingDragged;
>     bool m_opaque;
>+    bool m_displayed;
>     LayoutPoint m_dragStartPosition;
>     LayoutPoint m_dragStartEventLocation;
>-
>-    Timer<MediaControlPanelElement> m_transitionTimer;
> };
> 
> // ----------------------------
>diff --git a/Source/WebCore/html/shadow/MediaControlRootElement.cpp b/Source/WebCore/html/shadow/MediaControlRootElement.cpp
>index 4505fde80e94ab23af38a9118bd17f3c97621d34..ad5f4774670a8f142180a5d2ac57ecb2b93eb430 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->setDisplayed(true);
>     m_panel->show();
> }
> 
> void MediaControlRootElement::hide()
> {
>+    m_panel->setDisplayed(false);
>     m_panel->hide();
> }
> 
>diff --git a/Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp b/Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp
>index bed2abb8513de3f32977c3baffdc68f8e690ae8d..390efaca5e7aa658636c2997afb36eb57f500354 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->setDisplayed(true);
>     m_panel->show();
> }
> 
> void MediaControlRootElementChromium::hide()
> {
>+    m_panel->setDisplayed(false);
>     m_panel->hide();
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index f083c89b3b2a47ffbe42e26060edc99022a327ca..507f880f75493d4bc1ee4dab2d631cb80e30c6c1 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,20 @@
>+2012-04-06  Victor Carbune  <vcarbune@adobe.com>
>+
>+        Added test to ensure that controls are not displayed when
>+        the controls attribute is not set.
>+
>+        https://bugs.webkit.org/show_bug.cgi?id=82476
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * media/video-controls-toggling-expected.txt: Added.
>+        * media/video-controls-toggling.html: Added.
>+        * media/video-test.js: Minor change to the waitForEvent method
>+        in order to be able wait for events on other elements than the media
>+        element as well.
>+        (waitForEvent._eventCallback):
>+        (waitForEvent):
>+
> 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..a76248e80e7e7e051015451cd12cece63bd4ec5a
>--- /dev/null
>+++ b/LayoutTests/media/video-controls-toggling-expected.txt
>@@ -0,0 +1,29 @@
>+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))
>+EVENT(webkitTransitionEnd)
>+
>+** 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..ba6004ff621c49b565337df6901a3eb8eb7e922e
>--- /dev/null
>+++ b/LayoutTests/media/video-controls-toggling.html
>@@ -0,0 +1,81 @@
>+<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;
>+
>+        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)");
>+            }
>+
>+            waitForEvent("webkitTransitionEnd", continueTest, false, true, panel);
>+        }
>+
>+        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>
>diff --git a/LayoutTests/media/video-test.js b/LayoutTests/media/video-test.js
>index ebf0f43540080457eb34e705235648b9235271a1..03795f1ed423081cc13522d371028b332b1234b3 100644
>--- a/LayoutTests/media/video-test.js
>+++ b/LayoutTests/media/video-test.js
>@@ -137,12 +137,15 @@ function waitForEventAndEnd(eventName, funcString)
>     waitForEvent(eventName, funcString, true)
> }
> 
>-function waitForEvent(eventName, func, endit, oneTimeOnly)
>+function waitForEvent(eventName, func, endit, oneTimeOnly, element)
> {
>     function _eventCallback(event)
>     {
>+        if (!element)
>+            element = mediaElement;
>+
>         if (oneTimeOnly)
>-            mediaElement.removeEventListener(eventName, _eventCallback, true);
>+            element.removeEventListener(eventName, _eventCallback, true);
> 
>         consoleWrite("EVENT(" + eventName + ")");
> 
>@@ -153,7 +156,7 @@ function waitForEvent(eventName, func, endit, oneTimeOnly)
>             endTest();
>     }
> 
>-    mediaElement.addEventListener(eventName, _eventCallback, true);
>+    element.addEventListener(eventName, _eventCallback, true);
> }
> 
> function waitForEventTestAndEnd(eventName, testFuncString)

Actions: View

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