1.58 KB, text/html
236 bytes, text/html
37.69 KB, patch
chris hofmann: approval1.7b+
|Details | Diff | Splinter Review|
330 bytes, text/html
spun off from bug 203086. in the testcase, the wide table and div paint on top of the scrollbar. bz apparently has this figured out and will add more detail shortly...
This seems to be a regression from bug 225811 -- backing that patch out locally makes the testcase in bug 203086 happy again. Sounds like we need a different way to handle clipping by scroll views? Those _should_ clip the abs/fixed pos kids....
*** Bug 232341 has been marked as a duplicate of this bug. ***
Oh, and as expected it's the first hunk of the patch on bug 225811 that causes this bug.
The root scroll view is the only one that needs special treatment, right?
Not sure.... Should abs pos content really leak out of scrolling parents?
If the scrolling parent is not the containing block of the absolute content, then I believe CSS2.1 currently says that the absolute content will leak out. The root view is different because although in the frame hierarchy the scrollframe is a sibling of the fixed position content, logically the scrollframe is scrolling the viewport and thus should clip (although not scroll) the fixed position content.
Ah, ok. If it's not the containing block. That makes sense. Yeah, in that case this sounds like the root scroll view is what needs special treatment.
I think we need to add a new view property NS_VIEW_FLAG_CLIPCONTENTCHILDREN, set it on root scrolling views, and adjust nsView::GetClippedRect to take it into account. That will probably require a second pass up the view ancestor chain when non-containing-block ancestors are detected.
Created attachment 140344 [details] [diff] [review] patch Okay, here's the patch. Unfortunately it grew into a rework of most of the view clipping code because of various issues that came up. Hopefully this code will be less fragile than the old code turned out to be. This code identifies the three types of clipping we need to support and makes them all explicit. Thinking about this code exposed a latent bug in display list construction which I have a patch for and will file separately. One note: I moved the initialization of aTopView in CreateDisplayList() out to the call site in BuildDisplayList, where it ought to have been all along.
Comment on attachment 140344 [details] [diff] [review] patch Another views patch for you, David :-)
Created attachment 140348 [details] Another test Another test; I assume this patch handles this right?
that testcase is missing a </div> somewhere...
Created attachment 140371 [details] This is what I meant Note that in current builds the text paints on top of the scrollbar, which seems pretty bogus to me...
There seems to be a reflow bug in current builds. I'd better take a look at that. My patch is clipping the "Positioned" DIV though.
Created attachment 140393 [details] [diff] [review] revised patch After sleeping on the original patch, I realized there were a couple of issues. GetClippedView in the old patch optimized for the case when view->GetZParent() is an ancestor of view->GetParent(), which is the wrong case ... the case we should optimize for is when view->GetParent() is an ancestor of GetZParent(), which is what the old #if 0 code did. So I fixed that up. Also, the storage of clip rects in nsView has been made a bit simpler and more efficient.
I filed the reflow bug that causes text to overlap scrollbars as bug 232838.
*** Bug 233648 has been marked as a duplicate of this bug. ***
*** Bug 235950 has been marked as a duplicate of this bug. ***
*** Bug 155158 has been marked as a duplicate of this bug. ***
*** Bug 236488 has been marked as a duplicate of this bug. ***
This bug has several dups even though it hasn't existed long, and it's a regression, and the patch is nontrivial so I think it really needs 1.7b exposure.
Created attachment 143484 [details] Testcase Which of all those dependent bugs covers the case in the attachment (fixed pos div overlapping the bottom scrollbar)?
*** Bug 236998 has been marked as a duplicate of this bug. ***
Comment on attachment 140393 [details] [diff] [review] revised patch r+sr=dbaron. I may try to comment on the patch at a later date, but for now we're probably better off just getting it in so it gets tested.
Comment on attachment 140393 [details] [diff] [review] revised patch layout regression, needs 1.7b exposure
Comment on attachment 140393 [details] [diff] [review] revised patch a=chofmann for 1.7b
I think this checkin caused the DHTML problem in Bug 237426. With it removed the testcase in that bug shows a smooth gradually expanding square, with it applied the testcase jumps from a tiny square to a huge square.
Verified with firefox 0.10.1. My testcase from comment 23 is now fixed.