WebCore:
authorpdherbemont@apple.com <pdherbemont@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Jul 2009 01:04:07 +0000 (01:04 +0000)
committerpdherbemont@apple.com <pdherbemont@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Jul 2009 01:04:07 +0000 (01:04 +0000)
2009-07-09  Pierre d'Herbemont  <pdherbemont@apple.com>

        Reviewed by Simon Fraser.

        Full page zoom breaks remaining and elapsed time display in the <video> controller.
        https://bugs.webkit.org/show_bug.cgi?id=27123

        We are changing the size of the time remaining and time elapsed field, to
        automatically hide them, when the controller is too short.

        Because we toggle the size between 0 and the previous value of the
        controller, we miss any width change that may occur during full page zoom,
        and we fail to restore a correct width.

        This change fixes that problem by using a cloned style on which we
        set the width to 0, and restoring the previous style when going back to
        the normal width.

        We take care about properly using the cloned style or the pseudo style,
        by overriding styleForElement().

        * rendering/MediaControlElements.cpp:
        (WebCore::MediaControlElement::styleForElement):
        (WebCore::MediaControlElement::attach):
        (WebCore::MediaControlElement::updateStyle):
        (WebCore::MediaControlInputElement::styleForElement):
        (WebCore::MediaControlInputElement::attach):
        (WebCore::MediaControlInputElement::updateStyle):
        (WebCore::MediaControlTimeDisplayElement::MediaControlTimeDisplayElement):
        (WebCore::MediaControlTimeDisplayElement::styleForElement):
        (WebCore::MediaControlTimeDisplayElement::setVisible):
        * rendering/MediaControlElements.h:
        * rendering/RenderMedia.cpp:
        (WebCore::RenderMedia::shouldShowTimeDisplayControls): Make
        sure we take in account the zoom level when deciding if we should
        hide the ellapsed and remaining time.

LayoutTests:

2009-07-09  Pierre d'Herbemont  <pdherbemont@apple.com>

        Reviewed by Simon Fraser.

        Full page zoom breaks remaining and elapsed time display in the &lt;video&gt; controller.
        https://bugs.webkit.org/show_bug.cgi?id=27123

        * media/video-zoom-controls.html: Remove duplicated code.
        * platform/mac-leopard/media/video-zoom-controls-expected.txt: Copied from LayoutTests/platform/mac/media/video-zoom-controls-expected.txt.
        * platform/mac-snowleopard/Skipped:
        * platform/mac/media/video-zoom-controls-expected.checksum: Removed. The test is not reliable for pixel test.
        * platform/mac/media/video-zoom-controls-expected.png: Removed.
        * platform/mac/media/video-zoom-controls-expected.txt: Updated

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

LayoutTests/ChangeLog
LayoutTests/media/video-zoom-controls.html
LayoutTests/platform/mac-leopard/media/video-zoom-controls-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac-snowleopard/Skipped
LayoutTests/platform/mac/media/video-zoom-controls-expected.checksum [deleted file]
LayoutTests/platform/mac/media/video-zoom-controls-expected.png [deleted file]
LayoutTests/platform/mac/media/video-zoom-controls-expected.txt
WebCore/ChangeLog
WebCore/rendering/MediaControlElements.cpp
WebCore/rendering/MediaControlElements.h
WebCore/rendering/RenderMedia.cpp

index 0f93989..e96fe83 100644 (file)
@@ -1,3 +1,18 @@
+2009-07-09  Pierre d'Herbemont  <pdherbemont@apple.com>
+
+        Reviewed by Simon Fraser.
+
+        Full page zoom breaks remaining and elapsed time display in the
+        <video> controller.
+        https://bugs.webkit.org/show_bug.cgi?id=27123
+
+        * media/video-zoom-controls.html: Remove duplicated code.
+        * platform/mac-leopard/media/video-zoom-controls-expected.txt: Copied from LayoutTests/platform/mac/media/video-zoom-controls-expected.txt.
+        * platform/mac-snowleopard/Skipped:
+        * platform/mac/media/video-zoom-controls-expected.checksum: Removed. The test is not reliable for pixel test.
+        * platform/mac/media/video-zoom-controls-expected.png: Removed.
+        * platform/mac/media/video-zoom-controls-expected.txt: Updated
+
 2009-07-09  Chris Fleizach  <cfleizach@apple.com>
 
         Reviewed by Darin Adler.
index cd62f0a..7ec51cc 100644 (file)
         -webkit-transform: rotate(10deg);
       }
     </style>
-    <script>
-        function init()
-        {
-            var totalCount = document.getElementsByTagName('video').length;
-            var count = totalCount;
-            document.addEventListener("canplaythrough", function () {
-                if (!--count) {
-                    if (window.layoutTestController)
-                        setTimeout(function() { layoutTestController.notifyDone(); }, totalCount * 150);
-                }
-            }, true);
-
-            if (window.layoutTestController) {
-                layoutTestController.waitUntilDone();
-                setTimeout(function() { 
-                    document.body.appendChild(document.createTextNode('FAIL')); 
-                    if (window.layoutTestController)
-                        layoutTestController.notifyDone();
-                }, 1500);
-            }
-        }
-    </script>
-
+    <script src="video-paint-test.js"></script>
 </head>
 <body onload="init()">
     <p>Zoomed video with controls.</p>
diff --git a/LayoutTests/platform/mac-leopard/media/video-zoom-controls-expected.txt b/LayoutTests/platform/mac-leopard/media/video-zoom-controls-expected.txt
new file mode 100644 (file)
index 0000000..62fa55b
--- /dev/null
@@ -0,0 +1,34 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (12,12) size 776x543
+      RenderBlock {P} at (0,0) size 776x28
+        RenderText {#text} at (0,0) size 280x28
+          text run at (0,0) width 280: "Zoomed video with controls."
+layer at (57,85) size 240x180
+  RenderVideo {VIDEO} at (45,73) size 240x180
+layer at (57,85) size 240x180
+  RenderBlock (relative positioned) {DIV} at (0,0) size 240x180
+layer at (57,241) size 240x24
+  RenderFlexibleBox (positioned) {DIV} at (0,156) size 240x24
+    RenderButton {INPUT} at (0,0) size 24x24
+    RenderButton {INPUT} at (24,0) size 24x24
+    RenderFlexibleBox {DIV} at (48,0) size 144x24
+      RenderSlider {INPUT} at (0,0) size 144x24
+        RenderBlock {DIV} at (3,1) size 19x21
+    RenderButton {INPUT} at (192,0) size 24x24
+    RenderButton {INPUT} at (216,0) size 24x24
+layer at (57,310) size 240x180
+  RenderVideo {VIDEO} at (45,298) size 240x180
+layer at (57,310) size 240x180
+  RenderBlock (relative positioned) {DIV} at (0,0) size 240x180
+layer at (57,466) size 240x24
+  RenderFlexibleBox (positioned) {DIV} at (0,156) size 240x24
+    RenderButton {INPUT} at (0,0) size 24x24
+    RenderButton {INPUT} at (24,0) size 24x24
+    RenderFlexibleBox {DIV} at (48,0) size 144x24
+      RenderSlider {INPUT} at (0,0) size 144x24
+        RenderBlock {DIV} at (3,1) size 19x21
+    RenderButton {INPUT} at (192,0) size 24x24
+    RenderButton {INPUT} at (216,0) size 24x24
index a2a98c5..3868757 100644 (file)
@@ -37,4 +37,3 @@ media/video-controls-transformed.html
 media/video-controls-visible-audio-only.html
 media/video-controls-zoomed.html
 media/video-display-toggle.html
-media/video-zoom-controls.html
diff --git a/LayoutTests/platform/mac/media/video-zoom-controls-expected.checksum b/LayoutTests/platform/mac/media/video-zoom-controls-expected.checksum
deleted file mode 100644 (file)
index b5d815f..0000000
+++ /dev/null
@@ -1 +0,0 @@
-1cb71aacb185b3ec1d974a74f382c263
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/media/video-zoom-controls-expected.png b/LayoutTests/platform/mac/media/video-zoom-controls-expected.png
deleted file mode 100644 (file)
index 4250c5e..0000000
Binary files a/LayoutTests/platform/mac/media/video-zoom-controls-expected.png and /dev/null differ
index 62fa55b..be28ef6 100644 (file)
@@ -10,25 +10,23 @@ layer at (57,85) size 240x180
   RenderVideo {VIDEO} at (45,73) size 240x180
 layer at (57,85) size 240x180
   RenderBlock (relative positioned) {DIV} at (0,0) size 240x180
-layer at (57,241) size 240x24
-  RenderFlexibleBox (positioned) {DIV} at (0,156) size 240x24
-    RenderButton {INPUT} at (0,0) size 24x24
-    RenderButton {INPUT} at (24,0) size 24x24
-    RenderFlexibleBox {DIV} at (48,0) size 144x24
-      RenderSlider {INPUT} at (0,0) size 144x24
-        RenderBlock {DIV} at (3,1) size 19x21
-    RenderButton {INPUT} at (192,0) size 24x24
-    RenderButton {INPUT} at (216,0) size 24x24
+layer at (57,228) size 240x37
+  RenderFlexibleBox (positioned) {DIV} at (0,143) size 240x37
+    RenderButton {INPUT} at (10,6) size 24x24
+    RenderButton {INPUT} at (206,6) size 24x24
+    RenderButton {INPUT} at (54,6) size 24x24
+    RenderFlexibleBox {DIV} at (88,9) size 108x19
+      RenderSlider {INPUT} at (0,0) size 108x19
+        RenderBlock {DIV} at (0,9) size 0x0
 layer at (57,310) size 240x180
   RenderVideo {VIDEO} at (45,298) size 240x180
 layer at (57,310) size 240x180
   RenderBlock (relative positioned) {DIV} at (0,0) size 240x180
-layer at (57,466) size 240x24
-  RenderFlexibleBox (positioned) {DIV} at (0,156) size 240x24
-    RenderButton {INPUT} at (0,0) size 24x24
-    RenderButton {INPUT} at (24,0) size 24x24
-    RenderFlexibleBox {DIV} at (48,0) size 144x24
-      RenderSlider {INPUT} at (0,0) size 144x24
-        RenderBlock {DIV} at (3,1) size 19x21
-    RenderButton {INPUT} at (192,0) size 24x24
-    RenderButton {INPUT} at (216,0) size 24x24
+layer at (57,453) size 240x37
+  RenderFlexibleBox (positioned) {DIV} at (0,143) size 240x37
+    RenderButton {INPUT} at (10,6) size 24x24
+    RenderButton {INPUT} at (206,6) size 24x24
+    RenderButton {INPUT} at (54,6) size 24x24
+    RenderFlexibleBox {DIV} at (88,9) size 108x19
+      RenderSlider {INPUT} at (0,0) size 108x19
+        RenderBlock {DIV} at (0,9) size 0x0
index 979fe7e..0d037cf 100644 (file)
@@ -1,3 +1,40 @@
+2009-07-09  Pierre d'Herbemont  <pdherbemont@apple.com>
+
+        Reviewed by Simon Fraser.
+
+        Full page zoom breaks remaining and elapsed time display in the <video> controller.
+        https://bugs.webkit.org/show_bug.cgi?id=27123
+
+        We are changing the size of the time remaining and time elapsed field, to
+        automatically hide them, when the controller is too short.
+
+        Because we toggle the size between 0 and the previous value of the
+        controller, we miss any width change that may occur during full page zoom,
+        and we fail to restore a correct width.
+
+        This change fixes that problem by using a cloned style on which we
+        set the width to 0, and restoring the previous style when going back to
+        the normal width.
+
+        We take care about properly using the cloned style or the pseudo style,
+        by overriding styleForElement().
+
+        * rendering/MediaControlElements.cpp:
+        (WebCore::MediaControlElement::styleForElement):
+        (WebCore::MediaControlElement::attach):
+        (WebCore::MediaControlElement::updateStyle):
+        (WebCore::MediaControlInputElement::styleForElement):
+        (WebCore::MediaControlInputElement::attach):
+        (WebCore::MediaControlInputElement::updateStyle):
+        (WebCore::MediaControlTimeDisplayElement::MediaControlTimeDisplayElement):
+        (WebCore::MediaControlTimeDisplayElement::styleForElement):
+        (WebCore::MediaControlTimeDisplayElement::setVisible):
+        * rendering/MediaControlElements.h:
+        * rendering/RenderMedia.cpp:
+        (WebCore::RenderMedia::shouldShowTimeDisplayControls): Make sure
+        we take in account the zoom level when deciding if we should hide the 
+        ellapsed and remaining time.
+
 2009-07-09  Michael Nordman  <michaeln@google.com>
 
         Reviewed by Darin Adler.
index 8c25db4..8339586 100644 (file)
@@ -98,7 +98,7 @@ void MediaControlElement::update()
     updateStyle();
 }
 
-RenderStyle* MediaControlElement::styleForElement()
+PassRefPtr<RenderStyle> MediaControlElement::styleForElement()
 {
     RenderStyle* style = m_mediaElement->renderer()->getCachedPseudoStyle(m_pseudoStyleId);
     if (!style)
@@ -119,16 +119,16 @@ bool MediaControlElement::rendererIsNeeded(RenderStyle* style)
     
 void MediaControlElement::attach()
 {
-    RenderStyle* style = styleForElement();
+    RefPtr<RenderStyle> style = styleForElement();
     if (!style)
         return;
-    bool needsRenderer = rendererIsNeeded(style);
+    bool needsRenderer = rendererIsNeeded(style.get());
     if (!needsRenderer)
         return;
-    RenderObject* renderer = createRenderer(m_mediaElement->renderer()->renderArena(), style);
+    RenderObject* renderer = createRenderer(m_mediaElement->renderer()->renderArena(), style.get());
     if (!renderer)
         return;
-    renderer->setStyle(style);
+    renderer->setStyle(style.get());
     setRenderer(renderer);
     if (parent() && parent()->renderer()) {
         // Find next sibling with a renderer to determine where to insert.
@@ -145,21 +145,21 @@ void MediaControlElement::updateStyle()
     if (!m_mediaElement || !m_mediaElement->renderer())
         return;
 
-    RenderStyle* style = styleForElement();
+    RefPtr<RenderStyle> style = styleForElement();
     if (!style)
         return;
 
-    bool needsRenderer = rendererIsNeeded(style) && parent() && parent()->renderer();
+    bool needsRenderer = rendererIsNeeded(style.get()) && parent() && parent()->renderer();
     if (renderer() && !needsRenderer)
         detach();
     else if (!renderer() && needsRenderer)
         attach();
     else if (renderer()) {
-        renderer()->setStyle(style);
+        renderer()->setStyle(style.get());
 
         // Make sure that if there is any innerText renderer, it is updated as well.
         if (firstChild() && firstChild()->renderer())
-            firstChild()->renderer()->setStyle(style);
+            firstChild()->renderer()->setStyle(style.get());
     }
 }
 
@@ -258,7 +258,7 @@ void MediaControlInputElement::update()
     updateStyle();
 }
 
-RenderStyle* MediaControlInputElement::styleForElement()
+PassRefPtr<RenderStyle> MediaControlInputElement::styleForElement()
 {
     return m_mediaElement->renderer()->getCachedPseudoStyle(m_pseudoStyleId);
 }
@@ -270,17 +270,17 @@ bool MediaControlInputElement::rendererIsNeeded(RenderStyle* style)
 
 void MediaControlInputElement::attach()
 {
-    RenderStyle* style = styleForElement();
+    RefPtr<RenderStyle> style = styleForElement();
     if (!style)
         return;
     
-    bool needsRenderer = rendererIsNeeded(style);
+    bool needsRenderer = rendererIsNeeded(style.get());
     if (!needsRenderer)
         return;
-    RenderObject* renderer = createRenderer(m_mediaElement->renderer()->renderArena(), style);
+    RenderObject* renderer = createRenderer(m_mediaElement->renderer()->renderArena(), style.get());
     if (!renderer)
         return;
-    renderer->setStyle(style);
+    renderer->setStyle(style.get());
     setRenderer(renderer);
     if (parent() && parent()->renderer()) {
         // Find next sibling with a renderer to determine where to insert.
@@ -297,17 +297,17 @@ void MediaControlInputElement::updateStyle()
     if (!m_mediaElement || !m_mediaElement->renderer())
         return;
     
-    RenderStyle* style = styleForElement();
+    RefPtr<RenderStyle> style = styleForElement();
     if (!style)
         return;
     
-    bool needsRenderer = rendererIsNeeded(style) && parent() && parent()->renderer();
+    bool needsRenderer = rendererIsNeeded(style.get()) && parent() && parent()->renderer();
     if (renderer() && !needsRenderer)
         detach();
     else if (!renderer() && needsRenderer)
         attach();
     else if (renderer())
-        renderer()->setStyle(style);
+        renderer()->setStyle(style.get());
 }
     
 bool MediaControlInputElement::hitTest(const IntPoint& absPoint)
@@ -534,23 +534,32 @@ bool MediaControlFullscreenButtonElement::rendererIsNeeded(RenderStyle* style)
 
 MediaControlTimeDisplayElement::MediaControlTimeDisplayElement(Document* doc, PseudoId pseudo, HTMLMediaElement* element)
     : MediaControlElement(doc, pseudo, element)
-    , m_cachedWidth(Length(0, Fixed))
+    , m_isVisible(true)
 {
 }
 
+PassRefPtr<RenderStyle> MediaControlTimeDisplayElement::styleForElement()
+{
+    RefPtr<RenderStyle> style = MediaControlElement::styleForElement();
+    if (!m_isVisible) {
+        style = RenderStyle::clone(style.get());
+        style->setWidth(Length(0, Fixed));
+    }
+    return style;
+}
+
 void MediaControlTimeDisplayElement::setVisible(bool visible)
 {
+    // This function is used during the RenderMedia::layout()
+    // call, where we cannot change the renderer at this time.
     if (!renderer() || !renderer()->style())
         return;
 
-    if (!m_cachedWidth.value()) {
-        RenderStyle* style = m_mediaElement->renderer()->getCachedPseudoStyle(m_pseudoStyleId);
-        if (!style)
-            return;
-        m_cachedWidth = style->width();
-    }
-
-    renderer()->style()->setWidth(visible ? m_cachedWidth : Length(0, Fixed));
+    if (visible == m_isVisible)
+        return;
+    m_isVisible = visible;
+    RefPtr<RenderStyle> style = styleForElement();
+    renderer()->setStyle(style.get());
 }
 
 
index 33dd76a..92e1af1 100644 (file)
@@ -84,7 +84,7 @@ public:
     virtual void attach();
     virtual bool rendererIsNeeded(RenderStyle*);
 
-    RenderStyle* styleForElement();
+    virtual PassRefPtr<RenderStyle> styleForElement();
     void attachToParent(Element*);
     void update();
     virtual void updateStyle();
@@ -122,7 +122,7 @@ public:
     virtual void attach();
     virtual bool rendererIsNeeded(RenderStyle*);
 
-    RenderStyle* styleForElement();
+    virtual PassRefPtr<RenderStyle> styleForElement();
     void attachToParent(Element*);
     void update();
     void updateStyle();
@@ -214,8 +214,10 @@ class MediaControlTimeDisplayElement : public MediaControlElement {
 public:
     MediaControlTimeDisplayElement(Document*, PseudoId, HTMLMediaElement*);
     void setVisible(bool);
+    virtual PassRefPtr<RenderStyle> styleForElement();
+
 private:
-    Length m_cachedWidth;
+    bool m_isVisible;
 };
 
 // ----------------------------
index 87b1096..b87e99d 100644 (file)
@@ -543,7 +543,7 @@ bool RenderMedia::shouldShowTimeDisplayControls() const
         return false;
 
     int width = mediaElement()->renderBox()->width();
-    return width >= minWidthToDisplayTimeDisplays;
+    return width >= minWidthToDisplayTimeDisplays * style()->effectiveZoom();
 }
 
 } // namespace WebCore