[webkit-reviews] review requested: [Bug 56493] Drag-scrolling overlay scrollbars thumb in overflow regions does not work : [Attachment 88941] Patch that uses temporary clip rects

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Apr 9 19:40:49 PDT 2011


Beth Dakin <bdakin at apple.com> has asked  for review:
Bug 56493: Drag-scrolling overlay scrollbars thumb in overflow regions does not
work
https://bugs.webkit.org/show_bug.cgi?id=56493

Attachment 88941: Patch that uses temporary clip rects
https://bugs.webkit.org/attachment.cgi?id=88941&action=review

------- Additional Comments from Beth Dakin <bdakin at apple.com>
Here is a much better patch that uses temporary clip rects.

A few things about this patch:

1. Since this patch turned out to be so minimal, I am definitely open to
changing the name of the OverlayScrollbarSizeRelevancy enum in the same patch
if anyone has a better suggestion. Maybe it should just be
OverlayScrollbarSize? I still think the values of this enum have good names --
as a reminder, the values are IgnoreOverlayScrollbarSize and
IncludeOverlayScrollbarSize. I do think they add meaning over a bool, but I
agree that the type name of the enum is not the best. Anyone have a suggestion?


2. The concept of m_overlayScrollbarsRequireTemporaryClipRects that I added
with this patch is admittedly not the smartest mechanism, in that it never gets
set to false once it is set to true. So if a positioned overflow area with
overlay scrollbars is resized so that it doesn't have scrollbars anymore, then
m_overlayScrollbarsRequireTemporaryClipRects is not set to false as it should
be. That being said, you don't lose much by having it always be true in that
case -- you just do some extra work, but you don't sacrifice functionality at
all. Anyway, I think it's okay and it avoid the extra work in the vast majority
of cases. If you disagree, please chime in. Also the name is a bit
cumbersomely-long, so if you have a better suggestion, let me know


More information about the webkit-reviews mailing list