Changeset 147395 in webkit

Timestamp:

Apr 1, 2013 11:49:24 PM (11 years ago)

Author:

esprehn@chromium.org

Message:

RenderObject::offsetParent should return Element*
​https://bugs.webkit.org/show_bug.cgi?id=113739

Reviewed by Abhishek Arya.

The offsetParent of a node should always be an Element, ensure this
by fixing RenderObject::offsetParent to return an Element* and clean up
the method to match the algorithm in the spec which lets us remove the
comments that were trying to explain what was going on.

This also hardens against badness where RenderObject::offsetParent
could have returned a renderer with a non-Element node which would result
in a bad cast in Element::offsetParent. We fixed all cases of this, but
this patch makes sure such things never happen again.

No new tests, no change in behavior.

• dom/Element.cpp:

(WebCore::Element::offsetParent):

• rendering/RenderBoxModelObject.cpp:

(WebCore::RenderBoxModelObject::adjustedPositionRelativeToOffsetParent):

• rendering/RenderObject.cpp:

(WebCore::RenderObject::offsetParent): Now returns Element*.

• rendering/RenderObject.h:

(RenderObject):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Element.cpp (modified) (1 diff)
• rendering/RenderBoxModelObject.cpp (modified) (1 diff)
• rendering/RenderObject.cpp (modified) (2 diffs)
• rendering/RenderObject.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-04-01  Elliott Sprehn  <esprehn@chromium.org>
+
+        RenderObject::offsetParent should return Element*
+        https://bugs.webkit.org/show_bug.cgi?id=113739
+
+        Reviewed by Abhishek Arya.
+
+        The offsetParent of a node should always be an Element, ensure this
+        by fixing RenderObject::offsetParent to return an Element* and clean up
+        the method to match the algorithm in the spec which lets us remove the
+        comments that were trying to explain what was going on.
+
+        This also hardens against badness where RenderObject::offsetParent
+        could have returned a renderer with a non-Element node which would result
+        in a bad cast in Element::offsetParent. We fixed all cases of this, but
+        this patch makes sure such things never happen again.
+
+        No new tests, no change in behavior.
+
+        * dom/Element.cpp:
+        (WebCore::Element::offsetParent):
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::adjustedPositionRelativeToOffsetParent):
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::offsetParent): Now returns Element*.
+        * rendering/RenderObject.h:
+        (RenderObject):
+
 2013-04-01  Alpha Lam  <hclam@chromium.org>
 

trunk/Source/WebCore/dom/Element.cpp

@@ -549 +549 @@
 {
     document()->updateLayoutIgnorePendingStylesheets();
-    if (RenderObject* rend = renderer())
-        if (RenderObject* offsetParent = rend->offsetParent())
-            return toElement(offsetParent->node());
+    if (RenderObject* renderer = this->renderer())
+        return renderer->offsetParent();
     return 0;
 }

trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

@@ -453 +453 @@
     // return the distance between the canvas origin and the left border edge 
     // of the element and stop this algorithm.
-    if (const RenderBoxModelObject* offsetParent = this->offsetParent()) {
+    Element* element = offsetParent();
+    if (!element)
+        return referencePoint;
+
+    if (const RenderBoxModelObject* offsetParent = element->renderBoxModelObject()) {
         if (offsetParent->isBox() && !offsetParent->isBody())
             referencePoint.move(-toRenderBox(offsetParent)->borderLeft(), -toRenderBox(offsetParent)->borderTop());

trunk/Source/WebCore/rendering/RenderObject.cpp

@@ -3002 +3002 @@
 }
 
-RenderBoxModelObject* RenderObject::offsetParent() const
-{
-    // If any of the following holds true return null and stop this algorithm:
-    // A is the root element.
-    // A is the HTML body element.
-    // The computed value of the position property for element A is fixed.
-    if (isRoot() || isBody() || (isOutOfFlowPositioned() && style()->position() == FixedPosition))
+Element* RenderObject::offsetParent() const
+{
+    if (isRoot() || isBody())
+        return 0;
+
+    if (isOutOfFlowPositioned() && style()->position() == FixedPosition)
         return 0;
 
@@ -3015 +3014 @@
     // FIXME: Implement!
 
-    // FIXME: Stop the search at the flow thread boundary until we figure out the right
-    // behavior for elements inside a flow thread.
+    // FIXME: Figure out the right behavior for elements inside a flow thread.
     // https://bugs.webkit.org/show_bug.cgi?id=113276
-    
-    // Return the nearest ancestor element of A for which at least one of the following is
-    // true and stop this algorithm if such an ancestor is found:
-    //     * The computed value of the position property is not static.
-    //     * It is the HTML body element.
-    //     * The computed value of the position property of A is static and the ancestor
-    //       is one of the following HTML elements: td, th, or table.
-    //     * Our own extension: if there is a difference in the effective zoom
-
-    bool skipTables = isPositioned();
-    float currZoom = style()->effectiveZoom();
-    RenderObject* curr = parent();
-    while (curr && (!curr->node() || (!curr->isPositioned() && !curr->isBody())) && !curr->isRenderNamedFlowThread()) {
-        Node* element = curr->node();
-        if (!skipTables && element && (element->hasTagName(tableTag) || element->hasTagName(tdTag) || element->hasTagName(thTag)))
+
+    float effectiveZoom = style()->effectiveZoom();
+    Node* node = 0;
+    for (RenderObject* ancestor = parent(); ancestor; ancestor = ancestor->parent()) {
+        node = ancestor->node();
+
+        // Spec: http://www.w3.org/TR/cssom-view/#offset-attributes
+
+        if (!node)
+            continue;
+
+        if (ancestor->isPositioned())
             break;
 
-        float newZoom = curr->style()->effectiveZoom();
-        if (currZoom != newZoom)
+        if (node->hasTagName(HTMLNames::bodyTag))
             break;
-        currZoom = newZoom;
-        curr = curr->parent();
-    }
-    return curr && curr->isBoxModelObject() && !curr->isRenderNamedFlowThread() ? toRenderBoxModelObject(curr) : 0;
+
+        if (!isPositioned() && (node->hasTagName(tableTag) || node->hasTagName(tdTag) || node->hasTagName(thTag)))
+            break;
+
+        // Webkit specific extension where offsetParent stops at zoom level changes.
+        if (effectiveZoom != ancestor->style()->effectiveZoom())
+            break;
+    }
+
+    return node && node->isElementNode() ? toElement(node) : 0;
 }
 

trunk/Source/WebCore/rendering/RenderObject.h

@@ -667 +667 @@
     virtual RenderObject* hoverAncestor() const { return parent(); }
 
-    // IE Extension that can be called on any RenderObject.  See the implementation for the details.
-    RenderBoxModelObject* offsetParent() const;
+    Element* offsetParent() const;
 
     void markContainingBlocksForLayout(bool scheduleRelayout = true, RenderObject* newRoot = 0);