HTML: test inline margin on fieldset's legend #12569

zcorpan · 2018-08-20T12:54:55Z

See whatwg/html#3929

annevk

So the top margin ends up affecting the position of the fieldset, but bottom margin affects the height of the fieldset?

zcorpan · 2018-08-20T14:36:09Z

Yeah, that's the effect of it.

annevk

Seems weird, but if that's what implementations do...

zcorpan · 2018-08-20T17:09:33Z

It's what Edge does, and what I think is most useful. WebKit and Chromium ignore margin-top, which is not so useful. Gecko positions the fieldset's top border centered around the legend's margin box (as opposed to border box), which is not so visually pleasing as the legend ends up hanging inside or outside the fieldset.

mstensho · 2018-08-20T20:23:16Z

html/rendering/non-replaced-elements/the-fieldset-element-0/legend-margin-block.html

+   left: 0;
+   right: 0;
+   top: 150px;
+   height: 220px;


Is this really behavior we want to standardize on? Looks like this test expects the legend's top margin to collapse with that of the fieldset, even though the fieldset has a border, AND both elements establish a BFC.

FWIW, I'd personally be okay with ignoring margin altogether (or just ignoring margin-top) as well.

I guess we can ignore margin in the block flow direction. @MatsPalmgren @dholbert @emilio any opinions?

I don't find the LEGEND block margins useful at all. Since interoperability is so lousy in this regard anyway, it would be tempting to say that we just ignore them.

Bug where margin-bottom was implemented for WebKit
https://bugs.webkit.org/show_bug.cgi?id=35981

Thanks. There's nothing in that bug report that points out the usefulness of supporting margin-bottom on legend, though, is there? Might as well use padding-top on the fieldset, right?

Yeah, I agree.

Thanks. There's nothing in that bug report that points out the usefulness of supporting margin-bottom on legend, though, is there? Might as well use padding-top on the fieldset, right?

Seems like it -- though in Firefox, you can use margin-bottom values to move the legend outside of the fieldset's border entirely (which you can't do with padding-top, but you could achieve with relative positioning instead). That special margin-bottom behavior doesn't seem to happen in other browsers, so I'm happy to see that behavior go, anyway.

One could use relative positioning to achieve that if desired.

As per web-platform-tests/wpt#12569 (comment)

MatsPalmgren · 2018-08-21T21:15:19Z

Using relative positioning or padding to a simulate a legend margin seems like a hack to me.

I tend to think we should support block-axis margins on the rendered legend. It's the most logical property to reach for when you want to create some distance to the surrounding content and it would be inconsistent if it doesn't work on legends when it applies to all other boxes (except inlines). Exceptions like this makes the web platform harder to understand and teach IMO. Margin properties are working fine on <caption> and I see no reason why we can't make it work similarly on <legend>.

Also, margin-bottom works fine in both Firefox and Chrome currently and I see no reason to change that.

dholbert · 2018-08-21T21:43:35Z

margin-bottom works fine in both Firefox and Chrome currently and I see no reason to change that.

It does something quite different in Firefox vs Chrome right now, FWIW. Testcase: https://jsfiddle.net/28xhe3sj/

In Firefox, the legend is way outside the fieldset border there, for reasons that @zcorpan alluded to above. Whereas in Chrome, on the other hand, it just renders as if there were extra padding at the top of the fieldset's content box.

Using relative positioning or padding to a simulate a legend margin seems like a hack to me.

The margin-bottom usage/behavior feels like more of a hack to me than a more declarative padding/relpos setup, FWIW. The effect of <legend style="margin-bottom:Npx"> in Firefox right now is pretty bizarre -- it offsets the fieldset-border downwards by N/2 px, and then it offsets the fieldset's contents downward by the remaining N/2 px. If a site really wants the "legend detatched from the fieldset" look that Firefox (and only Firefox) gives them, it seems more direct for them to use relative positioning to shift the legend upwards by a precise amount, and to not arbitrarily offset the fieldset's contents downwards unless they really want that as well (in which case they can specify whatever fieldset padding they want).

zcorpan · 2018-09-11T17:35:58Z

@MatsPalmgren can we land this?

zcorpan · 2018-09-14T10:59:01Z

@MatsPalmgren @mstensho PTAL

mstensho · 2018-09-14T11:15:29Z

I can't build the spec anymore, caused by the last commit:

$ ../html-build/build.sh
[...]
Generating HTML variant...
Error: Unused reference: [COMPAT]
Error count: 1
Saving index-html
Splitting...
Saving index.html
Saving introduction.html
[...]
Saving acknowledgements.html
Generating DEV variant...
Warning: Could not find ID selector-read-only for annotation that uses URLs:
https://caniuse.com/#feat=css-read-only-write
Splitting...
Saving index.html
Saving introduction.html
[...]
Saving acknowledgements.html
Generating SNAP variant...
Error: Unused reference: [COMPAT]
Error count: 1
Saving index-snap

There were errors. Stopping.

zcorpan · 2018-09-14T11:37:41Z

Oops, fixed.

As per web-platform-tests/wpt#12569 (comment)

zcorpan · 2018-09-17T22:26:35Z

I had forgotten about this and made #13040 so now I've removed the block margin tests here.

mstensho · 2018-09-18T06:02:12Z

Can you rename the commit comment to say "inline margins", then?

zcorpan added the html-fieldset label Aug 20, 2018

zcorpan requested a review from mstensho August 20, 2018 12:54

wpt-pr-bot added the html label Aug 20, 2018

wpt-pr-bot requested review from annevk, domenic, foolip, jdm, jgraham, sideshowbarker and zqzhang August 20, 2018 12:55

annevk reviewed Aug 20, 2018

View reviewed changes

annevk approved these changes Aug 20, 2018

View reviewed changes

mstensho reviewed Aug 20, 2018

View reviewed changes

zcorpan added a commit to whatwg/html that referenced this pull request Aug 21, 2018

Ignore margins on the legend in the block-flow direction

ddbd0a1

As per web-platform-tests/wpt#12569 (comment)

annevk added the do not merge yet label Aug 21, 2018

zcorpan removed the do not merge yet label Aug 21, 2018

wpt-pr-bot requested a review from emilio August 27, 2018 13:52

zcorpan force-pushed the zcorpan/fieldset-legend-margin branch from 0e1a69f to d5c5158 Compare August 27, 2018 13:54

HTML: test margin on fieldset's legend

6768754

zcorpan force-pushed the zcorpan/fieldset-legend-margin branch from d5c5158 to 6768754 Compare August 27, 2018 13:55

zcorpan added the status:needs-spec-decision label Aug 27, 2018

Don't ignore block margins per whatwg/html@16d86e5

92d6d38

zcorpan removed the status:needs-spec-decision label Sep 14, 2018

zcorpan added a commit to whatwg/html that referenced this pull request Sep 14, 2018

Ignore margins on the legend in the block-flow direction

39d2eee

As per web-platform-tests/wpt#12569 (comment)

Remove block margin tests

6bbc140

zcorpan changed the title ~~HTML: test margin on fieldset's legend~~ HTML: test inline margin on fieldset's legend Sep 18, 2018

zcorpan merged commit c033091 into master Sep 18, 2018

zcorpan deleted the zcorpan/fieldset-legend-margin branch September 18, 2018 15:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HTML: test inline margin on fieldset's legend #12569

HTML: test inline margin on fieldset's legend #12569

zcorpan commented Aug 20, 2018 •

edited

Loading

annevk left a comment

zcorpan commented Aug 20, 2018

annevk left a comment

zcorpan commented Aug 20, 2018

mstensho Aug 20, 2018

annevk Aug 21, 2018

zcorpan Aug 21, 2018 •

edited

Loading

mstensho Aug 21, 2018

zcorpan Aug 21, 2018

zcorpan Aug 21, 2018

mstensho Aug 21, 2018

zcorpan Aug 21, 2018

dholbert Aug 21, 2018

zcorpan Aug 21, 2018

MatsPalmgren commented Aug 21, 2018

dholbert commented Aug 21, 2018 •

edited

Loading

zcorpan commented Sep 11, 2018

zcorpan commented Sep 14, 2018

mstensho commented Sep 14, 2018

zcorpan commented Sep 14, 2018

zcorpan commented Sep 17, 2018

mstensho commented Sep 18, 2018

HTML: test inline margin on fieldset's legend #12569

HTML: test inline margin on fieldset's legend #12569

Conversation

zcorpan commented Aug 20, 2018 • edited Loading

annevk left a comment

Choose a reason for hiding this comment

zcorpan commented Aug 20, 2018

annevk left a comment

Choose a reason for hiding this comment

zcorpan commented Aug 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zcorpan Aug 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MatsPalmgren commented Aug 21, 2018

dholbert commented Aug 21, 2018 • edited Loading

zcorpan commented Sep 11, 2018

zcorpan commented Sep 14, 2018

mstensho commented Sep 14, 2018

zcorpan commented Sep 14, 2018

zcorpan commented Sep 17, 2018

mstensho commented Sep 18, 2018

zcorpan commented Aug 20, 2018 •

edited

Loading

zcorpan Aug 21, 2018 •

edited

Loading

dholbert commented Aug 21, 2018 •

edited

Loading