Title: [140024] trunk
Revision
140024
Author
rob...@webkit.org
Date
2013-01-17 12:20:11 -0800 (Thu, 17 Jan 2013)

Log Message

Nested fixed position element not staying with parent
https://bugs.webkit.org/show_bug.cgi?id=65477

Reviewed by David Hyatt.

Source/WebCore:

Tests: fast/inline/fixed-pos-moves-with-abspos-inline-parent.html
       fast/inline/fixed-pos-moves-with-abspos-parent-relative-ancestor.html
       fast/inline/fixed-pos-moves-with-abspos-parent.html
       fast/inline/fixed-pos-with-transform-container-moves-with-abspos-parent.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::simplifiedLayout):
If an absolute position element inside a relative positioned container moves, and the absolute element has a fixed position
child, neither the container nor the fixed element learn of the movement since posChildNeedsLayout() is only marked as far as the
relative positioned container. So if our positioned objects list can contain fixed position elements perform the
checks in markFixedPositionObjectForLayoutIfNeeded for each fixed pos object in the container's positioned object list.
(WebCore::RenderBlock::markFixedPositionObjectForLayoutIfNeeded):
For a fixed position element in the positioned objects list that has a static x or y position check for an absolute positioned ancestor
and if we find one, see if the static x or y position of the fixed pos element has changed. If it has, mark it for layout.
(WebCore):
(WebCore::RenderBlock::layoutPositionedObjects):
A fixed position element with an absolute position ancestor has no way of learning if the latter has changed position. So perform the
checks in markFixedPositionObjectForLayoutIfNeeded for each fixed pos object in the container's positioned object list.
* rendering/RenderBlock.h:
(RenderBlock):
* rendering/RenderObject.cpp:
(WebCore::RenderObject::containingBlock): Use new helper function canContainFixedPositionObjects().
* rendering/RenderObject.h:
(WebCore::RenderObject::canContainFixedPositionObjects):
(RenderObject):

LayoutTests:

* fast/inline/fixed-pos-moves-with-abspos-inline-parent-expected.txt: Added.
* fast/inline/fixed-pos-moves-with-abspos-inline-parent.html: Added.
* fast/inline/fixed-pos-moves-with-abspos-parent-expected.txt: Added.
* fast/inline/fixed-pos-moves-with-abspos-parent-relative-ancestor-expected.txt: Added.
* fast/inline/fixed-pos-moves-with-abspos-parent-relative-ancestor.html: Added.
* fast/inline/fixed-pos-moves-with-abspos-parent.html: Added.
* fast/inline/fixed-pos-with-transform-container-moves-with-abspos-parent-expected.txt: Added.
* fast/inline/fixed-pos-with-transform-container-moves-with-abspos-parent.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (140023 => 140024)


--- trunk/LayoutTests/ChangeLog	2013-01-17 20:14:07 UTC (rev 140023)
+++ trunk/LayoutTests/ChangeLog	2013-01-17 20:20:11 UTC (rev 140024)
@@ -1,3 +1,19 @@
+2013-01-17  Robert Hogan  <rob...@webkit.org>
+
+        Nested fixed position element not staying with parent
+        https://bugs.webkit.org/show_bug.cgi?id=65477
+
+        Reviewed by David Hyatt.
+
+        * fast/inline/fixed-pos-moves-with-abspos-inline-parent-expected.txt: Added.
+        * fast/inline/fixed-pos-moves-with-abspos-inline-parent.html: Added.
+        * fast/inline/fixed-pos-moves-with-abspos-parent-expected.txt: Added.
+        * fast/inline/fixed-pos-moves-with-abspos-parent-relative-ancestor-expected.txt: Added.
+        * fast/inline/fixed-pos-moves-with-abspos-parent-relative-ancestor.html: Added.
+        * fast/inline/fixed-pos-moves-with-abspos-parent.html: Added.
+        * fast/inline/fixed-pos-with-transform-container-moves-with-abspos-parent-expected.txt: Added.
+        * fast/inline/fixed-pos-with-transform-container-moves-with-abspos-parent.html: Added.
+
 2013-01-17  Andrei Bucur  <abu...@adobe.com>
 
         [CSS Regions] Content flows incorrectly in autoheight regions with min/max-height set

Added: trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-inline-parent-expected.txt (0 => 140024)


--- trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-inline-parent-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-inline-parent-expected.txt	2013-01-17 20:20:11 UTC (rev 140024)
@@ -0,0 +1,3 @@
+Foo
+
+Success

Added: trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-inline-parent.html (0 => 140024)


--- trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-inline-parent.html	                        (rev 0)
+++ trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-inline-parent.html	2013-01-17 20:20:11 UTC (rev 140024)
@@ -0,0 +1,67 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+            #wrap {
+            position: absolute;
+            width: 800px;
+            height: 200px;
+            border: 2px solid black;
+            left:0;
+            top:0;
+        }
+        
+        #outer {
+            position: absolute;
+            display: inline-block;
+            width:200px;
+            height:200px;
+            background-color:grey;
+        }
+        
+        #outer2 {
+            display: inline-block;
+            width:200px;
+            height:200px;
+            background-color:blue;
+        }
+
+        #inner {
+            width: 100px;
+            height: 100px;
+            background-color:green;
+            position: fixed; top:2px;
+        }
+        
+        #outer p {
+            margin-left: 110px;
+        }
+    </style>
+</head>
+<body>
+    <div id='wrap'>
+        <div id='outer2'></div>
+        <div id='outer'>
+            <div id='inner'></div>
+            <p>Foo</p>
+        </div>
+    </div>
+    <div id="output">Failure</div>    
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+        }
+        
+        function measure() {
+            var output = document.getElementById("output");
+            if (document.getElementById('inner').offsetLeft == 2) {
+                output.innerHTML = "Success"
+            }
+        }
+        
+        var outer = document.getElementById("outer2");
+        document.getElementById("wrap").removeChild(outer);
+        measure();
+    </script>  
+</body>
+</html>

Added: trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-parent-expected.txt (0 => 140024)


--- trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-parent-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-parent-expected.txt	2013-01-17 20:20:11 UTC (rev 140024)
@@ -0,0 +1,3 @@
+Foo
+
+Success

Added: trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-parent-relative-ancestor-expected.txt (0 => 140024)


--- trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-parent-relative-ancestor-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-parent-relative-ancestor-expected.txt	2013-01-17 20:20:11 UTC (rev 140024)
@@ -0,0 +1,3 @@
+Foo
+
+Success

Added: trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-parent-relative-ancestor.html (0 => 140024)


--- trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-parent-relative-ancestor.html	                        (rev 0)
+++ trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-parent-relative-ancestor.html	2013-01-17 20:20:11 UTC (rev 140024)
@@ -0,0 +1,57 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        #wrap {
+            position: relative;
+            width: 400px;
+            height: 200px;
+            border: 2px solid black;
+        }
+        
+        #outer {
+            position: absolute;
+            width:200px;
+            height:200px;
+            background-color:grey;
+        }
+        
+        #inner {
+            width: 100px;
+            height: 100px;
+            background-color:green;
+            position: fixed;
+        }
+        
+        #outer p {
+            margin-left: 110px;
+        }
+    </style>
+</head>
+<body>
+    <div id='wrap'>
+        <div id='outer'>
+            <div>
+                <div id='inner'></div>
+                <p>Foo</p>
+            </div>
+        </div>
+    </div>
+    <div id="output">Failure</div>    
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+        }
+        
+        function measure() {
+            var output = document.getElementById("output");
+            if (document.getElementById('inner').offsetLeft == 210) {
+                output.innerHTML = "Success"
+            }
+        }
+        
+        document.getElementById('outer').style.left = '200px';
+        measure();
+    </script>  
+</body>
+</html>

Added: trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-parent.html (0 => 140024)


--- trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-parent.html	                        (rev 0)
+++ trunk/LayoutTests/fast/inline/fixed-pos-moves-with-abspos-parent.html	2013-01-17 20:20:11 UTC (rev 140024)
@@ -0,0 +1,57 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        #wrap {
+            position: absolute;
+            width: 400px;
+            height: 200px;
+            border: 2px solid black;
+            left:0;
+            top:0;
+        }
+        
+        #outer {
+            position: absolute;
+            width:200px;
+            height:200px;
+            background-color:grey; left:0; top:0;
+        }
+        
+        #inner {
+            width: 100px;
+            height: 100px;
+            background-color:green;
+            position: fixed; top:2px;
+        }
+        
+        #outer p {
+            margin-left: 110px;
+        }
+    </style>
+</head>
+<body>
+    <div id='wrap'>
+        <div id='outer'>
+            <div id='inner'></div>
+            <p>Foo</p>
+        </div>
+    </div>
+    <div id="output">Failure</div>    
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+        }
+        
+        function measure() {
+            var output = document.getElementById("output");
+            if (document.getElementById('inner').offsetLeft >= 200) {
+                output.innerHTML = "Success"
+            }
+        }
+        
+        document.getElementById('outer').style.left = '200px';
+        measure();
+    </script>  
+</body>
+</html>

Added: trunk/LayoutTests/fast/inline/fixed-pos-with-transform-container-moves-with-abspos-parent-expected.txt (0 => 140024)


--- trunk/LayoutTests/fast/inline/fixed-pos-with-transform-container-moves-with-abspos-parent-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/inline/fixed-pos-with-transform-container-moves-with-abspos-parent-expected.txt	2013-01-17 20:20:11 UTC (rev 140024)
@@ -0,0 +1,3 @@
+Foo
+
+Success

Added: trunk/LayoutTests/fast/inline/fixed-pos-with-transform-container-moves-with-abspos-parent.html (0 => 140024)


--- trunk/LayoutTests/fast/inline/fixed-pos-with-transform-container-moves-with-abspos-parent.html	                        (rev 0)
+++ trunk/LayoutTests/fast/inline/fixed-pos-with-transform-container-moves-with-abspos-parent.html	2013-01-17 20:20:11 UTC (rev 140024)
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        #wrap {
+            position: absolute;
+            width: 400px;
+            height: 200px;
+            border: 2px solid black;
+            left:0;
+            top:0;
+            -webkit-transform: rotate(0);
+        }
+        
+        #outer {
+            position: absolute;
+            width:200px;
+            height:200px;
+            background-color:grey; left:0; top:0;
+        }
+        
+        #inner {
+            width: 100px;
+            height: 100px;
+            background-color:green;
+            position: fixed; top:2px;
+        }
+        
+        #outer p {
+            margin-left: 110px;
+        }
+    </style>
+</head>
+<body>
+    <div id='wrap'>
+        <div id='outer'>
+            <div id='inner'></div>
+            <p>Foo</p>
+        </div>
+    </div>
+    <div id="output">Failure</div>    
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+        }
+        
+        function measure() {
+            var output = document.getElementById("output");
+            if (document.getElementById('inner').offsetLeft >= 200) {
+                output.innerHTML = "Success"
+            }
+            testRunner.notifyDone();
+        }
+        
+        document.getElementById('outer').style.left = '200px';
+        measure();
+    </script>  
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (140023 => 140024)


--- trunk/Source/WebCore/ChangeLog	2013-01-17 20:14:07 UTC (rev 140023)
+++ trunk/Source/WebCore/ChangeLog	2013-01-17 20:20:11 UTC (rev 140024)
@@ -1,3 +1,36 @@
+2013-01-17  Robert Hogan  <rob...@webkit.org>
+
+        Nested fixed position element not staying with parent
+        https://bugs.webkit.org/show_bug.cgi?id=65477
+
+        Reviewed by David Hyatt.
+
+        Tests: fast/inline/fixed-pos-moves-with-abspos-inline-parent.html
+               fast/inline/fixed-pos-moves-with-abspos-parent-relative-ancestor.html
+               fast/inline/fixed-pos-moves-with-abspos-parent.html
+               fast/inline/fixed-pos-with-transform-container-moves-with-abspos-parent.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::simplifiedLayout):
+        If an absolute position element inside a relative positioned container moves, and the absolute element has a fixed position
+        child, neither the container nor the fixed element learn of the movement since posChildNeedsLayout() is only marked as far as the 
+        relative positioned container. So if our positioned objects list can contain fixed position elements perform the
+        checks in markFixedPositionObjectForLayoutIfNeeded for each fixed pos object in the container's positioned object list. 
+        (WebCore::RenderBlock::markFixedPositionObjectForLayoutIfNeeded):
+        For a fixed position element in the positioned objects list that has a static x or y position check for an absolute positioned ancestor
+        and if we find one, see if the static x or y position of the fixed pos element has changed. If it has, mark it for layout.
+        (WebCore):
+        (WebCore::RenderBlock::layoutPositionedObjects):
+        A fixed position element with an absolute position ancestor has no way of learning if the latter has changed position. So perform the
+        checks in markFixedPositionObjectForLayoutIfNeeded for each fixed pos object in the container's positioned object list. 
+        * rendering/RenderBlock.h:
+        (RenderBlock):
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::containingBlock): Use new helper function canContainFixedPositionObjects().
+        * rendering/RenderObject.h:
+        (WebCore::RenderObject::canContainFixedPositionObjects):
+        (RenderObject):
+
 2013-01-17  Antti Koivisto  <an...@apple.com>
 
         Make renderer constructors take Element where possible

Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (140023 => 140024)


--- trunk/Source/WebCore/rendering/RenderBlock.cpp	2013-01-17 20:14:07 UTC (rev 140023)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp	2013-01-17 20:20:11 UTC (rev 140024)
@@ -2589,8 +2589,13 @@
         simplifiedNormalFlowLayout();
 
     // Lay out our positioned objects if our positioned child bit is set.
-    if (posChildNeedsLayout())
-        layoutPositionedObjects(false);
+    // Also, if an absolute position element inside a relative positioned container moves, and the absolute element has a fixed position
+    // child, neither the fixed element nor its container learn of the movement since posChildNeedsLayout() is only marked as far as the 
+    // relative positioned container. So if we can have fixed pos objects in our positioned objects list check if any of them
+    // are statically positioned and thus need to move with their absolute ancestors.
+    bool canContainFixedPosObjects = canContainFixedPositionObjects();
+    if (posChildNeedsLayout() || canContainFixedPosObjects)
+        layoutPositionedObjects(false, !posChildNeedsLayout() && canContainFixedPosObjects);
 
     // Recompute our overflow information.
     // FIXME: We could do better here by computing a temporary overflow object from layoutPositionedObjects and only
@@ -2610,8 +2615,38 @@
     return true;
 }
 
-void RenderBlock::layoutPositionedObjects(bool relayoutChildren)
+void RenderBlock::markFixedPositionObjectForLayoutIfNeeded(RenderObject* child)
 {
+    if (child->style()->position() != FixedPosition)
+        return;
+
+    bool hasStaticBlockPosition = child->style()->hasStaticBlockPosition(isHorizontalWritingMode());
+    bool hasStaticInlinePosition = child->style()->hasStaticInlinePosition(isHorizontalWritingMode());
+    if (!hasStaticBlockPosition && !hasStaticInlinePosition)
+        return;
+
+    RenderObject* o = child->parent();
+    while (o && !o->isRenderView() && o->style()->position() != AbsolutePosition)
+        o = o->parent();
+    if (o->style()->position() != AbsolutePosition)
+        return;
+
+    RenderBox* box = toRenderBox(child);
+    if (hasStaticInlinePosition) {
+        LayoutUnit oldLeft = box->logicalLeft();
+        box->updateLogicalWidth();
+        if (box->logicalLeft() != oldLeft)
+            child->setChildNeedsLayout(true, MarkOnlyThis);
+    } else if (hasStaticBlockPosition) {
+        LayoutUnit oldTop = box->logicalTop();
+        box->updateLogicalHeight();
+        if (box->logicalTop() != oldTop)
+            child->setChildNeedsLayout(true, MarkOnlyThis);
+    }
+}
+
+void RenderBlock::layoutPositionedObjects(bool relayoutChildren, bool fixedPositionObjectsOnly)
+{
     TrackedRendererListHashSet* positionedDescendants = positionedObjects();
     if (!positionedDescendants)
         return;
@@ -2623,6 +2658,14 @@
     TrackedRendererListHashSet::iterator end = positionedDescendants->end();
     for (TrackedRendererListHashSet::iterator it = positionedDescendants->begin(); it != end; ++it) {
         r = *it;
+
+        // A fixed position element with an absolute positioned ancestor has no way of knowing if the latter has changed position. So
+        // if this is a fixed position element, mark it for layout if it has an abspos ancestor and needs to move with that ancestor, i.e. 
+        // it has static position.
+        markFixedPositionObjectForLayoutIfNeeded(r);
+        if (fixedPositionObjectsOnly)
+            continue;
+
         // When a non-positioned block element moves, it may have positioned children that are implicitly positioned relative to the
         // non-positioned block.  Rather than trying to detect all of these movement cases, we just always lay out positioned
         // objects that are positioned implicitly like this.  Such objects are rare, and so in typical DHTML menu usage (where everything is

Modified: trunk/Source/WebCore/rendering/RenderBlock.h (140023 => 140024)


--- trunk/Source/WebCore/rendering/RenderBlock.h	2013-01-17 20:14:07 UTC (rev 140023)
+++ trunk/Source/WebCore/rendering/RenderBlock.h	2013-01-17 20:20:11 UTC (rev 140024)
@@ -465,7 +465,8 @@
 
     virtual void layout();
 
-    void layoutPositionedObjects(bool relayoutChildren);
+    void layoutPositionedObjects(bool relayoutChildren, bool fixedPositionObjectsOnly = false);
+    void markFixedPositionObjectForLayoutIfNeeded(RenderObject* child);
 
     virtual void paint(PaintInfo&, const LayoutPoint&);
     virtual void paintObject(PaintInfo&, const LayoutPoint&);

Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (140023 => 140024)


--- trunk/Source/WebCore/rendering/RenderObject.cpp	2013-01-17 20:14:07 UTC (rev 140023)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp	2013-01-17 20:20:11 UTC (rev 140024)
@@ -753,19 +753,8 @@
         o = toRenderScrollbarPart(this)->rendererOwningScrollbar();
     if (!isText() && m_style->position() == FixedPosition) {
         while (o) {
-            if (o->isRenderView())
+            if (o->canContainFixedPositionObjects())
                 break;
-            if (o->hasTransform() && o->isRenderBlock())
-                break;
-            // The render flow thread is the top most containing block
-            // for the fixed positioned elements.
-            if (o->isRenderFlowThread())
-                break;
-#if ENABLE(SVG)
-            // foreignObject is the containing block for its contents.
-            if (o->isSVGForeignObject())
-                break;
-#endif
             o = o->parent();
         }
         ASSERT(!o || !o->isAnonymousBlock());

Modified: trunk/Source/WebCore/rendering/RenderObject.h (140023 => 140024)


--- trunk/Source/WebCore/rendering/RenderObject.h	2013-01-17 20:14:07 UTC (rev 140023)
+++ trunk/Source/WebCore/rendering/RenderObject.h	2013-01-17 20:20:11 UTC (rev 140024)
@@ -712,6 +712,15 @@
     // returns the containing block level element for this element.
     RenderBlock* containingBlock() const;
 
+    bool canContainFixedPositionObjects() const
+    {
+        return isRenderView() || (hasTransform() && isRenderBlock())
+#if ENABLE(SVG)
+                || isSVGForeignObject()
+#endif
+                || isRenderFlowThread();
+    }
+
     // Convert the given local point to absolute coordinates
     // FIXME: Temporary. If UseTransforms is true, take transforms into account. Eventually localToAbsolute() will always be transform-aware.
     FloatPoint localToAbsolute(const FloatPoint& localPoint = FloatPoint(), MapCoordinatesFlags = 0) const;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to