Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(72)

Issue 1397383002: Don't keep float-lists in inline children (Closed)

Created:
5 years, 2 months ago by rhogan
Modified:
5 years, 2 months ago
CC:
blink-reviews, blink-reviews-layout_chromium.org, chromium-reviews, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Don't keep float-lists in inline children Remove the tolerance for floatlists in inline children that was added in https://codereview.chromium.org/850143002. This was motivated by the need to allow for inline children getting floatlists when their block container was changed to inline by collapseAnonymousBlocks() in https://codereview.chromium.org/253313005. The real solution is to allow the float lists to get cleared down when the child is moved to its new parent - this is achieved by waiting to set its children to inline until after the children have been moved (a child is removed from its parent's and its parent's ancestors' floatlists when it's removed). BUG=537111 Committed: https://crrev.com/42f33c141cbe3f2ad0e6ec038bf78404e22c7bb3 Cr-Commit-Position: refs/heads/master@{#353986}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -2 lines) Patch
A third_party/WebKit/PerformanceTests/Layout/add-remove-inline-floats.html View 1 chunk +40 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBlock.cpp View 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 7 (2 generated)
rhogan
5 years, 2 months ago (2015-10-12 14:51:02 UTC) #2
leviw_travelin_and_unemployed
lgtm Great! Thanks!
5 years, 2 months ago (2015-10-14 08:33:39 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1397383002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1397383002/1
5 years, 2 months ago (2015-10-14 08:35:11 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 2 months ago (2015-10-14 09:48:30 UTC) #6
commit-bot: I haz the power
5 years, 2 months ago (2015-10-14 09:49:28 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/42f33c141cbe3f2ad0e6ec038bf78404e22c7bb3
Cr-Commit-Position: refs/heads/master@{#353986}

Powered by Google App Engine
This is Rietveld 408576698