[LayoutNG] Ensure fieldset content applies padding in all cases

When the fieldset is expanded to encompass the legend, the fieldset
content no longer applies the fieldset padding. This change
ensures that in this case, the fieldset content is aware of the change
in the border box size so that it will properly take care of fieldset
padding.

Bug: 875235
Change-Id: I2ee67221f195bdbcb06122ee4f01a1024e49ceb9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2109128
Commit-Queue: Alison Maher <almaher@microsoft.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751829}

third_party/blink/renderer/core/layout/ng/ng_fieldset_layout_algorithm.cc

@@ -89,17 +89,12 @@ scoped_refptr<const NGLayoutResult> NGFieldsetLayoutAlgorithm::Layout() {
   // contents, with the conjecture being that legend is part of the contents.
   // Thus, only do this adjustment if we do not contain size.
   if (!Node().ShouldApplySizeContainment()) {
-    LayoutUnit minimum_border_box_block_size;
-    if (legend_needs_layout_) {
-      minimum_border_box_block_size =
-          borders_with_legend_.BlockSum() + padding_.BlockSum();
-    }
     // Similar to how we add the consumed block size to the intrinsic
     // block size when calculating border_box_size_.block_size, we also need to
     // do so when the fieldset is adjusted to encompass the legend.
     border_box_size_.block_size =
         std::max(border_box_size_.block_size,
-                 minimum_border_box_block_size + consumed_block_size);
+                 minimum_border_box_block_size_ + consumed_block_size);
   }
 
   // TODO(almaher): end border and padding may overflow the parent
@@ -150,10 +145,10 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutChildren() {
   }
 
   NGBlockNode legend = Node().GetRenderedLegend();
-  legend_needs_layout_ =
+  bool legend_needs_layout =
       legend && (legend_break_token || !IsResumingLayout(BreakToken()));
 
-  if (legend_needs_layout_) {
+  if (legend_needs_layout) {
     // TODO(almaher): Handle all break status cases.
     NGBreakStatus break_status = LayoutLegend(legend, legend_break_token);
     DCHECK_EQ(break_status, NGBreakStatus::kContinue);
@@ -161,6 +156,20 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutChildren() {
 
   borders_with_legend_ = borders_;
   borders_with_legend_.block_start = block_start_padding_edge_;
+
+  // The legend may eat from the available content box block size. If the
+  // border_box_size_ is expanded to encompass the legend, then update the
+  // border_box_size_ here, as well, to ensure the fieldset content gets the
+  // correct size.
+  if (!Node().ShouldApplySizeContainment() && legend_needs_layout) {
+    minimum_border_box_block_size_ =
+        borders_with_legend_.BlockSum() + padding_.BlockSum();
+    if (border_box_size_.block_size != kIndefiniteSize) {
+      border_box_size_.block_size =
+          std::max(border_box_size_.block_size, minimum_border_box_block_size_);
+    }
+  }
+
   LogicalSize adjusted_padding_box_size =
       ShrinkAvailableSize(border_box_size_, borders_with_legend_);
   if ((IsResumingLayout(content_break_token.get())) ||

third_party/blink/renderer/core/layout/ng/ng_fieldset_layout_algorithm.h

@@ -65,13 +65,14 @@ class CORE_EXPORT NGFieldsetLayoutAlgorithm
   LayoutUnit intrinsic_block_size_;
   LogicalSize border_box_size_;
 
+  // The legend may eat from the available content box block size. This
+  // represents the minimum block size needed by the border box to encompass
+  // the legend.
+  LayoutUnit minimum_border_box_block_size_;
+
   // If true, this indicates the block_start_padding_edge_ had changed from its
   // initial value during the current layout pass.
   bool block_start_padding_edge_adjusted_ = false;
-
-  // If true, this indicates that either the entire legend or part of the
-  // legend needs to be laid out during the current layout pass.
-  bool legend_needs_layout_ = false;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/layout/ng/ng_fieldset_layout_algorithm_test.cc

@@ -350,7 +350,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, ZeroHeight) {
   offset:unplaced size:1000x53
     offset:0,0 size:126x53
       offset:13,0 size:30x30
-      offset:3,30 size:120x0
+      offset:3,30 size:120x20
         offset:10,10 size:100x200
 )DUMP";
   EXPECT_EQ(expectation, dump);
@@ -445,6 +445,46 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendPercentHeightQuirks) {
   EXPECT_EQ(expectation, dump);
 }
 
+// This test makes sure that the fieldset content handles fieldset padding
+// when the fieldset is expanded to encompass the legend.
+TEST_F(NGFieldsetLayoutAlgorithmTest, FieldsetPaddingWithLegend) {
+  SetBodyInnerHTML(R"HTML(
+      <style>
+        #fieldset {
+          border:none; margin:0; padding:10px; width: 150px; height: 100px;
+        }
+        #legend {
+          padding:0px; margin:0; width: 50px; height: 120px;
+        }
+        #child {
+          width: 100px; height: 40px;
+        }
+      </style>
+      <fieldset id="fieldset">
+        <legend id="legend"></legend>
+        <div id="child"></div>
+      </fieldset>
+  )HTML");
+
+  NGBlockNode node(ToLayoutBox(GetLayoutObjectByElementId("fieldset")));
+  NGConstraintSpace space = ConstructBlockLayoutTestConstraintSpace(
+      WritingMode::kHorizontalTb, TextDirection::kLtr,
+      LogicalSize(LayoutUnit(1000), kIndefiniteSize), false,
+      node.CreatesNewFormattingContext());
+
+  scoped_refptr<const NGPhysicalBoxFragment> fragment =
+      NGBaseLayoutAlgorithmTest::RunFieldsetLayoutAlgorithm(node, space);
+
+  String dump = DumpFragmentTree(fragment.get());
+  String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
+  offset:unplaced size:170x140
+    offset:10,0 size:50x120
+    offset:0,120 size:170x20
+      offset:10,10 size:100x40
+)DUMP";
+  EXPECT_EQ(expectation, dump);
+}
+
 TEST_F(NGFieldsetLayoutAlgorithmTest, MinMax) {
   SetBodyInnerHTML(R"HTML(
     <style>