Skip to content

GitLab

T
tangled
Commit ecb99306 authored by Ana SollanoKim's avatar Ana SollanoKim Committed by Commit Bot
Browse files
Options

[LayoutNG] Block-end padding problem when block-fragmenting 

A fieldset-specific regression was introduced by the following change
https://chromium-review.googlesource.com/c/chromium/src/+/2263353. This
change fixes the bug crbug.com/1097012, addressing now all the TODOs
that were added as a result of this issue.

Because the padding of the fieldset's content is applied to the
anonymous fieldset content box rather than the fieldset itself, we
don't want to directly subtract the block-end padding from the fieldset
in the finish fragmentation function. For this reason, we're only
passing the border so that when subtracting to get the final block size,
we're not mistakenly eliminating the padding as well.

The minimum border box block size was not being calculated in every
case due to legends not fragmenting, thus resulting in missing padding
and border for some fragments. Additional conditions were added to allow
that calculation when needed.

Bug: 1097012
Change-Id: I8e9fd55ae11a14ce83e035a591ab0ec6a7f34261
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2373382
Commit-Queue: Ana Sollano Kim <ansollan@microsoft.com>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarAlison Maher <almaher@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#801460}
parent cdbdadef
Hide whitespace changes
Inline Side-by-side
Showing with 17 additions and 45 deletions
third_party/blink/renderer/core/layout/ng/ng_fieldset_layout_algorithm.cc
View file @ ecb99306
......@@ -126,9 +126,9 @@ scoped_refptr<const NGLayoutResult> NGFieldsetLayoutAlgorithm::Layout() {
container_builder_.SetIsFieldsetContainer();
if (ConstraintSpace().HasBlockFragmentation()) {
FinishFragmentation(
Node(), ConstraintSpace(), BreakToken(), BorderPadding(),
FragmentainerSpaceAtBfcStart(ConstraintSpace()), &container_builder_);
FinishFragmentation(Node(), ConstraintSpace(), BreakToken(), borders_,
FragmentainerSpaceAtBfcStart(ConstraintSpace()),
&container_builder_);
}
NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), &container_builder_).Run();
......@@ -158,11 +158,13 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutChildren() {
}
NGBlockNode legend = Node().GetRenderedLegend();
if (legend && !IsResumingLayout(BreakToken())) {
LayoutLegend(legend);
if (legend) {
if (!IsResumingLayout(BreakToken()))
LayoutLegend(legend);
// The legend may eat from the available content box block size. Calculate
// the minimum block size needed to encompass the legend.
if (!Node().ShouldApplySizeContainment()) {
if (!Node().ShouldApplySizeContainment() &&
!IsResumingLayout(content_break_token.get())) {
minimum_border_box_block_size_ =
intrinsic_block_size_ + padding_.BlockSum() + borders_.block_end;
}
......
third_party/blink/renderer/core/layout/ng/ng_fieldset_layout_algorithm_test.cc
View file @ ecb99306
......@@ -681,11 +681,8 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, FieldsetContentFragmentationAutoHeight) {
ASSERT_TRUE(fragment->BreakToken());
String dump = DumpFragmentTree(fragment.get());
// TODO(crbug.com/1097012): The height of the outermost fragment here should
// be 200, not 190, but the fragmentation machinery gets confused by the
// fieldset padding.
String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:176x190
offset:unplaced size:176x200
offset:3,3 size:170x197
offset:10,10 size:50x187
)DUMP";
......@@ -696,11 +693,8 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, FieldsetContentFragmentationAutoHeight) {
ASSERT_TRUE(fragment->BreakToken());
dump = DumpFragmentTree(fragment.get());
// TODO(crbug.com/1097012): The height of the outermost fragment here should
// be 200, not 190, but the fragmentation machinery gets confused by the
// fieldset padding.
expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:176x190
offset:unplaced size:176x200
offset:3,0 size:170x200
offset:10,0 size:50x200
)DUMP";
......@@ -811,11 +805,8 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendFragmentationAutoHeight) {
ASSERT_TRUE(fragment->BreakToken());
String dump = DumpFragmentTree(fragment.get());
// TODO(crbug.com/1097012): The height of the outermost fragment here should
// be 500, not 490, but the fragmentation machinery gets confused by the
// fieldset padding.
String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:176x490
offset:unplaced size:176x500
offset:13,0 size:50x500
)DUMP";
EXPECT_EQ(expectation, dump);
......@@ -863,11 +854,8 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendFragmentation) {
ASSERT_TRUE(fragment->BreakToken());
String dump = DumpFragmentTree(fragment.get());
// TODO(crbug.com/1097012): The height of the outermost fragment here should
// be 500, not 490, but the fragmentation machinery gets confused by the
// fieldset padding.
String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:176x490
offset:unplaced size:176x500
offset:13,0 size:50x500
)DUMP";
EXPECT_EQ(expectation, dump);
......@@ -877,11 +865,8 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendFragmentation) {
ASSERT_FALSE(fragment->BreakToken());
dump = DumpFragmentTree(fragment.get());
// TODO(crbug.com/1097012): The height of the outermost fragment here should
// be 23, not 0, but the fragmentation machinery gets confused by the
// fieldset padding.
expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:176x0
offset:unplaced size:176x23
offset:3,0 size:170x20
)DUMP";
EXPECT_EQ(expectation, dump);
......@@ -922,11 +907,8 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendAndContentFragmentationAutoHeight) {
ASSERT_TRUE(fragment->BreakToken());
String dump = DumpFragmentTree(fragment.get());
// TODO(crbug.com/1097012): The height of the outermost fragment here should
// be 500, not 490, but the fragmentation machinery gets confused by the
// fieldset padding.
String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:176x490
offset:unplaced size:176x500
offset:13,0 size:50x500
)DUMP";
EXPECT_EQ(expectation, dump);
......@@ -934,12 +916,9 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendAndContentFragmentationAutoHeight) {
fragment = NGBaseLayoutAlgorithmTest::RunFieldsetLayoutAlgorithm(
node, space, fragment->BreakToken());
ASSERT_TRUE(fragment->BreakToken());
// TODO(crbug.com/1097012): The height of the outermost fragment here should
// be 200, not 190, but the fragmentation machinery gets confused by the
// fieldset padding.
dump = DumpFragmentTree(fragment.get());
expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:176x190
offset:unplaced size:176x200
offset:3,0 size:170x200
offset:10,10 size:100x190
)DUMP";
......@@ -993,11 +972,8 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendAndContentFragmentation) {
ASSERT_TRUE(fragment->BreakToken());
String dump = DumpFragmentTree(fragment.get());
// TODO(crbug.com/1097012): The height of the outermost fragment here should
// be 500, not 490, but the fragmentation machinery gets confused by the
// fieldset padding.
String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:176x490
offset:unplaced size:176x500
offset:13,0 size:50x500
)DUMP";
EXPECT_EQ(expectation, dump);
......@@ -1007,11 +983,8 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendAndContentFragmentation) {
ASSERT_TRUE(fragment->BreakToken());
dump = DumpFragmentTree(fragment.get());
// TODO(crbug.com/1097012): The height of the outermost fragment here should
// be 23, not 0, but the fragmentation machinery gets confused by the
// fieldset padding.
expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:176x0
offset:unplaced size:176x23
offset:3,0 size:170x20
offset:10,10 size:100x190
)DUMP";
......@@ -1665,9 +1638,6 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, MarginBottomPastEndOfFragmentainer) {
ASSERT_TRUE(fragment->BreakToken());
String dump = DumpFragmentTree(fragment.get());
// TODO(crbug.com/1097012): The height of the outermost fragment here should
// be 100, not 110, but the fragmentation machinery gets confused
// and includes the margin bottom.
String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:100x110
offset:0,0 size:0x90
......
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