Fullscreen media controls are unusable in pagination mode
authorjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 Apr 2014 17:59:26 +0000 (17:59 +0000)
committerjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 Apr 2014 17:59:26 +0000 (17:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=131705

Reviewed by Darin Adler.

Source/WebCore:
When pagination mode is enabled, the full screen media will (depending on the width of the
pagination columns) overflow its column, and hit testing will be clipped to the column. In extreme
cases, where the column width < 0.5 * media element width, the media controls will be entirely
unclickable.

Rather than making the RenderFullScreen a child of the full screen element's parent's renderer,
make it a child of the RenderView, putting it outside of the columns entirely. Always create and
insert the fullscreenRenderer's placeholder, using it as the remembered insertion point for the
fullscreen element's renderer when we exit full screen.

Drive-by fix: don't wrap the full screen element's renderer in webkitWillEnterFullScreenForElement();
it will just be re-wrapped in createRendererIfNeeded().

* dom/Document.cpp:
(WebCore::Document::webkitWillEnterFullScreenForElement): Don't wrap the full screen element's renderer.
(WebCore::Document::setFullScreenRenderer): Call setPlaceholderStyle.
* rendering/RenderFullScreen.cpp:
(WebCore::RenderFullScreenPlaceholder::willBeDestroyed): Call clearPlaceholder.
(WebCore::RenderFullScreen::wrapRenderer): Make fullscreenRenderer a child of the view().
(WebCore::RenderFullScreen::unwrapRenderer): Return the children to the parent of the placeholder().
(WebCore::RenderFullScreen::clearPlaceholder): Renamed from setPlaceholder().
(WebCore::RenderFullScreen::ensurePlaceholder): Added.
(WebCore::RenderFullScreen::setPlaceholderStyle): Renamed from createPlaceholder().
(WebCore::RenderFullScreen::setPlaceholder): Deleted.
(WebCore::RenderFullScreen::createPlaceholder): Deleted.
* rendering/RenderFullScreen.h:

LayoutTests:
* fullscreen/full-screen-no-style-sharing-expected.txt: Rebaselined.
* fullscreen/video-cursor-auto-hide.html: Corrected test to move cursor
    to the middle of the video element.

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

LayoutTests/ChangeLog
LayoutTests/fullscreen/full-screen-no-style-sharing-expected.txt
LayoutTests/fullscreen/video-cursor-auto-hide.html
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/rendering/RenderFullScreen.cpp
Source/WebCore/rendering/RenderFullScreen.h

index 506952f464febd0c83d8044c455d0e7af6ced6ca..401c56296756712d2e52b656e82b6ef36aa74942 100644 (file)
@@ -1,3 +1,14 @@
+2014-04-16  Jer Noble  <jer.noble@apple.com>
+
+        Fullscreen media controls are unusable in pagination mode
+        https://bugs.webkit.org/show_bug.cgi?id=131705
+
+        Reviewed by Darin Adler.
+
+        * fullscreen/full-screen-no-style-sharing-expected.txt: Rebaselined.
+        * fullscreen/video-cursor-auto-hide.html: Corrected test to move cursor
+            to the middle of the video element.
+
 2014-04-16  Jer Noble  <jer.noble@apple.com>
 
         [MSE] Multiple initialization segments with same codecs in tracks fail validation.
index dade4ce7b854a65a14d101997f9c5687e88f8e5f..350023082e945dbc6e85fe773f68244961393e03 100644 (file)
@@ -1,4 +1,4 @@
+  
 EVENT(webkitfullscreenchange) TEST(video2.clientWidth==document.body.clientWidth) OK
 END OF TEST
 
index f1c49a86af751811a9624e44fe2e53f3b965ab10..17f8b2e173be8bf9b0fcec9a1fa38c2d271240df 100644 (file)
@@ -6,6 +6,7 @@
     <script src="full-screen-test.js"></script>
     <script>
         var wrapper = document.getElementById('wrapper');
+        var video = document.getElementById('video');
 
         function checkForHiddenMouse()
         {
@@ -17,8 +18,8 @@
         {
             if (window.internals) {
                 internals.settings.setTimeWithoutMouseMovementBeforeHidingControls(0);
-                wrapperBox = internals.boundingBox(wrapper);
-                eventSender.mouseMoveTo(wrapperBox.left + wrapperBox.width / 2, wrapperBox.top + wrapperBox.height / 2);
+                videoBox = internals.boundingBox(video);
+                eventSender.mouseMoveTo(videoBox.left + videoBox.width / 2, videoBox.top + videoBox.height / 2);
                 testExpected('window.internals.getCurrentCursorInfo()', 'type=Pointer hotSpot=0,0');
                 setTimeout(checkForHiddenMouse, 0);
             }
index 5e135e1c59cdc0a81d76d5de8abc17bdc2b2cf8e..1439b46d8e31a3a428fb3168bd90c42f6c774a6c 100644 (file)
@@ -1,3 +1,37 @@
+2014-04-15  Jer Noble  <jer.noble@apple.com>
+
+        Fullscreen media controls are unusable in pagination mode
+        https://bugs.webkit.org/show_bug.cgi?id=131705
+
+        Reviewed by Darin Adler.
+
+        When pagination mode is enabled, the full screen media will (depending on the width of the
+        pagination columns) overflow its column, and hit testing will be clipped to the column. In extreme
+        cases, where the column width < 0.5 * media element width, the media controls will be entirely
+        unclickable.
+
+        Rather than making the RenderFullScreen a child of the full screen element's parent's renderer,
+        make it a child of the RenderView, putting it outside of the columns entirely. Always create and
+        insert the fullscreenRenderer's placeholder, using it as the remembered insertion point for the
+        fullscreen element's renderer when we exit full screen.
+
+        Drive-by fix: don't wrap the full screen element's renderer in webkitWillEnterFullScreenForElement();
+        it will just be re-wrapped in createRendererIfNeeded().
+
+        * dom/Document.cpp:
+        (WebCore::Document::webkitWillEnterFullScreenForElement): Don't wrap the full screen element's renderer.
+        (WebCore::Document::setFullScreenRenderer): Call setPlaceholderStyle.
+        * rendering/RenderFullScreen.cpp:
+        (WebCore::RenderFullScreenPlaceholder::willBeDestroyed): Call clearPlaceholder.
+        (WebCore::RenderFullScreen::wrapRenderer): Make fullscreenRenderer a child of the view().
+        (WebCore::RenderFullScreen::unwrapRenderer): Return the children to the parent of the placeholder().
+        (WebCore::RenderFullScreen::clearPlaceholder): Renamed from setPlaceholder().
+        (WebCore::RenderFullScreen::ensurePlaceholder): Added. 
+        (WebCore::RenderFullScreen::setPlaceholderStyle): Renamed from createPlaceholder().
+        (WebCore::RenderFullScreen::setPlaceholder): Deleted.
+        (WebCore::RenderFullScreen::createPlaceholder): Deleted.
+        * rendering/RenderFullScreen.h:
+
 2014-04-16  Jer Noble  <jer.noble@apple.com>
 
         [MSE] Multiple initialization segments with same codecs in tracks fail validation.
index ebb29ad24246e8263b9f53d2bd1fc5b29c14956f..fc600d59e42064425681d1c3cb0aca39f9c1b428 100644 (file)
@@ -5365,9 +5365,6 @@ void Document::webkitWillEnterFullScreenForElement(Element* element)
         m_savedPlaceholderRenderStyle = RenderStyle::clone(&renderer->style());
     }
 
-    if (m_fullScreenElement != documentElement())
-        RenderFullScreen::wrapRenderer(renderer, renderer ? renderer->parent() : nullptr, *this);
-
     m_fullScreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(true);
     
     recalcStyle(Style::Force);
@@ -5432,10 +5429,10 @@ void Document::setFullScreenRenderer(RenderFullScreen* renderer)
         return;
 
     if (renderer && m_savedPlaceholderRenderStyle) 
-        renderer->createPlaceholder(m_savedPlaceholderRenderStyle.releaseNonNull(), m_savedPlaceholderFrameRect);
+        renderer->setPlaceholderStyle(m_savedPlaceholderRenderStyle.releaseNonNull(), m_savedPlaceholderFrameRect);
     else if (renderer && m_fullScreenRenderer && m_fullScreenRenderer->placeholder()) {
         RenderBlock* placeholder = m_fullScreenRenderer->placeholder();
-        renderer->createPlaceholder(RenderStyle::clone(&placeholder->style()), placeholder->frameRect());
+        renderer->setPlaceholderStyle(RenderStyle::clone(&placeholder->style()), placeholder->frameRect());
     }
 
     if (m_fullScreenRenderer)
index 97e8efd35b450b3e28437059ac4dec79b0ba3bf8..25df572174769875f0291f4f97adc3ba3948a1fc 100644 (file)
@@ -31,6 +31,7 @@
 #include "RenderBlockFlow.h"
 #include "RenderLayer.h"
 #include "RenderLayerCompositor.h"
+#include "RenderView.h"
 
 namespace WebCore {
 
@@ -50,7 +51,7 @@ private:
 
 void RenderFullScreenPlaceholder::willBeDestroyed()
 {
-    m_owner.setPlaceholder(0);
+    m_owner.clearPlaceholder();
     RenderBlockFlow::willBeDestroyed();
 }
 
@@ -104,7 +105,7 @@ static PassRef<RenderStyle> createFullScreenStyle()
     return fullscreenStyle;
 }
 
-RenderFullScreen* RenderFullScreen::wrapRenderer(RenderObject* object, RenderElement* parent, Document& document)
+RenderElement* RenderFullScreen::wrapRenderer(RenderObject* object, RenderElement* parent, Document& document)
 {
     RenderFullScreen* fullscreenRenderer = new RenderFullScreen(document, createFullScreenStyle());
     fullscreenRenderer->initializeStyle();
@@ -118,11 +119,12 @@ RenderFullScreen* RenderFullScreen::wrapRenderer(RenderObject* object, RenderEle
         if (RenderElement* parent = object->parent()) {
             RenderBlock* containingBlock = object->containingBlock();
             ASSERT(containingBlock);
+
             // Since we are moving the |object| to a new parent |fullscreenRenderer|,
             // the line box tree underneath our |containingBlock| is not longer valid.
             containingBlock->deleteLines();
 
-            parent->addChild(fullscreenRenderer, object);
+            parent->addChild(fullscreenRenderer->ensurePlaceholder(), object);
             object->removeFromParent();
             
             // Always just do a full layout to ensure that line boxes get deleted properly.
@@ -131,16 +133,17 @@ RenderFullScreen* RenderFullScreen::wrapRenderer(RenderObject* object, RenderEle
             parent->setNeedsLayoutAndPrefWidthsRecalc();
             containingBlock->setNeedsLayoutAndPrefWidthsRecalc();
         }
+
+        object->view().addChild(fullscreenRenderer);
         fullscreenRenderer->addChild(object);
-        fullscreenRenderer->setNeedsLayoutAndPrefWidthsRecalc();
     }
     document.setFullScreenRenderer(fullscreenRenderer);
-    return fullscreenRenderer;
+    return fullscreenRenderer->ensurePlaceholder();
 }
 
 void RenderFullScreen::unwrapRenderer()
 {
-    if (parent()) {
+    if (placeholder() && placeholder()->parent()) {
         RenderObject* child;
         while ((child = firstChild())) {
             // We have to clear the override size, because as a flexbox, we
@@ -149,39 +152,38 @@ void RenderFullScreen::unwrapRenderer()
             if (child->isBox())
                 toRenderBox(child)->clearOverrideSize();
             child->removeFromParent();
-            parent()->addChild(child, this);
-            parent()->setNeedsLayoutAndPrefWidthsRecalc();
+            placeholder()->parent()->addChild(child, m_placeholder);
         }
-    }
-    if (placeholder())
+
         placeholder()->removeFromParent();
+    }
     removeFromParent();
     document().setFullScreenRenderer(0);
 }
 
-void RenderFullScreen::setPlaceholder(RenderBlock* placeholder)
+void RenderFullScreen::clearPlaceholder()
 {
-    m_placeholder = placeholder;
+    m_placeholder = nullptr;
 }
 
-void RenderFullScreen::createPlaceholder(PassRef<RenderStyle> style, const LayoutRect& frameRect)
+RenderBlock* RenderFullScreen::ensurePlaceholder()
+{
+    if (m_placeholder)
+        return m_placeholder;
+
+    m_placeholder = new RenderFullScreenPlaceholder(*this, RenderStyle::create());
+    m_placeholder->initializeStyle();
+    return m_placeholder;
+}
+
+void RenderFullScreen::setPlaceholderStyle(PassRef<RenderStyle> style, const LayoutRect& frameRect)
 {
     if (style.get().width().isAuto())
         style.get().setWidth(Length(frameRect.width(), Fixed));
     if (style.get().height().isAuto())
         style.get().setHeight(Length(frameRect.height(), Fixed));
 
-    if (m_placeholder) {
-        m_placeholder->setStyle(std::move(style));
-        return;
-    }
-
-    m_placeholder = new RenderFullScreenPlaceholder(*this, std::move(style));
-    m_placeholder->initializeStyle();
-    if (parent()) {
-        parent()->addChild(m_placeholder, this);
-        parent()->setNeedsLayoutAndPrefWidthsRecalc();
-    }
+    ensurePlaceholder()->setStyle(std::move(style));
 }
 
 }
index c8b397af1b7a0d10076af44266db4205c93b4323..e295ea68ce032f3ffaf21a095048f11ea6ef5ce3 100644 (file)
@@ -39,11 +39,12 @@ public:
     virtual bool isRenderFullScreen() const override { return true; }
     virtual const char* renderName() const override { return "RenderFullScreen"; }
 
-    void setPlaceholder(RenderBlock*);
+    void clearPlaceholder();
+    RenderBlock* ensurePlaceholder();
     RenderBlock* placeholder() { return m_placeholder; }
-    void createPlaceholder(PassRef<RenderStyle>, const LayoutRect& frameRect);
+    void setPlaceholderStyle(PassRef<RenderStyle>, const LayoutRect& frameRect);
 
-    static RenderFullScreen* wrapRenderer(RenderObject*, RenderElement*, Document&);
+    static RenderElement* wrapRenderer(RenderObject*, RenderElement*, Document&);
     void unwrapRenderer();
 
 private: