User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20040913 Firefox/0.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20040913 Firefox/0.10
When assigning a value other than visible to the overflow property of a fixed
height/width fieldset its content is rendered as if overflow: visible was specified.
Reproducible: Always
Steps to Reproduce:
1. add a fieldset element with its style set to "overflow: scroll; height: 2em;"
2. open the document and notice the lack of (disabled) scrollbars
Actual Results:
The overflowing content of the fieldset is visible.
Expected Results:
The overflowing content of the fieldset should be clipped and, in case of
scroll/auto, scrollbars should be added to the fieldset.
bernd on IRC requested to add "its probably a frame construction bug".

>severe impact
Hey come on, look at the bug number, this has never worked. It took 6 years of
the project to get this bug filed, before you nobody noticed the "severe
impact". I am not arguing against the bug or against fixing it, but severe is
something else.

(In reply to comment #5)
> >severe impact
That should have read 'the "severe impact"', and if I really felt this bug to be
a showstopper I would have set the Severity to either critical or blocker. I was
quite happy with the inital testcase as it did show the issue in the least lines
of code but someone convinced me to changing it to display data falling out of
it. I couldn't care less whether or not this bug is ever fixed, knowing that it
exists and the workaround (adding an overflowing div around the content) is
enough for me. To reiterate: "severe impact" wasn't meant as a Severity rating
on this bug, only a out-of-context quote. Same applies to the "public demand".

overflow:hidden seem to work now. well kind of work... text rows are not cut off
at correct point if they should be cut off in the middle of the row).
overflow:scroll still broken.
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b5) Gecko/20050929 Firefox/1.4
ID:2005092905

I just triggered this bug today; reading scrollHeight from an overflowing fieldset didn't return any useful information, since the fieldset never overflowed! Obscure enough that it'd rarely be an issue, but major enough that I need to specifically code around it. I'd also like to see this bug fixed.

Created attachment 449431[details]
Fieldset overflow:hidden testcase
I came across this bug today. In a form i have a fieldset with fixed height and overflow: hidden but the content does not get clipped. This problem is reproduceable sice Firefox 3.6.x. In older Versions like 3.5.8 oder 3.0.16 the problem does not exist for me. I attached another testcase.
Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3 (.NET CLR 3.5.30729)

Created attachment 562954[details]
Fieldset overflow and dynamic content testcase
This test case shows the problem to be even worse when there is dynamic content involved. Not only does the fieldset not crop its contents with overflow: hidden, but any protruding content is not erased and redrawn correctly if the fieldset moves. This can lead to artefacts being left on the page as the fieldset is moved around.

I came across this today on OS X version 7.0.1
Like others, if I replaced the <fieldset> with <div> with overflow:hidden the overflowing content was hidden correctly, otherwise it was shown as visible.

Dynamically changing page content above a fieldset no longer leaves artefacts on the page, as mentioned in my comment #16 above.
Overflow is still not implemented for fieldsets, so this bug is still valid.

EIGHT YEARS ARE YOU FICKING KIDDING ME????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????
WHAT THE **** MAN

Comment on attachment 817599[details][diff][review]
fix v2
>layout/base/nsCSSFrameConstructor.cpp
>+ fieldsetFrame->AddStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN);
>+ if (fieldsetFrame->IsPositioned()) {
>+ aState.PushAbsoluteContainingBlock(fieldsetFrame, fieldsetFrame, absoluteSaveState);
> }
Hmm, I think we want a positioned scrolled fieldset to behave the same
as an ordinary positioned scroll frame where the inner scrolled frame
acts as the abs.pos. containing block.
If the fieldset frame is the containing block then overflow contributions
from positioned children are not taken into account by the scroll frame.
See attached reftest, which I think should pass.
>- (!aParentFrame ||
>- (aParentFrame->GetType() != nsGkAtoms::fieldSetFrame &&
>- aParentFrame->StyleContext()->GetPseudo() !=
>- nsCSSAnonBoxes::fieldsetContent) ||
>+ (!IsFrameForFieldSet(aParentFrame) ||
I would prefer to keep the explicit "!aParentFrame" here and remove
the null-check in IsFrameForFieldSet.
>layout/forms/nsFieldSetFrame.cpp
>+ virtual nsIScrollableFrame* GetScrollTargetFrame() MOZ_OVERRIDE
>+ {
>+ return do_QueryFrame(GetInner());
>+ }
Doesn't this require nsIScrollableFrame.h to compile? If so, I think we
should add that explicitly to the list of #includes instead of depending
on some other header doing so. (I might mistaken though)
>layout/generic/nsHTMLReflowState.cpp
Seems unnecessary to touch this file just to remove a space.

Created attachment 818033[details][diff][review]
some overflow:auto reftests
I thought it would be good to have some overflow:auto tests as well,
so I wrote some. This also demonstrates the abs.pos. error described
above. Please add this to the patch set.

(In reply to Mats Palmgren (:mats) from comment #39)
> FYI, while testing this I also discovered that we get the fieldset height
> wrong
> in an edge case, bug 927484. It's unrelated to your changes though, but feel
> free to have a look while you're here ;-)
I'd rather not turn this into a can of worms, thanks :-)

Yeah, I didn't mean to suggest it should be fixed in this bug. Just that it might be easy
to fix next while you have this code fresh in your head. It's an unimportant edge case though
so it can wait...

(In reply to Mats Palmgren (:mats) from comment #37)
> Created attachment 818033[details][diff][review]
> some overflow:auto reftests
>
> I thought it would be good to have some overflow:auto tests as well,
> so I wrote some. This also demonstrates the abs.pos. error described
> above. Please add this to the patch set.
OK, added.

Created attachment 820669[details][diff][review]
fix v3
Fixed the things I mentioned above. Also fixed the dynamic legend issues by calling IsFrameForFieldSet in a couple more places in nsCSSFrameConstructor. I made it take aFrame's type as a parameter to save a virtual call for the new callers.

> Are you sure about the change of Target Milestone ?
Yeah, I'd just changed it from 27 to 28, then realized this has been in nightlies all through 27, but got backed out at beta, so 27 is the right TM but we're not shipping this until 28.

(In reply to Boris Zbarsky [:bz] from comment #61)
> > Are you sure about the change of Target Milestone ?
>
> Yeah, I'd just changed it from 27 to 28, then realized this has been in
> nightlies all through 27, but got backed out at beta, so 27 is the right TM
> but we're not shipping this until 28.
So then this should be "disabled" for status-firefox27, shouldn't it?

(In reply to Boris Zbarsky [:bz] from comment #63)
> No idea when to use "disabled" vs "wontfix"....
Wontfix applies when a code has not and will not be landed/uplifted for the respective Firefox version. Disabled applies when code was landed for that version but disabled, usually behind a pref.
I think in this case firefox27:disabled applies. Please correct me if you think otherwise.

Note

You need to
log in
before you can comment on or make changes to this bug.