1485924 - Fieldset painting order is wrong

Status: NEW

(Core :: Web Painting, defect, P3)

Description

[Simon Pieters [:zcorpan]]

http://web-platform.test:8000/html/rendering/non-replaced-elements/the-fieldset-element-0/fieldset-painting-order.html

Comment 1

[Emilio Cobos Álvarez (:emilio)]

So are we supposed to always paint the legend and then the content on top? Do you have a link to the spec by any chance?

This behavior seems somewhat intentional per this comment from bug 344894.

Comment 2

[Simon Pieters [:zcorpan]]

https://html.spec.whatwg.org/multipage/rendering.html#the-fieldset-and-legend-elements

"If the element has a rendered legend, then that element is expected to be the first child box."

The spec doesn't say to always paint the content on top, just what the order of the boxes is, and then CSS defines what to render on top of what (depending on things like z-index).

Safari passes the test.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Are you sure Safari is right on this one?

So even if I remove the special painting code with a patch like:

diff --git a/layout/forms/nsFieldSetFrame.cpp b/layout/forms/nsFieldSetFrame.cpp
index f0bc9dcfbbe0..178abbd6b5b6 100644
--- a/layout/forms/nsFieldSetFrame.cpp
+++ b/layout/forms/nsFieldSetFrame.cpp
@@ -220,7 +220,7 @@ void nsFieldSetFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
   // REVIEW: We don't really need to check frame emptiness here; if it's empty,
   // the background/border display item won't do anything, and if it isn't
   // empty, we need to paint the outline
-  if (!(GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER) &&
+  if (!HasAnyStateBits(NS_FRAME_IS_OVERFLOW_CONTAINER) &&
       IsVisibleForPainting()) {
     DisplayOutsetBoxShadowUnconditional(aBuilder, aLists.BorderBackground());
 
@@ -243,27 +243,12 @@ void nsFieldSetFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
     DisplayOverflowContainers(aBuilder, aLists);
   }
 
-  nsDisplayListCollection contentDisplayItems(aBuilder);
+  if (nsIFrame* legend = GetLegend()) {
+    BuildDisplayListForChild(aBuilder, legend, aLists);
+  }
   if (nsIFrame* inner = GetInner()) {
-    // Collect the inner frame's display items into their own collection.
-    // We need to be calling BuildDisplayList on it before the legend in
-    // case it contains out-of-flow frames whose placeholders are in the
-    // legend. However, we want the inner frame's display items to be
-    // after the legend's display items in z-order, so we need to save them
-    // and append them later.
-    BuildDisplayListForChild(aBuilder, inner, contentDisplayItems);
+    BuildDisplayListForChild(aBuilder, inner, aLists);
   }
-  if (nsIFrame* legend = GetLegend()) {
-    // The legend's background goes on our BlockBorderBackgrounds list because
-    // it's a block child.
-    nsDisplayListSet set(aLists, aLists.BlockBorderBackgrounds());
-    BuildDisplayListForChild(aBuilder, legend, set);
-  }
-  // Put the inner frame's display items on the master list. Note that this
-  // moves its border/background display items to our BorderBackground() list,
-  // which isn't really correct, but it's OK because the inner frame is
-  // anonymous and can't have its own border and background.
-  contentDisplayItems.MoveTo(aLists);
 }
 
 image::ImgDrawResult nsFieldSetFrame::PaintBorder(

We paint the float on top. css2 says that floats are painted after in-flow blocks, but I may be missing something...

Then the other issue here (for which we show the red fieldset border) is that we use the legend size to size the fieldset box, so it ends up being not zero-height in the end IIUC.

Comment 4

[Simon Pieters [:zcorpan]]

It is possible that the test is wrong. If the float is supposed to be on top, Chrome has the correct rendering (right?).

For the block size, the spec says

"For the purpose of calculating the used 'block-size', if the computed 'block-size' is not 'auto', the space allocated for the rendered legend's margin box that spills out past the border, if any, is expected to be substracted from the 'block-size'. If the content box's block-size would be negative, then let the content box's block-size be zero instead."