Per summary; testcase coming up, plus a patch sometime soon. I think the testcase shouldn't show scrollbars, since there's nothing to scroll to, but it does in all current browsers. This is a significant change in behavior, and will make us incompatible with IE and Opera; however, I think it makes a lot more sense than what we're doing now. There might be a bug filed on this already, although I couldn't find it.

This patch has the necessary changes plus some cleanup to the invalidation logic made possible by this change. I'm not completely sure that we want to do this; it does seem logical, but it's not consistent with the way both Gecko and other browsers (I tested Opera and IE) deal with clip and overflow. I believe this patch is safe... the places that use overflow rects for purposes other than painting won't be affected. I'll check in the testcase attached to this bug as a reftest along with the patch.

Does this cause assertions to fire if you have an element that uses 'clip' and 'overflow:auto'? I guess not because it's the scrollframe that ends up clipped, not the scrolled frame. Maybe we should get some other opinions on whether we should do this.

(In reply to comment #2) > Does this cause assertions to fire if you have an element that uses 'clip' and > 'overflow:auto'? I guess not because it's the scrollframe that ends up clipped, > not the scrolled frame. It doesn't assert, but it's broken on trunk, and has been for a long time. Guess that's another bug to file. > Maybe we should get some other opinions on whether we should do this. Agreed... where should we ask?

Hmm, I tried searching www-style. Found http://lists.w3.org/Archives/Public/www-style/2003Aug/0075, which seems to be this issue; unfortunately, nobody replied... I guess I should CC dbaron and bz at the very least.

Good catch. WG input would certainly be useful here...

Comment on attachment 256752 [details] [diff] [review] Patch Dropping the review request for the moment because there's an invalication bug switching from non-auto clip to auto clip. (I have a fixed version I'll post sometime soon.)

Hmm, didn't really get much of a response off the www-style list; only one person seemed to have anything to say, and he was mostly asking for clarifications. Is that an implicit approval?

I think so. Let's go for it. I can't think of any issues that this would cause with real content, off the top of my head.

Just make sure we don't clip things that the 'clip' property doesn't apply to. (It looks like you're ok on that, but you might want to add tests for it to reftest.)

+ hasAbsPosClip = GetAbsPosClipRect(GetStyleDisplay(), &absPosClipRect); There's a GetStyleDisplay() earlier on which you should reuse ("disp"). Actually we should reuse it here too: if (geometricOverflow && GetStyleDisplay()->mOverflowX == NS_STYLE_OVERFLOW_CLIP) { and in the NS_ASSERTION as well. + if (hasOutline || geometricOverflow || hasAbsPosClip) { Why are you adding hasAbsPosClip here? If we didn't already overflow, we won't start overflowing just because there's a clip.

(In reply to comment #11) > + hasAbsPosClip = GetAbsPosClipRect(GetStyleDisplay(), &absPosClipRect); > > There's a GetStyleDisplay() earlier on which you should reuse ("disp"). > Actually we should reuse it here too: > if (geometricOverflow && > GetStyleDisplay()->mOverflowX == NS_STYLE_OVERFLOW_CLIP) { > and in the NS_ASSERTION as well. Okay. > + if (hasOutline || geometricOverflow || hasAbsPosClip) { > > Why are you adding hasAbsPosClip here? If we didn't already overflow, we won't > start overflowing just because there's a clip. If hasAbsPosClip is true, the overflow rect could be different from the frame boundaries. Note the the overflow rect can end up being smaller than the frame's rect.

Ah, hrm. That doesn't sound good. We may find bugs where we're assuming that the overflow area contains the frame bounds. But I guess they are most likely to manifest in relatively innocuous ways.

I think this addresses all your comments so far.

Comment on attachment 259573 [details] [diff] [review] Patch v3 Can you add a comment to the definition of NS_FRAME_OUTSIDE_CHILDREN that it can be set when the "overflow" rect is *smaller* than the frame size, as well?

I'll check this in sometime soon. (Note to self: make sure to check in the right nsIFrame.h.)

Checked in.