1485502 - <legend> in display:contents fieldset is fit-content width in Firefox (i.e. shrinkwraps text), but fills available space in Chrome.

Status: NEW

(Core :: Layout: Form Controls, defect, P3)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Load attached testcase.

Firefox's results:
The second box, "LegendInDisplayContentsFieldset", is only as wide as its text.

Chrome's results:
The second box, "LegendInDisplayContentsFieldset", is the full width of the page.

Firefox's behavior comes from this CSS declaration in forms.css:

> fieldset > legend {
>   [...]
>   inline-size: -moz-fit-content;
https://dxr.mozilla.org/mozilla-central/rev/4e56a2f51ad739ca52046723448f3129a58f1666/layout/style/res/forms.css#58,61

zcorpan, do you know if there's any spec text to cover this yet? (If not, mind taking it on the whatwg/spec side?)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Comment 2

[Daniel Holbert [:dholbert]]

More complete summary of browser results here, for the whole testcase:

                                   Firefox    Chrome   Safari  Edge
                                   =======    ======   ======  ====
NormalLegend                       skinny     skinny   skinny  skinny
LegendInDisplayContentsFieldset    skinny     wide     skinny  not-implemented*
StandaloneLegend                    wide      wide     skinny  skinny

We could trivially match the Safari/Edge behavior by simply dropping the "fieldset>" specifier in forms.css, if that ends up making sense. The Chrome behavior seems more subtle (for them, the width behavior depends on whether or not we are a direct child of a fieldset box, regardless of our DOM-tree parentage, which is kind of nice but also a bit magical).

* Edge doesn't have useful results on the seocnd part of the testcase because they don't implement display:contents.

Comment 3

[Mats Palmgren (inactive)]

We're also not compatible for OtherLegend in:
  <fieldset>
    <legend>NormalLegend</legend>
    <legend>OtherLegend</legend>
  </fieldset>

(we shouldn't shrink-wrap it)

So, that selector needs to be :-moz-rendered-legend or some such,
or do it in C++.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

(In reply to Mats Palmgren (:mats) from comment #3)
> So, that selector needs to be :-moz-rendered-legend or some such,
> or do it in C++.

It'd be kinda hard to know the rendered legend in style, so I'd prefer to implement it in layout if possible.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)
> (In reply to Mats Palmgren (:mats) from comment #3)
> > So, that selector needs to be :-moz-rendered-legend or some such,
> > or do it in C++.
> 
> It'd be kinda hard to know the rendered legend in style, so I'd prefer to
> implement it in layout if possible.

Oh, also, invalidation becomes tricky, because removing a legend can cause the pseudo-class to change somewhere else in e.g. a Shadow Tree and such, so it can be slightly annoying to get right.

Comment 6

[Daniel Holbert [:dholbert]]

Yeah, comment 3 is an interesting scenario I hadn't thought to test.  The distinction between "OtherLegend" and "WrappedLegend" is particularly silly here (we shrinkwrap the former, and not the latter):
  <fieldset>
    <legend>NormalLegend</legend>
    <legend>OtherLegend</legend>
    <div><legend>WrappedLegend</legend></div>
  </fieldset>

I tend to think we should just standardize on the Safari/Edge sizing behavior, as documented in comment 2, and make legend default-sizing not care about whether it's a child of a fieldset.

Motiviation:
 - this reduces the need for new magic C++ layout code (and/or a new magic pseudo-class) that would otherwise be needed to cover edge cases of debatable value.
 - We're already resigned to <legend> getting some default styling regardless of whether it's in a fieldset (bug 1483499), and this is just one extra bit of default styling.  (So if someone really wants to have a <legend> element that's wholly unrelated to any fieldsets, they'll probably already need to be resetting some properties anyway.)
 - This is what 2 engines already do right now (per chart in comment 2), which means it's likely web-compatible.
 - It's trivial to implement.

Comment 7

[Mats Palmgren (inactive)]

I don't think that'll work, since with the new spec we need to
support <legend> w. -webkit-appearance:none which should remove all
special legend behavior, and -webkit-appearance:fieldset-legend
on any element which should make it eligible to be a rendered
legend.  So I tend to think we need to do this in C++ since
using any kind of selector for it is tricky as emilio said.

Bug 1483499 seems different since it's just a legacy default
styling for all legend elements specifically.  I don't think
we'd want that padding for a -webkit-appearance:fieldset-legend
<div> for example.

Comment 8

[Simon Pieters [:zcorpan]]

I think we can do one of the following:

1) Only shrink-wrap the "rendered legend". This is what the spec currently requires. Matches Chrome.
2) Always shrink-wrap legends. Matches Webkit and EdgeHTML.

Demo: http://software.hixie.ch/utilities/js/live-dom-viewer/saved/6142

Specifying "width: auto" doesn't remove the shrink-wrapping.

I'd like to scope any magic to "the rendered legend", which would mean doing (1).

Comment 9

[Mats Palmgren (inactive)]

FWIW, based on the "wip" patch I did in bug 1483787,
I think doing 1) would be easy for us to implement.