971933 - overflow: hidden on fieldsets spills over

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[nemo]

http://m8y.org/tmp/testcase367.xhtml

Encountered this with dl/dt/dd (sometimes) and table (always) under fieldsets.
http://m8y.org/tmp/testcase367.png is a screenshot with the dl/dt/dd and tables spilling over on the actual site.

Rendered correctly in IE9, Safari, Chrome and Opera 11

Comment 1

[nemo]

Oh, and also happened if display: inline-block was set, which I thought might possibly make it render the fieldset differently.

Comment 2

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Does this only happen when there's a <legend>?

If so, then this might be a dupe of (or at least strongly related to) bug 971655.

Comment 3

[nemo]

http://m8y.org/tmp/testcase368.xhtml

Legend removed.  Seems so?

Comment 4

[Mats Palmgren (inactive)]

Attachment: Testcase #2

I think the rendering of the inner frame (grey) is correct.
It's just that it's placed wrong - it should overlap the lower
half of the green area, i.e. the bottom padding.

Alternatively, that the green area is too small and should
be extended in this case so its bottom aligns with the grey area
in the same way.

Alternatively, that the green area should have its current size
but be placed lower.  I haven't investigated which makes the more
sense in this case, or what other UAs do in edge cases like this.

Comment 5

[Mats Palmgren (inactive)]

I think it's unrelated to bug 971655 though, which is an abs.pos. issue only.

Comment 6

[Mats Palmgren (inactive)]

Is this a regression from bug 261037, or have we always rendered it like this?

Comment 7

[nemo]

FYI to mats, as view source thoughtfully highlights, that 2nd testcase is invalid HTML, no idea if it would trigger quirks.  (this is the sort of thing that XHTML parsing would immediately warn you about ;) )

For some reason there's a closing tag called "height" in it :)

Comment 8

[nemo]

Oh, and WRT comment #5, while I have no idea as to internals of code, it does seem similar in that the area of overflow clipping seems identical to the area of the top/bottom/left/right 0 containing block, and both are offset by the label for some reason.
Soo, internal area of the fieldset pushed down by the label.  That seems like it might be the same cause, with the other bug just using position absolute to demo it.

Comment 9

[Mats Palmgren (inactive)]

Attachment: wip

This seems to fix it for me, and a bunch of similar cases too...

Comment 10

[Mats Palmgren (inactive)]

Attachment: Testcase #2

Comment 11

[Mats Palmgren (inactive)]

Attachment: screenshot of Testcase #2

This is what Testcase #2 looks like with the patch applied.
nemo, does this look like the correct rendering to you?

Comment 12

[nemo]

O_o  woooooah that screenshot and patch is making me dizzy.
Comparing back and forth between my nightly and the screenshot I see slight differences, but knowing what the "correct" behaviour is is another matter :)

I'd have a better idea if it was a screenshot of the original testcase ;)

BTW, I loaded up your tons-o-fieldsets testcase in chromium and it did not render the same. Rendered rather oddly in fact.
None of C6 had legends.
Don't have IE9 handy at the moment to get a 2nd opinion.  Could be chrome bugs.

Comment 13

[nemo]

Hm. IE9 agrees with Chrome on not showing the legend in C6.
They have other rendering disagreements like in the display of the blue and grey areas, but they both clip the legend text leaving whitespace above it.

I don't know what the correct behaviour is for your testcase, but I can say that 99% of the time I'd want to always see the legend.  But, yeah, your screenshot doesn't agree too much w/ existing implementations.  bz did say that there's no spec for fieldsets, so I guess that explains it :)

Opera 11 BTW renders something totally different, and does display the legends in C6.

Comment 14

[Mats Palmgren (inactive)]

Attachment: fix+reftest

https://tbpl.mozilla.org/?tree=Try&rev=e229dd016d1f

Comment 15

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/46223ef9b330

Comment 16

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/46223ef9b330

Comment 17

[Mats Palmgren (inactive)]

Comment on attachment 8376369 [details] [diff] [review]
fix+reftest

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 261037?
User impact if declined: layout errors with overflow:hidden on <fieldset>
Testing completed (on m-c, etc.): on m-c since 2014-02-15
Risk to taking this patch (and alternatives if risky): low risk
String or IDL/UUID changes made by this patch: none

Comment 18

[Mats Palmgren (inactive)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0a3fdcf8b7ef