Title: [149653] trunk/Source/WebCore
- Revision
- 149653
- Author
- [email protected]
- Date
- 2013-05-06 17:33:05 -0700 (Mon, 06 May 2013)
Log Message
Reproducible crash in RenderBoxModelObject::adjustedPositionRelativeToOffsetParent()
https://bugs.webkit.org/show_bug.cgi?id=115685
-and corresponding-
<rdar://problem/13700734>
Reviewed by Sam Weinig.
This fix here is just to rollout the change that caused this regression, which is
http://trac.webkit.org/changeset/147395 . That change was not intended to cause any
behavioral differences. The change made it so RenderObject::offsetParent() returned
an Element* instead of a RenderBoxModelObject*. However, can muddle things when the
object we are returning is a continuation. Multiple RenderObjects have the same
Element in a continuation, so this new code can lead to a crash in
adjustedPositionRelativeToOffsetParent() when we expect to walk the RenderObject
chain and find the offsetParent in the Element’s parent chain. But we might crash in
some complicated continuation scenarios because we lost this disambiguation of which
RenderObject to start with.
Roll out.
* dom/Element.cpp:
(WebCore::Element::offsetParent):
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::adjustedPositionRelativeToOffsetParent):
* rendering/RenderObject.cpp:
(WebCore::RenderObject::offsetParent):
* rendering/RenderObject.h:
(RenderObject):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (149652 => 149653)
--- trunk/Source/WebCore/ChangeLog 2013-05-07 00:20:05 UTC (rev 149652)
+++ trunk/Source/WebCore/ChangeLog 2013-05-07 00:33:05 UTC (rev 149653)
@@ -1,3 +1,33 @@
+2013-05-06 Beth Dakin <[email protected]>
+
+ Reproducible crash in RenderBoxModelObject::adjustedPositionRelativeToOffsetParent()
+ https://bugs.webkit.org/show_bug.cgi?id=115685
+ -and corresponding-
+ <rdar://problem/13700734>
+
+ Reviewed by Sam Weinig.
+
+ This fix here is just to rollout the change that caused this regression, which is
+ http://trac.webkit.org/changeset/147395 . That change was not intended to cause any
+ behavioral differences. The change made it so RenderObject::offsetParent() returned
+ an Element* instead of a RenderBoxModelObject*. However, can muddle things when the
+ object we are returning is a continuation. Multiple RenderObjects have the same
+ Element in a continuation, so this new code can lead to a crash in
+ adjustedPositionRelativeToOffsetParent() when we expect to walk the RenderObject
+ chain and find the offsetParent in the Element’s parent chain. But we might crash in
+ some complicated continuation scenarios because we lost this disambiguation of which
+ RenderObject to start with.
+
+ Roll out.
+ * dom/Element.cpp:
+ (WebCore::Element::offsetParent):
+ * rendering/RenderBoxModelObject.cpp:
+ (WebCore::RenderBoxModelObject::adjustedPositionRelativeToOffsetParent):
+ * rendering/RenderObject.cpp:
+ (WebCore::RenderObject::offsetParent):
+ * rendering/RenderObject.h:
+ (RenderObject):
+
2013-05-06 Ryosuke Niwa <[email protected]>
Unify ways to cache named item in HTMLCollections
Modified: trunk/Source/WebCore/dom/Element.cpp (149652 => 149653)
--- trunk/Source/WebCore/dom/Element.cpp 2013-05-07 00:20:05 UTC (rev 149652)
+++ trunk/Source/WebCore/dom/Element.cpp 2013-05-07 00:33:05 UTC (rev 149653)
@@ -548,8 +548,10 @@
Element* Element::offsetParent()
{
document()->updateLayoutIgnorePendingStylesheets();
- if (RenderObject* renderer = this->renderer())
- return renderer->offsetParent();
+ if (RenderObject* renderer = this->renderer()) {
+ if (RenderObject* offsetParent = renderer->offsetParent())
+ return toElement(offsetParent->node());
+ }
return 0;
}
Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp (149652 => 149653)
--- trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2013-05-07 00:20:05 UTC (rev 149652)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2013-05-07 00:33:05 UTC (rev 149653)
@@ -488,11 +488,7 @@
// If the offsetParent of the element is null, or is the HTML body element,
// return the distance between the canvas origin and the left border edge
// of the element and stop this algorithm.
- Element* element = offsetParent();
- if (!element)
- return referencePoint;
-
- if (const RenderBoxModelObject* offsetParent = element->renderBoxModelObject()) {
+ if (const RenderBoxModelObject* offsetParent = this->offsetParent()) {
if (offsetParent->isBox() && !offsetParent->isBody())
referencePoint.move(-toRenderBox(offsetParent)->borderLeft(), -toRenderBox(offsetParent)->borderTop());
if (!isOutOfFlowPositioned()) {
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (149652 => 149653)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2013-05-07 00:20:05 UTC (rev 149652)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2013-05-07 00:33:05 UTC (rev 149653)
@@ -2976,46 +2976,46 @@
imageChanged(static_cast<WrappedImagePtr>(image), rect);
}
-Element* RenderObject::offsetParent() const
+RenderBoxModelObject* RenderObject::offsetParent() const
{
- if (isRoot() || isBody())
+ // 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))
return 0;
- if (isOutOfFlowPositioned() && style()->position() == FixedPosition)
- return 0;
-
// If A is an area HTML element which has a map HTML element somewhere in the ancestor
// chain return the nearest ancestor map HTML element and stop this algorithm.
// FIXME: Implement!
- // FIXME: Figure out the right behavior for elements inside a flow thread.
+ // FIXME: Stop the search at the flow thread boundary until we 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
- 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())
+ 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)))
break;
-
- if (node->hasTagName(HTMLNames::bodyTag))
+
+ float newZoom = curr->style()->effectiveZoom();
+ if (currZoom != newZoom)
break;
-
- 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;
+ currZoom = newZoom;
+ curr = curr->parent();
}
-
- return node && node->isElementNode() ? toElement(node) : 0;
+ return curr && curr->isBoxModelObject() && !curr->isRenderNamedFlowThread() ? toRenderBoxModelObject(curr) : 0;
}
VisiblePosition RenderObject::createVisiblePosition(int offset, EAffinity affinity)
Modified: trunk/Source/WebCore/rendering/RenderObject.h (149652 => 149653)
--- trunk/Source/WebCore/rendering/RenderObject.h 2013-05-07 00:20:05 UTC (rev 149652)
+++ trunk/Source/WebCore/rendering/RenderObject.h 2013-05-07 00:33:05 UTC (rev 149653)
@@ -659,7 +659,7 @@
virtual RenderObject* hoverAncestor() const { return parent(); }
- Element* offsetParent() const;
+ RenderBoxModelObject* offsetParent() const;
void markContainingBlocksForLayout(bool scheduleRelayout = true, RenderObject* newRoot = 0);
void setNeedsLayout(bool needsLayout, MarkingBehavior = MarkContainingBlockChain);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
