[webkit-reviews] review denied: [Bug 46739] Vertical scroll bar on apple.com is too short with WebKit2 : [Attachment 69074] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 28 11:26:32 PDT 2010


Darin Adler <darin at apple.com> has denied Sam Weinig <sam at webkit.org>'s request
for review:
Bug 46739: Vertical scroll bar on apple.com is too short with WebKit2
https://bugs.webkit.org/show_bug.cgi?id=46739

Attachment 69074: Patch
https://bugs.webkit.org/attachment.cgi?id=69074&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=69074&action=review

review- because there is no reason to handle only the top two levels of
FrameView.

> WebKit2/UIProcess/API/mac/WKView.mm:323
> +    // Temporarily enable the resize indicator to make a the
_ownsWindowGrowBox
> +    // calculation work.

Comment should go on one line.

> WebKit2/WebProcess/WebPage/WebPage.cpp:578
> +    const HashSet<RefPtr<Widget> >* viewChildren = view->children();

Why do we need to do this one level deep, and not multiple levels? Might be
better to use the frame tree and walk all the frames, not just the top level
subframes.

> WebKit2/WebProcess/WebPage/WebPage.cpp:583
> +	       static_cast<FrameView*>(widget)->invalidateScrollbars();

The “invalidate” terminology here seems unclear to me.


More information about the webkit-reviews mailing list