-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revamp how fieldset and legend rendering is defined #3934
Conversation
cc @whatwg/fieldset |
source
Outdated
positioned in the inline direction as follows:</p> | ||
|
||
<ul> | ||
<li><p>If the computed value of 'text-align' is 'left', then position the element on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the rationale behind letting the text-align property affect the LEGEND block itself.
In my book, that should be determined by inline margins, and possibly also the ALIGN attribute (but that one's deprecated, right?).
Text-align should only affect the position of text within lines. The position of the lines (or their container) should not be affected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rationale is just that WebKit and Chromium do this, but we can change it to be more like EdgeHTML (align
attribute on ancestor div
affects the legend, but text-align
does not affect the position).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we should be more like Edge here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 6bdb923
source
Outdated
|
||
<li>The child is generating a box (e.g. it is not 'display:none' or 'display:contents').</li> | ||
<li><p>If the element has a <span>rendered legend</span>, then there border on the block-start |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"then there border" -> "then the border"
source
Outdated
|
||
<li><p>The element is expected to have a child anonymous box, the <dfn>anonymous fieldset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not so sure about the anonymous box idea anymore. First of all, we'd then have to distribute the CSS properties on the FIELDSET node between the wrapper box and the contents container. Things like 'width', 'border', 'background' 'float', 'z-index', 'position' and 'transform' would intuitively belong in the wrapper, while 'display' and 'overflow' would have to be set on the contents container anonymous box. Do we want to go there?
WebKit creates just one box for a FIELDSET (box type determined by 'display'). The legend is simply encapsulated into the border block-start area. Any chance we could just spec it like that? Having the legend encapsulated into the border makes a lot of sense, e.g. when it comes to determining the containing block size for out-of-flow positioned descendants (in the cases where the FIELDSET isn't statically positioned). Scrolling would also work well then, since borders are outside the scrollport.
One thing that's still not obvious with this approach, though, is paint order. Should the legend be painted in the same phase as the borders (unless the legend establishes a stacking context entry (position:relative, etc.))?
Another thing would be that the used border-width may diverge from the computed border-width. Is that a problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this approach works for implementation, it should work for the spec. 🙂
I think paint order should be such that the legend is painted after the fieldset's borders, but before other content of the fieldset. Does that match WebKit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it. Would be interesting to know what others think about this "legend is border" approach, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, please take a look at the latest commit.
source
Outdated
<li>The child is not <span>out-of-flow</span> (e.g. not absolutely positioned or floated).</li> | ||
<li><p>If the computed value of 'display' is one of 'block', 'table', 'table-row-group', | ||
'table-header-group', 'table-footer-group', 'table-row', 'table-cell', 'table-column-group', | ||
'table-column', 'table-caption', 'list-item', 'flow', 'flow-root', or 'run-in', then the used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Promoting table-column / table-column-group (something essentially invisible and display:none-ish) to a block seems a bit weird.
Might also consider setting the used value to "flow-root" (rather than "block") for the other values in this list, and get a new formatting context for free. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's a bit weird but it's what browsers do, AFAICT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still need to say to establish the BFC for flex/grid, so it doesn't help much.
source
Outdated
<ul> | ||
<li><p>If the computed value of 'display' is one of 'inline', 'inline-block', 'inline-table', | ||
'ruby', 'ruby-base', 'ruby-text', 'ruby-base-container', 'ruby-text-container', then the | ||
used value is 'inline-block'.</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "used value" thing here bothers me a bit here - first and foremost as an implementor. :)
There are specs that do blockification, for instance, but that's about changing the computed value, not the used value. WebKit does actually change the used value for fieldsets, but is there any reason why we shouldn't change the computed value instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this was to match existing implementations. If we can clean this up that seems nice though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can change this if it's easier to implement. @MatsPalmgren any opinion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting used value of display does nothing; it's not a meaningful operation, since 'display' is only ever consulted during the computed->used transformation.
You can say that if [...] holds, then 'display' behaves as "inline-block", with a note that it doesn't change the computed value for backwards-compatibility reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should try to avoid having the computed value of one property (-webkit-appearance) affect the computed value of another property (display) in general, because it makes style engines more complicated and might degrade performance. So Tab's "behaves as" suggestion sounds good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, fixed in 256593b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the phrasing "behaves as", it's becoming a term of art in CSS. (Not quite actually defined yet, but I've been thinking about how to formalize it lately.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what Simon did, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tabatkins fixed.
source
Outdated
<ul class="brief"> | ||
<li>The child's used value of '-webkit-appearance' is 'fieldset-legend'.</li> | ||
<li>The child's used value of 'float' is 'none'.</li> | ||
<li>The child's used value of 'position' is not 'absolute' or 'fixed'.</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really think 'sticky' should also disqualify it from becoming a rendered legend. It's just gonna look silly anyway (right?), and probably not straight-forward to implement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the rendered legend in WebKit/Chromium/Gecko/EdgeHTML. It is sticky in Chromium/Gecko/EdgeHTML but not in WebKit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should legends be allowed to be multicol containers, BTW? Yes, because they are allowed to be blocks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Test at web-platform-tests/wpt#12592
source
Outdated
<p>The used value of 'display' is determined as follows:</p> | ||
|
||
<ul> | ||
<li><p>If the computed value of 'display' is 'inline-flex', then the used value is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why allow only flex, grid and block?
I think we should either allow it to only be a block container, or allow practically any display-inside values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Browsers today blockify legends, but @MatsPalmgren said they should support grid and flex. We could potentially support more if there are use cases and it doesn't break web compat.
https://gist.github.com/zcorpan/2f590536cf64c1aaabbc70f63dbfe2b5
source
Outdated
positioned in the inline direction as follows:</p> | ||
|
||
<ul> | ||
<li><p>If the computed value of 'text-align' is 'left', then position the element on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we should be more like Edge here.
Containing block size calculation (for e.g. percentage height resolution) for legends needs to be specced. I imagine it should contain the logical top border and padding areas (since legends are placed above the padding area, and are part of the border), although no browsers do that currently: http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=6133 (only testing borders, no padding) |
Edge/Firefox/Chrome renders the percentage height testcase the same though, so I think we should spec that rather than risk regressing existing web content. Also, I don't consider a rendered legend to be "part of the border" because then its extent should be part of the border-box area and thus the fieldset background should render behind it (fully), a box-shadow should wrap it etc, and that's not what any UAs do as far as I know. Perhaps the spec would benefit from an example figure that visualizes the rendering? Something like this table figure. In Gecko we implement the |
Yes, all browsers render the same with my test, and I do agree that it's risky to spec it otherwise then. However, since legends are placed at the very top of the fieldset (and thus ignores top border and padding), it's kind of strange that top border and padding are included in the containing block size when resolving percentages. Anyway, my post about containing block size calculation concerns was written under the assumption that we really let the legend be part of the border area (that's what the current spec changes in this pull request say), but I believe this is something we still need to discuss. I was quite sold on the "two boxes" approach myself, but then I noticed that WebKit just puts it in the border area (and expands the border area if necessary), which seems simpler, and also easy to reason about. Blink doesn't use two boxes for tables, like the spec says (that implementation detail isn't leaked via any web-exposed API that I'm aware of, though), but what I find tricky with this approach is to determine which properties should be set on the outer box and which to set on the inner one. https://www.w3.org/TR/CSS22/tables.html#model mentions 'position', 'float', 'margin-*', 'top', 'right', 'bottom', and 'left' as properties to set on the wrapper box. That list is far from complete, though. First of all, it only covers CSS 2 (and even then it seems to forget about things like z-index). How do you determine which property goes where? EDIT: Looks like the answer to that lies in https://dxr.mozilla.org/mozilla-central/source/layout/style/res/forms.css |
Related bug re percentage height. https://bugs.chromium.org/p/chromium/issues/detail?id=178782 |
I find 20 pages in httparchive that style legend with a percentage height. https://gist.github.com/zcorpan/186444076d75b735e99439bcddf92e40 |
source
Outdated
|
||
<li><p>For the purposes of painting order, the <span>rendered legend</span> is expected to | ||
be painted after the element's own borders and before the element's other content.</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine in the general case, but I'm realizing that it won't do if the legend establishes a stacking order entry (using relative position, opacity != 1.0, transform, etc.). Then we won't be able to paint it in the background phase. One special thing with the background phase is that it's not affected by scroll offset. But if we don't paint the legend in the background phase, it will be affected by the scroll offset if the fieldset has scrollable overflow.
This is something that won't be a problem if we wrap everything but the legend inside an anonymous fieldset child (like Firefox), so that e.g. overflow:scroll will be set on that anonymous child (which obviously won't affect the legend when it's on the outside).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does WebKit do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fails to paint the legend when it's relatively positioned and the fieldset is scrollable.
http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=6141
Here's a demo with percentage heights: http://software.hixie.ch/utilities/js/live-dom-viewer/saved/6143 A is 150px in Safari/Chrome/Firefox/Edge. B is 5px in Safari/Firefox, 150px in Chrome/Edge. |
The B result in Firefox seems like a bug to me. I think it should be 150px. I filed bug 1485646. |
In that case, I think what https://drafts.csswg.org/css2/visudet.html#containing-block-details says should just apply to fieldset, without HTML having to say anything in particular. Right? |
I'd like us to settle on a FIELDSET/LEGEND layout model.
In this example we have a scrollable fieldset. The legend is not expected to scroll along with the fieldset contents, according to Firefox, Safari, and common sense. This does suggest that it makes sense to create an anonymous child for the actual contents of the fieldset. So I'd like to explore that approach a bit further (rather than my former favorite: slap the legend in the border area, expand the border as necessary, paint it as part of border painting - like WebKit). The question then is, which properties should the anonymous child inherit, and which ones should the parent fieldset ignore (if any). And how much magic do we have to apply in addition to that? Intuitively, 'overflow' should go on the anoymous child, so that the legend isn't scrolled along with the rest. Padding probably needs to be ignored on the parent FIELDSET and be applied on the anonymous child, though, so that the scrollport includes the padding. However, percentages need to be resolved on the FIELDSET and NOT on the anonymous child. @MatsPalmgren Does Firefox really get away with no special magic here? Firefox has a nice selector for the anonymous child here: https://dxr.mozilla.org/mozilla-central/source/layout/style/res/forms.css |
Why doesn't the WebKit approach work when there is a scrollport? What does WebKit do in that case? |
…', a=testonly Automatic update from web-platform-testsHTML: legend align maps to 'justify-self' See whatwg/html#3934 whatwg/html#4013 -- wpt-commits: 51ceab16ec539fd421322727f93a847f7855e317 wpt-pr: 12965 UltraBlame original commit: 29a65b24c9343cd0ee4e3f3b455ede769fe7f9c9
…erent display values, a=testonly Automatic update from web-platform-testsHTML: test rendering of legend with different display values See whatwg/html#3934 -- wpt-commits: a0d4be661b5c006fe64b42fe08aeb992668052ac wpt-pr: 12932 UltraBlame original commit: d0a604c0e54d21f2e6f7acc4e04159d22c354f93
…e, a=testonly Automatic update from web-platform-testsHTML: test fieldset percentage block size See whatwg/html#3934 -- wpt-commits: c4998afb119a14386c5645e0d1a7c90954849dab wpt-pr: 12930 UltraBlame original commit: bef4326766956abb1594de06a3dc02cf494b6201
…ap to text-align, a=testonly Automatic update from web-platform-testsHTML: test how <legend align> does not map to text-align See whatwg/html#3934 -- wpt-commits: 7fc2450b557f563772fcab4f58c8be7a0ac4d0f1 wpt-pr: 12811 UltraBlame original commit: 977177e75fabc38be4ee9cf463e86580cad246ff
…set, a=testonly Automatic update from web-platform-testsHTML: test ::before and ::after on fieldset See whatwg/html#3934 -- wpt-commits: d0949fa95df08de5fd7ce52e471275458ffc3eae wpt-pr: 12939 UltraBlame original commit: 6cd3000ba9336dced29983eef0c276c1b8dae30f
…culation, a=testonly Automatic update from web-platform-testsHTML: test fieldset min-/max-content calculation See whatwg/html#3934 -- wpt-commits: af3dc210d2ec8d12710f113e0111593fc0e54a43 wpt-pr: 12928 UltraBlame original commit: 84c2e053f9079c90bddcba02272ba7dc40a2b0a8
…set, a=testonly Automatic update from web-platform-testsHTML: test ::before and ::after on fieldset See whatwg/html#3934 -- wpt-commits: d0949fa95df08de5fd7ce52e471275458ffc3eae wpt-pr: 12939 UltraBlame original commit: 873fc52d8225c55401c9f1c41ae1d2b301cac763
…culation, a=testonly Automatic update from web-platform-testsHTML: test fieldset min-/max-content calculation See whatwg/html#3934 -- wpt-commits: af3dc210d2ec8d12710f113e0111593fc0e54a43 wpt-pr: 12928 UltraBlame original commit: 91a65226cc3f514630c3a3d281f9202709a24059
…eft margin masks the border, a=testonly Automatic update from web-platform-testsHTML: test that a legend with negative left margin masks the border See whatwg/html#3934 -- wpt-commits: e241e4de1c07c47f004199fc12d031559208b922 wpt-pr: 13004 UltraBlame original commit: aace32da71166232ec51bb72cc8a601036823114
…stonly Automatic update from web-platform-testsHTML: Test block margins on legend See whatwg/html#3934 -- wpt-commits: e40235366bbd17f42e23e0177352c5f52c655909 wpt-pr: 13040 UltraBlame original commit: a5948eab1e97437c0743f669fdf124575efc3cf2
…', a=testonly Automatic update from web-platform-testsHTML: legend align maps to 'justify-self' See whatwg/html#3934 whatwg/html#4013 -- wpt-commits: 51ceab16ec539fd421322727f93a847f7855e317 wpt-pr: 12965 UltraBlame original commit: 29a65b24c9343cd0ee4e3f3b455ede769fe7f9c9
…erent display values, a=testonly Automatic update from web-platform-testsHTML: test rendering of legend with different display values See whatwg/html#3934 -- wpt-commits: a0d4be661b5c006fe64b42fe08aeb992668052ac wpt-pr: 12932 UltraBlame original commit: d0a604c0e54d21f2e6f7acc4e04159d22c354f93
…e, a=testonly Automatic update from web-platform-testsHTML: test fieldset percentage block size See whatwg/html#3934 -- wpt-commits: c4998afb119a14386c5645e0d1a7c90954849dab wpt-pr: 12930 UltraBlame original commit: bef4326766956abb1594de06a3dc02cf494b6201
…g order, a=testonly Automatic update from web-platform-testsHTML: test fieldset's legend and painting order Part of whatwg/html#3934 -- wpt-commits: fa88a22846b422d4128c9e6c049ad33fe675c2c9 wpt-pr: 12590 UltraBlame original commit: d1cf8fc10da1952486898430623e325010e4f269
…dset's legend, a=testonly Automatic update from web-platform-testsHTML: test flexbox/grid/multicol on fieldset's legend Part of whatwg/html#3934 -- wpt-commits: f0d9781d64a016c49e0f12a874e07f5598d2262f wpt-pr: 12592 UltraBlame original commit: b9fe9f60448eaecfdb329df928d8e5107710b6a1
…only Automatic update from web-platform-testsHTML: test fieldset's border gap Part of whatwg/html#3934 -- wpt-commits: a68b451920b32057c705d614874ac1392536dd9b wpt-pr: 12587 UltraBlame original commit: 91c2af6e159616655a85279b0acb55c003426f3e
…ap to text-align, a=testonly Automatic update from web-platform-testsHTML: test how <legend align> does not map to text-align See whatwg/html#3934 -- wpt-commits: 7fc2450b557f563772fcab4f58c8be7a0ac4d0f1 wpt-pr: 12811 UltraBlame original commit: 977177e75fabc38be4ee9cf463e86580cad246ff
…set, a=testonly Automatic update from web-platform-testsHTML: test ::before and ::after on fieldset See whatwg/html#3934 -- wpt-commits: d0949fa95df08de5fd7ce52e471275458ffc3eae wpt-pr: 12939 UltraBlame original commit: 6cd3000ba9336dced29983eef0c276c1b8dae30f
…culation, a=testonly Automatic update from web-platform-testsHTML: test fieldset min-/max-content calculation See whatwg/html#3934 -- wpt-commits: af3dc210d2ec8d12710f113e0111593fc0e54a43 wpt-pr: 12928 UltraBlame original commit: 84c2e053f9079c90bddcba02272ba7dc40a2b0a8
…set, a=testonly Automatic update from web-platform-testsHTML: test ::before and ::after on fieldset See whatwg/html#3934 -- wpt-commits: d0949fa95df08de5fd7ce52e471275458ffc3eae wpt-pr: 12939 UltraBlame original commit: 873fc52d8225c55401c9f1c41ae1d2b301cac763
…culation, a=testonly Automatic update from web-platform-testsHTML: test fieldset min-/max-content calculation See whatwg/html#3934 -- wpt-commits: af3dc210d2ec8d12710f113e0111593fc0e54a43 wpt-pr: 12928 UltraBlame original commit: 91a65226cc3f514630c3a3d281f9202709a24059
…eft margin masks the border, a=testonly Automatic update from web-platform-testsHTML: test that a legend with negative left margin masks the border See whatwg/html#3934 -- wpt-commits: e241e4de1c07c47f004199fc12d031559208b922 wpt-pr: 13004 UltraBlame original commit: aace32da71166232ec51bb72cc8a601036823114
…stonly Automatic update from web-platform-testsHTML: Test block margins on legend See whatwg/html#3934 -- wpt-commits: e40235366bbd17f42e23e0177352c5f52c655909 wpt-pr: 13040 UltraBlame original commit: a5948eab1e97437c0743f669fdf124575efc3cf2
…', a=testonly Automatic update from web-platform-testsHTML: legend align maps to 'justify-self' See whatwg/html#3934 whatwg/html#4013 -- wpt-commits: 51ceab16ec539fd421322727f93a847f7855e317 wpt-pr: 12965 UltraBlame original commit: 29a65b24c9343cd0ee4e3f3b455ede769fe7f9c9
…erent display values, a=testonly Automatic update from web-platform-testsHTML: test rendering of legend with different display values See whatwg/html#3934 -- wpt-commits: a0d4be661b5c006fe64b42fe08aeb992668052ac wpt-pr: 12932 UltraBlame original commit: d0a604c0e54d21f2e6f7acc4e04159d22c354f93
…e, a=testonly Automatic update from web-platform-testsHTML: test fieldset percentage block size See whatwg/html#3934 -- wpt-commits: c4998afb119a14386c5645e0d1a7c90954849dab wpt-pr: 12930 UltraBlame original commit: bef4326766956abb1594de06a3dc02cf494b6201
Properly define the rendering of the fieldset and legend elements.
The layout model used is most similar to Gecko, which uses an anonymous box to hold the fieldset's contents.
Fixes #3955, fixes #3930, fixes #3929, fixes #3928, fixes #3927, fixes #3915, fixes #3913, fixes #3660, fixes #3331, fixes #2756, fixes #4013.
/infrastructure.html ( diff )
/interaction.html ( diff )
/interactive-elements.html ( diff )
/references.html ( diff )
/rendering.html ( diff )