I don't see any reason not to support grid and flexbox layout for <fieldset>. data:text/html,<fieldset style="display:flex"> data:text/html,<fieldset style="display:grid"> Dumping the frame tree for either of those results in: FieldSet(fieldset) ... < Block(fieldset) ... < If we make the inner frame there a nsGrid/FlexContainerFrame instead it should just work I think.
Created attachment 8695965 [details] [diff] [review] wip This seems to work fine when I test it briefly with display:grid. Also with display:grid;overflow:scroll. Boris, do you see any problems from a frame construction POV?
Assignee: nobody → mats
Attachment #8695965 - Flags: feedback?(bzbarsky)
Created attachment 8696051 [details] [diff] [review] fix I decided to keep 'display:block' on the pseudo frame rather than inherit it. It seems safer to have nsGrid/FlexContainerFrame with mDisplay == NS_STYLE_DISPLAY_BLOCK, rather than having a nsBlockFrame with an arbitrary mDisplay. I guess we could hack ApplyStyleFixups for this too, if necessary. Daniel, do you see any problem with FlexContainerFrame having the "wrong" mDisplay?
Comment on attachment 8696051 [details] [diff] [review] fix It really worries me to have these frames not matching their display types. I would much rather we had a magic -moz-prefixed "display" value (restricted to UA sheets, if possible) that acted like "inherit" in some cases and "block" in others. Or you could take the big hunk that already does different display fixeups based on aContext->GetPseudo() in nsRuleNode::ComputeDisplayData and add some more stuff to that, since you have a nice separate pseudo type here. Apart from that, I assume the scrollframe handling of flex/grid is ok here? In any case, someone who actually knows something about flex and grid should review the forms.css change.
Attachment #8696051 - Flags: review?(bzbarsky) → review?(dholbert)
Comment on attachment 8696051 [details] [diff] [review] fix The forms.css changes looks fine to me, assuming they're copypasted from the -moz-scrolled-content section of ua.css for flexbox, grid, & align. (looks like they are.) Might be worth adding a comment in both instances of this copypasted list saying "Note: This should match the corresponding section in $FILE.css for $PSEUDO-ELEMENT", so that any changes to one of these instances will be more likely to be correctly copied over to the other instance. r=me, anyway, for the forms.css stuff. I assume there's still an implicit r?bz here on the actual C++ changes, pending answers to his questions in comment 6.
Attachment #8696051 - Flags: review?(dholbert) → review+
Created attachment 8696249 [details] [diff] [review] fix (In reply to Boris Zbarsky [:bz] from comment #6) > Apart from that, I assume the scrollframe handling of flex/grid is ok here? Yep, scrolling works fine.
Comment on attachment 8696249 [details] [diff] [review] fix r=me. This makes me much happier, thanks! And sorry for the lag...
Attachment #8696249 - Flags: review?(bzbarsky) → review+
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Duplicate of this bug: 949476
Shouldn't https://bugzilla.mozilla.org/show_bug.cgi?id=1047590 also be marked as duplicate of this?
Is there a way for the legend of a fieldset to be taken in account by flexbox as well ? Here is a test case : https://codepen.io/mh-nichts/pen/VWLjjm The fieldset has display:flex, and the divs inside behave accordingly (they are distributed evenly on the whole width), but the legend doesn't (it stays at the top of the fieldset, as a block element). It looks like flexbox is applied only to the elements after the legend (as if they were in a separate invisible container that has display:flex itself).
You mean the alignment of the <legend> should be derived from the <fieldset>'s justify-content value? That's an interesting idea! As far as I know, there is nothing in the CSS specs about that. (as a workaround you can do this: <legend align="center">)
> as if they were in a separate invisible container that has display:flex itself That is in fact exactly how it works in terms of the implementation. In terms of what the spec says, it's defined at https://html.spec.whatwg.org/multipage/rendering.html#the-fieldset-and-legend-elements which certainly sounds like the placement of the rendered <legend> should not be affected by the display of the fieldset. Note that if you make a <legend> not be the rendered legend by floating it, that might do what you want, because the flex container will then proceed to ignore the float style and just make it a flex item. At least in Firefox; browser compat around this stuff is pretty poor. For example, that technique will fail in Chrome due to <https://bugs.chromium.org/p/chromium/issues/detail?id=350505> and in Safari due to <https://bugs.webkit.org/show_bug.cgi?id=172999>. And Edge seems to not really support fieldset being a flex container so far...
Thank you, the float did it for Firefox ! Yes I observed the compatibility issues, surprisingly only IE10-11 is applying flexbox the way I expected it, but Edge and Chrome don't do anything at all for the fieldset elements. Thinking more about it and after reading the spec, I agree with what you say : not sure there is something that should be implemented to force the legend to "flex" like the others, because of its special positioning by default. I guess the float-workaround is also enough to adapt to the various needs.
FYI, Chrome doesn't support display:flex on <fieldset> yet. https://bugs.chromium.org/p/chromium/issues/detail?id=375693
You need to log in before you can comment on or make changes to this bug.