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

Issue 2759693002: Push the top margin of floats past all useless fragmentainers. (Closed)

Created:
3 years, 9 months ago by mstensho (USE GERRIT)
Modified:
3 years, 9 months ago
Reviewers:
eae
CC:
chromium-reviews, szager+layoutwatch_chromium.org, zoltan1, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, blink-reviews, mstensho (USE GERRIT)
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Push the top margin of floats past all useless fragmentainers. The top margin of a float is not to be split across fragmentainer boundaries if it can be avoided. We had code to push the margin over to the next fragmentainer if we were out of space, but we may actually have to push it all the way to the next fragmentainer *group* (i.e column row) in some cases. calculatePaginationStrutToFitContent() helps us get there. That's the method we use to push oversize content (lines and unbreakable blocks) to a better place, so let's use it for margins too. Review-Url: https://codereview.chromium.org/2759693002 Cr-Commit-Position: refs/heads/master@{#457881} Committed: https://chromium.googlesource.com/chromium/src/+/fd525b8b047cf0843d05107567c14aea9a5858e0

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -2 lines) Patch
A third_party/WebKit/LayoutTests/fast/multicol/float-margin-at-row-boundary.html View 1 chunk +9 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/multicol/float-margin-at-row-boundary-expected.html View 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp View 1 chunk +4 lines, -2 lines 0 comments Download

Messages

Total messages: 16 (12 generated)
mstensho (USE GERRIT)
3 years, 9 months ago (2017-03-17 20:23:48 UTC) #10
eae
OK, LGTM
3 years, 9 months ago (2017-03-17 20:42:57 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2759693002/1
3 years, 9 months ago (2017-03-17 20:43:25 UTC) #13
commit-bot: I haz the power
3 years, 9 months ago (2017-03-17 20:50:29 UTC) #16
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/fd525b8b047cf0843d05107567c1...

Powered by Google App Engine
This is Rietveld 408576698