[webkit-reviews] review denied: [Bug 5909] overlapping element leaves trail when scrolling iframe (affects Gmail) : [Attachment 12985] check layer hierarchy for overlap

[bugzilla-request-daemon at macosforge.org]

Dave Hyatt <hyatt at apple.com> has denied Dave Hyatt <hyatt at apple.com>'s request
for review:
Bug 5909: overlapping element leaves trail when scrolling iframe (affects
Gmail)
http://bugs.webkit.org/show_bug.cgi?id=5909

Attachment 12985: check layer hierarchy for overlap
http://bugs.webkit.org/attachment.cgi?id=12985&action=edit

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
I should start off by saying that I don't understand why this bug is a P1,
since it's not a regression.

(1) The change to isStackingContext is not needed.  We already force the
z-index of the root element to be 0 (and don't allow it to be auto).  This
means it will always be a stacking context anyway.

(2) I'd prefer isRectangleOverlapped to rectangleIsOverlapped.

(3) if (l && l->rectangleIsOverlapped(IntRect(x, y, width(), height())))

You want contentWidth() and contentHeight() here.  The relevant box is the one
that excludes border and padding.  Your x and y values are correct.

(4) I like the idea behind this, but I think this is too inefficient.  For
pages with lots of layers and iframes, this will slow down painting.

(5) This approach will potentially put frames in the "penalty box" forever if a
layer ever overlaps them.  A good example of this would be DHTML menus that
come up on  top of an iframe.  If you open the menus and then close them, the
iframe will now scroll slowly all the time just because a DHTML menu was opened
on top of it at some point in the past.

My recommendation would be to defer this bug for now, since it really is a
pretty high-risk feature to be implementing now.

Ultimately I think you need to know how to turn fast repaints back on (we
manage to do this in the fixed positioned case as fixed positioned elements
come and go, so you could look to that code as an example), and be smarter
about caching knowledge of the overlap in the widget itself, only recomputing
overlap when layers actually change positions.