third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp - Issue 2414683002: Make offsetTop/Left handle a relative positioned inline offsetParent correctly.

Unified Diff: third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp

Issue 2414683002: Make offsetTop/Left handle a relative positioned inline offsetParent correctly. (Closed)

Patch Set: Changes according to review. Created 4 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« no previous file with comments | « third_party/WebKit/LayoutTests/fast/inline/inline-offsetLeft-relpos-expected.txt ('k') | third_party/WebKit/Source/core/layout/LayoutInline.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp

diff --git a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp

index 653dffb1188e470df8c63354f5ee628f2b27cf94..da3cedc0100d40ac068930f9b9c675fb138b07a1 100644

--- a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp

+++ b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp

@@ -990,7 +990,7 @@ LayoutSize LayoutBoxModelObject::stickyPositionOffset() const {

LayoutPoint LayoutBoxModelObject::adjustedPositionRelativeTo(

const LayoutPoint& startPoint,

- const Element* element) const {

+ const Element* offsetParent) const {

// If the element is the HTML body element or doesn't have a parent

// return 0 and stop this algorithm.

if (isBody() || !parent())

@@ -998,13 +998,13 @@ LayoutPoint LayoutBoxModelObject::adjustedPositionRelativeTo(

LayoutPoint referencePoint = startPoint;

- // If the base element is null, return the distance between the canvas origin

- // and the left border edge of the element and stop this algorithm.

- if (!element)

+ // If the offsetParent is null, return the distance between the canvas origin

+ // and the left/top border edge of the element and stop this algorithm.

+ if (!offsetParent)

return referencePoint;

- if (const LayoutBoxModelObject* offsetParent =

- element->layoutBoxModelObject()) {

+ if (const LayoutBoxModelObject* offsetParentObject =

+ offsetParent->layoutBoxModelObject()) {

if (!isOutOfFlowPositioned()) {

if (isInFlowPositioned())

referencePoint.move(offsetForInFlowPosition());

@@ -1014,22 +1014,40 @@ LayoutPoint LayoutBoxModelObject::adjustedPositionRelativeTo(

// continuations: <body style="display:inline;" id="offsetParent"><div>

// </div><span id="this">

// This is why we have to do a nullptr check here.

- // offset(Left|Top) is generally broken when offsetParent is inline.

for (const LayoutObject* current = container();

- current && current != offsetParent; current = current->container()) {

+ current && current != offsetParentObject;

+ current = current->container()) {

// FIXME: What are we supposed to do inside SVG content?

referencePoint.move(current->columnOffset(referencePoint));

if (current->isBox() && !current->isTableRow())

referencePoint.moveBy(toLayoutBox(current)->topLeftLocation());

}

- if (offsetParent->isBox() && offsetParent->isBody() &&

- !offsetParent->isPositioned())

- referencePoint.moveBy(toLayoutBox(offsetParent)->topLeftLocation());

+ if (offsetParentObject->isBox() && offsetParentObject->isBody() &&

+ !offsetParentObject->isPositioned()) {

+ referencePoint.moveBy(

+ toLayoutBox(offsetParentObject)->topLeftLocation());

+ }

+ if (offsetParentObject->isLayoutInline()) {

atotic 2016/10/31 22:54:25 Can offsetParentObject be null here? We're checkin

Karl Øygard 2016/11/03 11:23:31 offsetParentObject is checked for null in an if st

+ const LayoutInline* inlineParent = toLayoutInline(offsetParentObject);

+ if (isBox() && style()->position() == AbsolutePosition &&

+ inlineParent->isInFlowPositioned()) {

+ // Absolute positioned objects need to be placed on the line

atotic 2016/10/31 22:54:25 This still leaves me wondering: - "on the line" wi

Karl Øygard 2016/11/03 11:23:31 Agreed, I have amended the comment accordingly.

atotic 2016/10/31 22:54:25 Remove blank line

+ referencePoint +=

+ inlineParent->offsetForInFlowPositionedInline(*toLayoutBox(this));

+ }

+ referencePoint -= inlineParent->firstLineBoxTopLeft();

+ }

+ if (offsetParentObject->isBox() && !offsetParentObject->isBody()) {

+ referencePoint.move(-toLayoutBox(offsetParentObject)->borderLeft(),

+ -toLayoutBox(offsetParentObject)->borderTop());

}

- if (offsetParent->isBox() && !offsetParent->isBody())

- referencePoint.move(-toLayoutBox(offsetParent)->borderLeft(),

- -toLayoutBox(offsetParent)->borderTop());

}

return referencePoint;