Title: [167441] trunk
Revision
167441
Author
jer.no...@apple.com
Date
2014-04-17 10:59:26 -0700 (Thu, 17 Apr 2014)

Log Message

Fullscreen media controls are unusable in pagination mode
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.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (167440 => 167441)


--- trunk/LayoutTests/ChangeLog	2014-04-17 17:54:59 UTC (rev 167440)
+++ trunk/LayoutTests/ChangeLog	2014-04-17 17:59:26 UTC (rev 167441)
@@ -1,5 +1,16 @@
 2014-04-16  Jer Noble  <jer.no...@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.no...@apple.com>
+
         [MSE] Multiple initialization segments with same codecs in tracks fail validation.
         https://bugs.webkit.org/show_bug.cgi?id=131768
 

Modified: trunk/LayoutTests/fullscreen/full-screen-no-style-sharing-expected.txt (167440 => 167441)


--- trunk/LayoutTests/fullscreen/full-screen-no-style-sharing-expected.txt	2014-04-17 17:54:59 UTC (rev 167440)
+++ trunk/LayoutTests/fullscreen/full-screen-no-style-sharing-expected.txt	2014-04-17 17:59:26 UTC (rev 167441)
@@ -1,4 +1,4 @@
- 
+  
 EVENT(webkitfullscreenchange) TEST(video2.clientWidth==document.body.clientWidth) OK
 END OF TEST
 

Modified: trunk/LayoutTests/fullscreen/video-cursor-auto-hide.html (167440 => 167441)


--- trunk/LayoutTests/fullscreen/video-cursor-auto-hide.html	2014-04-17 17:54:59 UTC (rev 167440)
+++ trunk/LayoutTests/fullscreen/video-cursor-auto-hide.html	2014-04-17 17:59:26 UTC (rev 167441)
@@ -6,6 +6,7 @@
     <script src=""
     <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);
             }

Modified: trunk/Source/WebCore/ChangeLog (167440 => 167441)


--- trunk/Source/WebCore/ChangeLog	2014-04-17 17:54:59 UTC (rev 167440)
+++ trunk/Source/WebCore/ChangeLog	2014-04-17 17:59:26 UTC (rev 167441)
@@ -1,3 +1,37 @@
+2014-04-15  Jer Noble  <jer.no...@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.no...@apple.com>
 
         [MSE] Multiple initialization segments with same codecs in tracks fail validation.

Modified: trunk/Source/WebCore/dom/Document.cpp (167440 => 167441)


--- trunk/Source/WebCore/dom/Document.cpp	2014-04-17 17:54:59 UTC (rev 167440)
+++ trunk/Source/WebCore/dom/Document.cpp	2014-04-17 17:59:26 UTC (rev 167441)
@@ -5365,9 +5365,6 @@
         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 @@
         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)

Modified: trunk/Source/WebCore/rendering/RenderFullScreen.cpp (167440 => 167441)


--- trunk/Source/WebCore/rendering/RenderFullScreen.cpp	2014-04-17 17:54:59 UTC (rev 167440)
+++ trunk/Source/WebCore/rendering/RenderFullScreen.cpp	2014-04-17 17:59:26 UTC (rev 167441)
@@ -31,6 +31,7 @@
 #include "RenderBlockFlow.h"
 #include "RenderLayer.h"
 #include "RenderLayerCompositor.h"
+#include "RenderView.h"
 
 namespace WebCore {
 
@@ -50,7 +51,7 @@
 
 void RenderFullScreenPlaceholder::willBeDestroyed()
 {
-    m_owner.setPlaceholder(0);
+    m_owner.clearPlaceholder();
     RenderBlockFlow::willBeDestroyed();
 }
 
@@ -104,7 +105,7 @@
     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 @@
         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 @@
             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 @@
             if (child->isBox())
                 toRenderBox(child)->clearOverrideSize();
             child->removeFromParent();
-            parent()->addChild(child, this);
-            parent()->setNeedsLayoutAndPrefWidthsRecalc();
+            placeholder()->parent()->addChild(child, m_placeholder);
         }
+
+        placeholder()->removeFromParent();
     }
-    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));
 }
 
 }

Modified: trunk/Source/WebCore/rendering/RenderFullScreen.h (167440 => 167441)


--- trunk/Source/WebCore/rendering/RenderFullScreen.h	2014-04-17 17:54:59 UTC (rev 167440)
+++ trunk/Source/WebCore/rendering/RenderFullScreen.h	2014-04-17 17:59:26 UTC (rev 167441)
@@ -39,11 +39,12 @@
     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:
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to