third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp - Issue 2454693003: adjustedPositionRelativeTo() couldn't find offsetParent.

Side by Side Diff: third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp

Issue 2454693003: adjustedPositionRelativeTo() couldn't find offsetParent. (Closed)

Patch Set: Created 4 years, 1 month ago

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

Jump to:

« no previous file with comments | « third_party/WebKit/LayoutTests/fast/inline/inline-offsetLeft-continuation-expected.txt ('k') | no next file » | no next file with comments »

OLD	NEW
1 /*	1 /*

2 * Copyright (C) 1999 Lars Knoll (knoll@kde.org)	2 * Copyright (C) 1999 Lars Knoll (knoll@kde.org)

3 * (C) 1999 Antti Koivisto (koivisto@kde.org)	3 * (C) 1999 Antti Koivisto (koivisto@kde.org)

4 * (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)	4 * (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)

5 * (C) 2005, 2006 Samuel Weinig (sam.weinig@gmail.com)	5 * (C) 2005, 2006 Samuel Weinig (sam.weinig@gmail.com)

6 * Copyright (C) 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.	6 * Copyright (C) 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.

7 * Copyright (C) 2010 Google Inc. All rights reserved.	7 * Copyright (C) 2010 Google Inc. All rights reserved.

8 *	8 *

9 * This library is free software; you can redistribute it and/or	9 * This library is free software; you can redistribute it and/or

10 * modify it under the terms of the GNU Library General Public	10 * modify it under the terms of the GNU Library General Public

(...skipping 998 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
1009 if (isInFlowPositioned())	1009 if (isInFlowPositioned())

1010 referencePoint.move(offsetForInFlowPosition());	1010 referencePoint.move(offsetForInFlowPosition());

1011	1011

1012 // Note that we may fail to find \|offsetParent\| while walking the	1012 // Note that we may fail to find \|offsetParent\| while walking the

1013 // container chain, if \|offsetParent\| is an inline split into	1013 // container chain, if \|offsetParent\| is an inline split into

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

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

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

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

1018 for (const LayoutObject* current = container();	1018 for (const LayoutObject* current = container();

1019 current && current != offsetParent; current = current->container()) {	1019 current && current->node() != element;
	Gleb Lanbin 2016/10/31 22:29:53 I'm not very familiar with this code. I would sugg I'm not very familiar with this code. I would suggest mstensho@ as a reviewer. According to git blame he authored this code. From my point of view I have 2 comments: 1) should we change the comment at line 1012 as well? 2) if we don't depend on offsetParent anymore then perhaps we can move this part of code out of the "offsetParent = element->layoutBoxModelObject()" IF statement? Karl Øygard 2016/11/02 12:48:01 Agreed, I've updated the comment with another prob Show quoted text On 2016/10/31 22:29:53, Gleb Lanbin wrote: > I'm not very familiar with this code. I would suggest mstensho@ as a reviewer. > According to git blame he authored this code. > > From my point of view I have 2 comments: > > 1) should we change the comment at line 1012 as well? > 2) if we don't depend on offsetParent anymore then perhaps we can move this part > of code out of the "offsetParent = element->layoutBoxModelObject()" IF > statement? Agreed, I've updated the comment with another problem that I know of that isn't addressed by this fix. I can't see an easy way of moving the code out without some code duplication, so I've let it remain as is.
	1020 current = current->container()) {

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

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

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

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

1024 }	1025 }

1025	1026

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

1027 !offsetParent->isPositioned())	1028 !offsetParent->isPositioned())

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

1029 }	1030 }

(...skipping 356 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
1386 if (rootElementStyle->hasBackground())	1387 if (rootElementStyle->hasBackground())

1387 return false;	1388 return false;

1388	1389

1389 if (node() != document().firstBodyElement())	1390 if (node() != document().firstBodyElement())

1390 return false;	1391 return false;

1391	1392

1392 return true;	1393 return true;

1393 }	1394 }

1394	1395

1395 } // namespace blink	1396 } // namespace blink

OLD	NEW