Issue 14759010: Fix the float logic to not return an anonymous block ancestor

Issue 14759010: Fix the float logic to not return an anonymous block ancestor (Closed)

Created:
7 years, 7 months ago by inferno

Modified:
7 years, 7 months ago

Reviewers:
Julien - ping for review

CC:
blink-reviews, jchaffraix+rendering, leviw_travelin_and_unemployed

Base URL:
svn://svn.chromium.org/blink/trunk

Visibility:
Public.

More Reviews

Description

Before this change, we would wrongly bail-out from the loop if our ancestor was anonymous. That meant that in some circumstances, we would not notify the right object of the float removal, leading to tree badness. In this case, these objects were the next sibling blocks containing the overhanging floats. Next sibling blocks need to be determined relative to the containing block / non-anonymous parent. BUG=222852 R=jchaffraix@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=149981

Patch Set 1 #

Total comments: 2

Patch Set 2 : Update as per Julien's comments. #

Total comments: 7

Patch Set 3 : Update as per Julien's comments #

Patch Set 4 : Sorry, forgot to update test in last commit #

Created: 7 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+25 lines, -0 lines)			Patch
A	LayoutTests/fast/block/float/float-not-removed-crash2.html	View	1 2 3	1 chunk	+25 lines, -0 lines	0 comments	Download
A +	LayoutTests/fast/block/float/float-not-removed-crash2-expected.txt	View	1 2 3	0 chunks	+-1 lines, --1 lines	0 comments	Download
M	Source/core/rendering/RenderBox.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

Julien - ping for review

https://codereview.chromium.org/14759010/diff/1/LayoutTests/fast/block/float/float-not-removed-crash2.html File LayoutTests/fast/block/float/float-not-removed-crash2.html (right): https://codereview.chromium.org/14759010/diff/1/LayoutTests/fast/block/float/float-not-removed-crash2.html#newcode3 LayoutTests/fast/block/float/float-not-removed-crash2.html:3: <figure> ]V 8[</figure><row><source id=test style="float: right; -webkit-padding-before: 238px; ">></foobar>>>>><listing ...

7 years, 7 months ago (2013-05-07 00:51:22 UTC) #2

inferno

On 2013/05/07 00:51:22, Julien Chaffraix wrote: > https://codereview.chromium.org/14759010/diff/1/LayoutTests/fast/block/float/float-not-removed-crash2.html > File LayoutTests/fast/block/float/float-not-removed-crash2.html (right): > > https://codereview.chromium.org/14759010/diff/1/LayoutTests/fast/block/float/float-not-removed-crash2.html#newcode3 ...

7 years, 7 months ago (2013-05-07 15:45:27 UTC) #3

Julien - ping for review

I believe the code change to be fine (well at least half of it). Please ...

7 years, 7 months ago (2013-05-07 18:43:20 UTC) #4

Julien - ping for review

LGTM but please update the description to be a proper git log message and include ...

7 years, 7 months ago (2013-05-08 21:21:39 UTC) #5

inferno

Committed patchset #4 manually as r149981 (presubmit successful).

7 years, 7 months ago (2013-05-08 21:38:53 UTC) #6

pdr.

7 years, 7 months ago (2013-05-10 21:02:56 UTC) #7

Message was sent while issue was closed.

We may want to reconsider this patch.

Hyatt says this fix is totally wrong but would like more info. Do you mind
connecting with him to see what's up? I would just give him access to the bug
but I don't have it either.

Expand Messages | Collapse Messages