Skip to content

GitLab

T
tangled
Commit feb8974e authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot
Browse files
Options

[layout] Fix self-collapsing end-margins contributing to overflow. 

Previously we just took the end margin-strut, and used this for the
scrollable-overflow. This was incorrect.

As the end margin-strut is used for positioning self-collapsing blocks
we ended up "double counting" these margins.

This patch uses the "final" margin-strut, and subtracts the
self-collapsing end margin-strut to correctly determine the block-end
margin which contributes to scrollable-overflow.

Bug: 1136403
Change-Id: I7d0d0fe801948c3ce818719f28f1a7c795a7ec56
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466379
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816586}
parent 144d0d0b
Hide whitespace changes
Inline Side-by-side
Showing with 36 additions and 4 deletions
third_party/blink/renderer/core/layout/ng/ng_box_fragment_builder.cc
View file @ feb8974e
...@@ -199,7 +199,8 @@ void NGBoxFragmentBuilder::AddResult(const NGLayoutResult& child_layout_result, ...@@ -199,7 +199,8 @@ void NGBoxFragmentBuilder::AddResult(const NGLayoutResult& child_layout_result,
// No margins should pierce outside formatting-context roots. // No margins should pierce outside formatting-context roots.
DCHECK(!fragment.IsFormattingContextRoot() || end_margin_strut.IsEmpty()); DCHECK(!fragment.IsFormattingContextRoot() || end_margin_strut.IsEmpty());
AddChild(fragment, offset, /* inline_container */ nullptr, &end_margin_strut); AddChild(fragment, offset, /* inline_container */ nullptr, &end_margin_strut,
child_layout_result.IsSelfCollapsing());
if (fragment.IsBox()) if (fragment.IsBox())
PropagateBreak(child_layout_result); PropagateBreak(child_layout_result);
} }
...@@ -207,7 +208,8 @@ void NGBoxFragmentBuilder::AddResult(const NGLayoutResult& child_layout_result, ...@@ -207,7 +208,8 @@ void NGBoxFragmentBuilder::AddResult(const NGLayoutResult& child_layout_result,
void NGBoxFragmentBuilder::AddChild(const NGPhysicalContainerFragment& child, void NGBoxFragmentBuilder::AddChild(const NGPhysicalContainerFragment& child,
const LogicalOffset& child_offset, const LogicalOffset& child_offset,
const LayoutInline* inline_container, const LayoutInline* inline_container,
const NGMarginStrut* margin_strut) { const NGMarginStrut* margin_strut,
bool is_self_collapsing) {
LogicalOffset adjusted_offset = child_offset; LogicalOffset adjusted_offset = child_offset;
if (box_type_ != NGPhysicalBoxFragment::NGBoxType::kInlineBox) { if (box_type_ != NGPhysicalBoxFragment::NGBoxType::kInlineBox) {
...@@ -254,7 +256,13 @@ void NGBoxFragmentBuilder::AddChild(const NGPhysicalContainerFragment& child, ...@@ -254,7 +256,13 @@ void NGBoxFragmentBuilder::AddChild(const NGPhysicalContainerFragment& child,
if (margin_strut) { if (margin_strut) {
NGMarginStrut end_margin_strut = *margin_strut; NGMarginStrut end_margin_strut = *margin_strut;
end_margin_strut.Append(margins.block_end, /* is_quirky */ false); end_margin_strut.Append(margins.block_end, /* is_quirky */ false);
margins.block_end = end_margin_strut.Sum();
// Self-collapsing blocks are special, their end margin-strut is part
// of their inflow position. To correctly determine the "end" margin,
// we need to the "final" margin-strut from their end margin-strut.
margins.block_end = is_self_collapsing
? end_margin_strut.Sum() - margin_strut->Sum()
: end_margin_strut.Sum();
} }
NGFragment fragment(GetWritingDirection(), child); NGFragment fragment(GetWritingDirection(), child);
......
third_party/blink/renderer/core/layout/ng/ng_box_fragment_builder.h
View file @ feb8974e
...@@ -209,7 +209,8 @@ class CORE_EXPORT NGBoxFragmentBuilder final ...@@ -209,7 +209,8 @@ class CORE_EXPORT NGBoxFragmentBuilder final
void AddChild(const NGPhysicalContainerFragment&, void AddChild(const NGPhysicalContainerFragment&,
const LogicalOffset&, const LogicalOffset&,
const LayoutInline* inline_container = nullptr, const LayoutInline* inline_container = nullptr,
const NGMarginStrut* margin_strut = nullptr); const NGMarginStrut* margin_strut = nullptr,
bool is_self_collapsing = false);
// Manually add a break token to the builder. Note that we're assuming that // Manually add a break token to the builder. Note that we're assuming that
// this break token is for content in the same flow as this parent. // this break token is for content in the same flow as this parent.
......
third_party/blink/web_tests/external/wpt/css/css-overflow/scrollable-overflow-self-collapsing.html 0 → 100644
View file @ feb8974e
<!DOCTYPE html>
<meta name="assert" content="This ensures that an end self-collapsing block contributes to the scrollable overflow correctly.">
<link rel="help" href="https://drafts.csswg.org/css-overflow-3/#scrollable" />
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>
<body onload="checkLayout('.target')">
<div class="target" style="width: 100px; height: 100px; overflow: scroll;" data-expected-scroll-height="110">
<div style="width: 50px; height: 100px; margin-bottom: 10px; background: lime;"></div>
<div></div> <!-- self-collapsing -->
</div>
<div class="target" style="width: 100px; height: 100px; overflow: scroll;" data-expected-scroll-height="150">
<div style="width: 50px; height: 100px; margin-bottom: 10px; background: lime;"></div>
<div style="margin-bottom: 50px;"></div> <!-- self-collapsing -->
</div>
<div class="target" style="width: 100px; height: 100px; overflow: scroll;" data-expected-scroll-height="180">
<div style="width: 50px; height: 100px; margin-bottom: 30px; background: lime;"></div>
<div style="margin-bottom: 100px;"> <!-- self-collapsing -->
<div style="margin-top: -20px;"></div>
</div>
</div>
<div id=log></div>
</body>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment