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

Unresolvable dependencies in determining the rendered fieldset legend #2756

Closed
mstensho opened this Issue Jun 14, 2017 · 13 comments

Comments

3 participants
@mstensho

mstensho commented Jun 14, 2017

https://html.spec.whatwg.org/multipage/rendering.html#rendered-legend

The spec says that a fieldset element's rendered legend is to be rendered over the block-start border edge of the fieldset element as a block box, overriding any explicit 'display' value.

How to determine whether it's a rendered legend? The spec says that, among other things, we need to make sure that a candidate LEGEND element is not out-of-flow (e.g. not absolutely positioned or floated).

This is typically not something you want to figure out without creating the layout box for it first, is it?

So, we first need to figure out if a LEGEND is in-flow before we determine whether to override 'display' to 'block'. We need to resolve the actual display type before creating the box, but in order to do that, we need to know that the box is going to be a rendered legend, i.e. it has to be in-flow, among other things. But we don't have the box yet, because we don't know what type of box to create, because first we need to know if it's going to become a rendered legend. Chicken, meet egg. :)

Okay, it could indeed be possible to walk the flat DOM tree and make highly qualified guesses as to whether an element would generate an in-flow box or not, but it really doesn't sound that tempting.

The only solution i can think of, is to force all LEGEND elements (that don't have display:none or display:contents) to become display:block.

Then again: I'm an Blink layout engine implementor. Could this be something that's only awkward to do in Blink / WebKit?

@zcorpan

This comment has been minimized.

Show comment
Hide comment
@zcorpan

zcorpan Jun 14, 2017

Member

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5243 suggests Gecko doesn't force display:block. @bzbarsky do you know how this is done in Gecko?

Member

zcorpan commented Jun 14, 2017

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5243 suggests Gecko doesn't force display:block. @bzbarsky do you know how this is done in Gecko?

@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Jun 14, 2017

Collaborator

So just so we're clear, the "overriding the display value" means overriding used display. Computed display is unaffected. This means the determination does not in fact need to be made until you're creating the box or otherwise need the used display value.

The exact check Gecko does to determine whether a legend is a rendered legend is that all of the following conditions have to hold:

  1. The parent box is for a fieldset (in Gecko this could be the box of the fieldset itself or one of various anonymous boxes that we use to implement scrolling/columnation/etc for fieldsets; I'm not sure how this would best be expressed in spec terms). This is not the same thing as checking the flattened tree parent, note, because of display:contents, but this code predates display:contents existing. And it's not entirely clear how display:contents and fieldset should interact anyway...
  2. The computed value of "float" is none.
  3. The computed value of "position" is not absolute or fixed.

This is not the same as checking whether the box is positioned or floated; as you note that is a much more complicated check and has chicken-and-egg problems. But it is equivalent in common cases. And in some cases (e.g. a display:flex fieldset), it's actually preferable to do the checks on computed values: it allows making the legend a regular flex item inside the fieldset just by specifying "float" or "position" on it.

Collaborator

bzbarsky commented Jun 14, 2017

So just so we're clear, the "overriding the display value" means overriding used display. Computed display is unaffected. This means the determination does not in fact need to be made until you're creating the box or otherwise need the used display value.

The exact check Gecko does to determine whether a legend is a rendered legend is that all of the following conditions have to hold:

  1. The parent box is for a fieldset (in Gecko this could be the box of the fieldset itself or one of various anonymous boxes that we use to implement scrolling/columnation/etc for fieldsets; I'm not sure how this would best be expressed in spec terms). This is not the same thing as checking the flattened tree parent, note, because of display:contents, but this code predates display:contents existing. And it's not entirely clear how display:contents and fieldset should interact anyway...
  2. The computed value of "float" is none.
  3. The computed value of "position" is not absolute or fixed.

This is not the same as checking whether the box is positioned or floated; as you note that is a much more complicated check and has chicken-and-egg problems. But it is equivalent in common cases. And in some cases (e.g. a display:flex fieldset), it's actually preferable to do the checks on computed values: it allows making the legend a regular flex item inside the fieldset just by specifying "float" or "position" on it.

@zcorpan

This comment has been minimized.

Show comment
Hide comment
@zcorpan

zcorpan Jun 15, 2017

Member

Thank you! Checking computed value seems sane to me. I don't like checking parent box rather than DOM parent.

@mstensho would this approach work better in chromium?

Member

zcorpan commented Jun 15, 2017

Thank you! Checking computed value seems sane to me. I don't like checking parent box rather than DOM parent.

@mstensho would this approach work better in chromium?

@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Jun 15, 2017

Collaborator

I don't like checking parent box rather than DOM parent.

What is the interaction with display: contents (and does CSS even define the behavior of that on fieldset yet)? Specifically, what should happen in these testcases?

<fieldset>
  <div style="display:contents;"><legend>Something</legend></div>
</fieldset>

<div>Some
  <fieldset style="display: contents">
    <legend>thing</legend>
  </fieldset>
</div>

<fieldset>
  <fieldset style="display:contents;"><legend>Something</legend></fieldset>
</fieldset>

What Gecko does is that the first testcase has the legend be the rendered legend; the second and third show that Gecko does not support display:contents on <fieldset> elements at the moment.

Chrome with --enable-experimental-web-platform-feature doesn't support display:contents on <fieldset> elements and doesn't make that first legend rendered.

What is the interaction with shadow DOM?

Collaborator

bzbarsky commented Jun 15, 2017

I don't like checking parent box rather than DOM parent.

What is the interaction with display: contents (and does CSS even define the behavior of that on fieldset yet)? Specifically, what should happen in these testcases?

<fieldset>
  <div style="display:contents;"><legend>Something</legend></div>
</fieldset>

<div>Some
  <fieldset style="display: contents">
    <legend>thing</legend>
  </fieldset>
</div>

<fieldset>
  <fieldset style="display:contents;"><legend>Something</legend></fieldset>
</fieldset>

What Gecko does is that the first testcase has the legend be the rendered legend; the second and third show that Gecko does not support display:contents on <fieldset> elements at the moment.

Chrome with --enable-experimental-web-platform-feature doesn't support display:contents on <fieldset> elements and doesn't make that first legend rendered.

What is the interaction with shadow DOM?

@mstensho

This comment has been minimized.

Show comment
Hide comment
@mstensho

mstensho Jun 15, 2017

Using computed style as an approximation to actual layout should be easy enough, but as @bzbarsky points out, it's not guaranteed to be the same, although it will in common cases. But I suppose the spec needs to explain this somehow.

Another funny complication of letting the renderedness of a legend affect box type, is that changing style on one LEGEND (or inserting or removing the node from the DOM, for that matter) node dynamically may affect some sibling (that will suddenly cease to be, or become, the rendered legend):

<fieldset>
    <legend id="first" style="display:none;"></legend>
    <legend id="second" style="display:inline;"></legend>
</fieldset>

Here, #second is the rendered legend, so it's block-displayed. Now, if we change the display property of #first to "block" (or "inline" or "table" or whatever, except perhaps "contents"), #first will become the rendered legend, which means that #second will change to a regular box, and no longer be the rendered legend. In other words, the box is changed from "block" to "inline". Nothing wrong with that, just funny/complicated.

mstensho commented Jun 15, 2017

Using computed style as an approximation to actual layout should be easy enough, but as @bzbarsky points out, it's not guaranteed to be the same, although it will in common cases. But I suppose the spec needs to explain this somehow.

Another funny complication of letting the renderedness of a legend affect box type, is that changing style on one LEGEND (or inserting or removing the node from the DOM, for that matter) node dynamically may affect some sibling (that will suddenly cease to be, or become, the rendered legend):

<fieldset>
    <legend id="first" style="display:none;"></legend>
    <legend id="second" style="display:inline;"></legend>
</fieldset>

Here, #second is the rendered legend, so it's block-displayed. Now, if we change the display property of #first to "block" (or "inline" or "table" or whatever, except perhaps "contents"), #first will become the rendered legend, which means that #second will change to a regular box, and no longer be the rendered legend. In other words, the box is changed from "block" to "inline". Nothing wrong with that, just funny/complicated.

@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Jun 15, 2017

Collaborator

That's an interesting point. As far as I can tell, Gecko gives any non-floated non-abs/fixedpos legend that's a child of a fieldset a block-inside box. Depending on whether the original display was block-level or inline-level it basically acts like a block or inline-block. This is not restricted to just the "rendered legend". So I guess Gecko actually has two concepts: an "in-flow legend in a fieldset", which gets special box handling, and "rendered legend", which is the first "in-flow legend in a fieldset" for a given fiedset.

What do other UAs do?

Collaborator

bzbarsky commented Jun 15, 2017

That's an interesting point. As far as I can tell, Gecko gives any non-floated non-abs/fixedpos legend that's a child of a fieldset a block-inside box. Depending on whether the original display was block-level or inline-level it basically acts like a block or inline-block. This is not restricted to just the "rendered legend". So I guess Gecko actually has two concepts: an "in-flow legend in a fieldset", which gets special box handling, and "rendered legend", which is the first "in-flow legend in a fieldset" for a given fiedset.

What do other UAs do?

@mstensho

This comment has been minimized.

Show comment
Hide comment
@mstensho

mstensho Jun 16, 2017

Blink: All LEGEND elements that aren't display:none are forced to display:block. The rendered legend is determined after box creation: The first child box that is a LEGEND element and isn't out of flow becomes the rendered legend. All LEGEND elements also establish a block formatting context and they all shrink to fit, but I'm going to change that: https://chromium-review.googlesource.com/c/535595/

There's no mechanism to properly re-layout the boxes if we change which of the LEGENDs is the rendered one. E.g.

<fieldset>
    <legend id="first" style="border:solid;">legend 1</legend>
    <legend id="second" style="border:solid;">legend 2</legend>
</fieldset>

#first is the rendered legend, but if we change it to display:none, #second becomes the rendered legend. But Blink has no mechanism to detect that this requires re-layout of #second, so it won't shrink to fit (until you somehow trigger layout of it via other means, such as resizing the window).

mstensho commented Jun 16, 2017

Blink: All LEGEND elements that aren't display:none are forced to display:block. The rendered legend is determined after box creation: The first child box that is a LEGEND element and isn't out of flow becomes the rendered legend. All LEGEND elements also establish a block formatting context and they all shrink to fit, but I'm going to change that: https://chromium-review.googlesource.com/c/535595/

There's no mechanism to properly re-layout the boxes if we change which of the LEGENDs is the rendered one. E.g.

<fieldset>
    <legend id="first" style="border:solid;">legend 1</legend>
    <legend id="second" style="border:solid;">legend 2</legend>
</fieldset>

#first is the rendered legend, but if we change it to display:none, #second becomes the rendered legend. But Blink has no mechanism to detect that this requires re-layout of #second, so it won't shrink to fit (until you somehow trigger layout of it via other means, such as resizing the window).

@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Jun 16, 2017

Collaborator

All LEGEND elements also establish a block formatting context and they all shrink to fit, but I'm going to change that:

Hmm. That's also the case in Gecko, for all legend whose parent box is a fieldset. What do other UAs do?

It really would be good to sort out what spec we can all agree on before currently-compatible-across-UAs behavior gets changed in some UAs.

Collaborator

bzbarsky commented Jun 16, 2017

All LEGEND elements also establish a block formatting context and they all shrink to fit, but I'm going to change that:

Hmm. That's also the case in Gecko, for all legend whose parent box is a fieldset. What do other UAs do?

It really would be good to sort out what spec we can all agree on before currently-compatible-across-UAs behavior gets changed in some UAs.

@mstensho

This comment has been minimized.

Show comment
Hide comment
@mstensho

mstensho Jun 19, 2017

I'd really like to land this change in Blink rather sooner than later, because this is a regression fix, sort of. Previously, no LEGEND established a block formatting context, but they all rather had this non-standard "avoid floats on the outside, but don't contain descendant floats" thing, which I removed some weeks ago, and turned them all into proper BFCs instead. And then I realized that this change made non-rendered legends more magical than before, in a sense.

mstensho commented Jun 19, 2017

I'd really like to land this change in Blink rather sooner than later, because this is a regression fix, sort of. Previously, no LEGEND established a block formatting context, but they all rather had this non-standard "avoid floats on the outside, but don't contain descendant floats" thing, which I removed some weeks ago, and turned them all into proper BFCs instead. And then I realized that this change made non-rendered legends more magical than before, in a sense.

@zcorpan

This comment has been minimized.

Show comment
Hide comment
@zcorpan

zcorpan Jun 19, 2017

Member

I would be OK with having a concept of "potential rendered legend", which is any legend that is a child of a fieldset (DOM child, possibly flat tree child if there's a use case for that).

A potential rendered legend would always be a BFC and shrink-wrap, maybe even force display:block (if not display:none or display:contents).

Per https://drafts.csswg.org/css-display/#unbox-html , CSS WG consensus is that display:contents should work on fieldset and legend. I think it would be nice to have it work, and there's at least a recent tweet wishing for the same. So how about display:contents on fieldset means it can't have any potential rendered legend, and a legend with display:contents has no magic.

Member

zcorpan commented Jun 19, 2017

I would be OK with having a concept of "potential rendered legend", which is any legend that is a child of a fieldset (DOM child, possibly flat tree child if there's a use case for that).

A potential rendered legend would always be a BFC and shrink-wrap, maybe even force display:block (if not display:none or display:contents).

Per https://drafts.csswg.org/css-display/#unbox-html , CSS WG consensus is that display:contents should work on fieldset and legend. I think it would be nice to have it work, and there's at least a recent tweet wishing for the same. So how about display:contents on fieldset means it can't have any potential rendered legend, and a legend with display:contents has no magic.

@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Jun 19, 2017

Collaborator

And then I realized that this change made non-rendered legends more magical than before, in a sense

Ah. Again, what is the behavior in other UAs?

So how about display:contents on fieldset means it can't have any potential rendered legend, and a legend with display:contents has no magic.

Again, what is the expected rendering of:

<fieldset>
  <fieldset style="display:contents;"><legend>Something</legend></fieldset>
</fieldset>

and why?

Collaborator

bzbarsky commented Jun 19, 2017

And then I realized that this change made non-rendered legends more magical than before, in a sense

Ah. Again, what is the behavior in other UAs?

So how about display:contents on fieldset means it can't have any potential rendered legend, and a legend with display:contents has no magic.

Again, what is the expected rendering of:

<fieldset>
  <fieldset style="display:contents;"><legend>Something</legend></fieldset>
</fieldset>

and why?

@zcorpan

This comment has been minimized.

Show comment
Hide comment
@zcorpan

zcorpan Jun 20, 2017

Member

The outer fieldset renders as a normal fieldset (without a legend); the inner fieldset honors display:contents, and the legend is a normal block (because "display:contents on fieldset means it can't have any potential rendered legend").

I can work on a pull request and tests to make things clearer. I'll also check WebKit and EdgeHTML.

Member

zcorpan commented Jun 20, 2017

The outer fieldset renders as a normal fieldset (without a legend); the inner fieldset honors display:contents, and the legend is a normal block (because "display:contents on fieldset means it can't have any potential rendered legend").

I can work on a pull request and tests to make things clearer. I'll also check WebKit and EdgeHTML.

@zcorpan

This comment has been minimized.

Show comment
Hide comment
@zcorpan

zcorpan Aug 13, 2018

Member

Per comments in #3331 it appears that this is implemented differently to what I suggested would happen above. In particular, the with example above, the outer fieldset has the legend as its rendered legend in WebKit/Chromium/Gecko. (EdgeHTML 17 doesn't support display: contents.)

Member

zcorpan commented Aug 13, 2018

Per comments in #3331 it appears that this is implemented differently to what I suggested would happen above. In particular, the with example above, the outer fieldset has the legend as its rendered legend in WebKit/Chromium/Gecko. (EdgeHTML 17 doesn't support display: contents.)

zcorpan added a commit that referenced this issue Sep 19, 2018

Revamp how fieldset and legend rendering is defined
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.

Tests:
https://github.com/web-platform-tests/wpt/tree/master/html/rendering/non-replaced-elements/the-fieldset-and-legend-elements
https://github.com/web-platform-tests/wpt/tree/master/html/semantics/forms/the-fieldset-element
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment