Skip to content
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

Does <legend> element establish a new BFC? #2519

Closed
yisibl opened this issue Apr 11, 2017 · 25 comments
Closed

Does <legend> element establish a new BFC? #2519

yisibl opened this issue Apr 11, 2017 · 25 comments
Labels
interop Implementations are not interoperable with each other topic: rendering

Comments

@yisibl
Copy link

yisibl commented Apr 11, 2017

In HTML5 spec said:

 The `<fieldset>` element is expected to establish a new block formatting context.

https://www.w3.org/TR/html5/rendering.html#the-fieldset-and-legend-elements

What about the legend element?

Chrome and Safari will establish a BFC, see Blink code.

bool LayoutBox::AvoidsFloats() const {
  // crbug.com/460704: This should be merged with createsNewFormattingContext().
  return ShouldBeConsideredAsReplaced() || HasOverflowClip() || IsHR() ||
         IsLegend() || IsWritingModeRoot() || IsFlexItemIncludingDeprecated() ||
         Style()->GetColumnSpan() == kColumnSpanAll ||
         Style()->ContainsPaint() || Style()->ContainsLayout() ||
         Style()->Display() == EDisplay::kFlowRoot;
}

Testcase http://jsbin.com/kocubixuzu/edit?html,css,output

image

image

I think the specification should be clear, to avoid incompatibility between browsers.


Summary for this issue:

PR for the standard: #2718
web-platform-tests: web-platform-tests/wpt#6125
Browser bugs:
https://bugs.chromium.org/p/chromium/issues/detail?id=727378
https://bugzilla.mozilla.org/show_bug.cgi?id=1368723
https://bugs.webkit.org/show_bug.cgi?id=172718
https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/12174566/

@annevk
Copy link
Member

annevk commented Apr 11, 2017

cc @dbaron

@annevk
Copy link
Member

annevk commented Apr 11, 2017

https://html.spec.whatwg.org/multipage/rendering.html#the-fieldset-and-legend-elements seems pretty clear on how the legend element is expected to be rendered by the way, but I guess your issue is that it does not match Chrome?

@annevk annevk added the interop Implementations are not interoperable with each other label Apr 11, 2017
@zcorpan
Copy link
Member

zcorpan commented Apr 11, 2017

I think legend that is not the "rendered legend" should have minimum amount of magic, to make it possible to reuse for something else in the future. (A number of years ago the spec used legend in figure and details, IIRC, but was changed because browsers had too much magic for legend to make it unpractical to use outside fieldset.)

Although CSS recently gained ability to make any element into a flow root (in author style sheet), I think it still doesn't have ability to take away flow root from an element that has it by default (like fieldset), right?

@yisibl
Copy link
Author

yisibl commented Apr 11, 2017

@annevk Yes, I don't understand why Chrome is rendered here as BFC. Which browser is in line with the spec?

@zcorpan
Copy link
Member

zcorpan commented Apr 12, 2017

As far as creating a BFC or not goes, Firefox is like the spec. (edit: for the test case in OP)

@mstensho
Copy link

Reported bug https://bugs.chromium.org/p/chromium/issues/detail?id=727378 for Chrome.

We should at least refrain from creating a BFC if the LEGEND isn't the rendered LEGEND of a FIELDSET.

@zcorpan
Copy link
Member

zcorpan commented May 30, 2017

Hmm, this appears to be a bit weird. legend in WebKit/Chromium is not exactly a BFC. For the aspect of growing to contain its floats, it doesn't act like a BFC. It does avoid floats when positioning though. However in Gecko, a legend that is a child of fieldset appears to be a BFC. In EdgeHTML legend is always a BFC it seems.

See http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5207

@zcorpan
Copy link
Member

zcorpan commented May 30, 2017

There is a 2/2 split for containing floats for the "rendered legend". So we could decide either way on whether the rendered legend should be a BFC I think.

For the aspect of positioning the "rendered legend" to avoid earlier floats, I think it is not observable if it is a BFC since fieldset is a BFC and you can't get a sibling float before the "rendered legend" in the layout. (http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5208)

@mstensho
Copy link

Right, forgot to mention: I'm working on cleaning up some code in Blink [1], where legend previously was in a hybrid state - almost establishing a new formatting context, but not quite [2]. Which is how I found this discussion here, because I couldn't find any spec suggesting LEGEND to establish a formatting context. Since Blink currently relies on some BFCness for a LEGEND rendered inside a FIELDSET, they'll now become proper block formatting contexts.

[1] https://chromium-review.googlesource.com/c/512824/

[2] Blink has (i.e. soon to be cleaned up and removed) two characteristics of relevance here, exposed via CreatesNewFormattingContext() and AvoidsFloats(). In most cases, for most object types, CreatesNewFormattingContext() == AvoidsFloats(), but there were some exceptions, like for LEGEND and HR. A proper block formatting context prevents surrounding floats from entering the block formatting context (as in affecting layout), and prevents floats inside the block formatting context from escaping into surrounding boxes. AvoidsFloats() only makes sure of the former. We really don't need any object to behave like that.

@zcorpan
Copy link
Member

zcorpan commented May 30, 2017

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5217 -- EdgeHTML does not use the same trick for legend as it does for hr.

@zcorpan
Copy link
Member

zcorpan commented May 30, 2017

Created a PR at #2718

@mstensho can you move the test to external/wpt/html/rendering/non-replaced-elements/the-fieldset-element-0/ or some such? It will then be automatically upstreamed to web-platform-tests after it has landed in Chromium. Having actual tests for the cases discussed in this thread would also be nice.

I can file bugs for other engines to implement this and ask to raise any concerns.

@mstensho
Copy link

https://chromium-review.googlesource.com/c/512824/ isn't going to fix this! It's about code cleanup, and the idea was to not make more changes than necessary. The only change I settled on, was to get rid of this "avoid floats, but don't actually establish a new block formatting context" thing (currently used with LEGEND and HR), because it just seemed too bogus to keep. Yes, with 512824, LEGEND will create a proper block formatting context, but this will also happen when it's not a rendered fieldset legend.

@zcorpan
Copy link
Member

zcorpan commented May 30, 2017

Thanks for clarifying! Ok so then a new issue for chromium is necessary. I can file tomorrow and maybe work on tests.

Edit: the issue seems to capture the spec change, so not filing a new issue for now.

@MatsPalmgren
Copy link

Wouldn't it be less magical if a <legend> always establish a BFC, rather depending on where it happens to be in the DOM? We could do this by saying that the UA sheet has:

legend { display: flow-root; ... }

This allows the author to override that as they please, regardless of if it's a "rendered legend" or not.

CC @bzbarsky

@domenic
Copy link
Member

domenic commented May 31, 2017

In general, the problem with putting things in the UA stylesheet is then UAs actually have to implement it that way, e.g. getComputedStyle() has to return the correct values. Since nobody does that today, we'd need multi-implementer interest to do so.

@mstensho
Copy link

mstensho commented May 31, 2017

Not exactly the same, but a similar clarification happened recently for column-span:all and whether it should create a BFC or not. It should. Always. Chrome used to decide depending on whether it ended up as a proper column spanner or not (i.e. it had to be an element in a block formatting context established by a multicol container). See w3c/csswg-drafts#1074

@dbaron
Copy link
Member

dbaron commented May 31, 2017

It's also tricky because we'd then need to define the behavior for a rendered legend that isn't a BFC.

@bzbarsky
Copy link
Contributor

I was under the impression that random <legend> was meant to have a minimum of weird because it was used in places other than <fieldset> in the spec. If that's not the case anymore, then we should do whatever is simplest to spec and implement, basically.

That said, I suspect that things that depend on the fieldset's displayed legend being a BFC, if any, also depend on it staying a BFC even if its display is set to "inline" or "block" in author styles. Certainly other aspects of displayed legend behavior depend on it behaving pretty independently of its specified (and computed!) display value. So I'd be pretty worried about the compat implications there.

Given that, and that as a result we probably need something explicit to make a display: inline rendered legend a BFC anyway, we might as well just explicitly make rendered legends BFCs somewhere.

@zcorpan
Copy link
Member

zcorpan commented Jun 1, 2017

The spec forces the "rendered legend" to be a block box, and indeed the idea is to have minimum magic for legend that is not the rendered legend.

A fieldset element's rendered legend, if any, is expected to be rendered over the block-start border edge of the fieldset element as a block box (overriding any explicit 'display' value).

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

There isn't anything currently that uses legend for a different purpose, because last time that was tried (in details and figure IIRC) it wasn't usable because browsers had too much magic for legend. As long as we still have magic for legend outside fieldset, we're prevented from reusing it for a different purpose in the future.

@zcorpan
Copy link
Member

zcorpan commented Jun 1, 2017

@MatsPalmgren it's not clear that it is possible or if there's enough interest in making the "rendered legend" have less magic. And implementations needs to be aware of where it is in the DOM anyway, because only one legend element is placed over the fieldset's border. So I think the best we can do is remove all the magic for other legend elements.

@zcorpan
Copy link
Member

zcorpan commented Jun 1, 2017

Related: #2729

@zcorpan
Copy link
Member

zcorpan commented Jun 1, 2017

I have now written tests for this.

HTML PR: #2718
WPT PR: web-platform-tests/wpt#6125

@yisibl
Copy link
Author

yisibl commented Jun 21, 2017

Chrome fixed in: https://chromium-review.googlesource.com/c/535595/ 👻

@zcorpan
Copy link
Member

zcorpan commented Jun 21, 2017

Nice!

Note there's an ongoing discussion about tweaking how these things should work at #2756

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Implementations are not interoperable with each other topic: rendering
Development

No branches or pull requests

8 participants