[webkit-reviews] review denied: [Bug 47036] Wheel scrolls and autoscrolls should not scroll overflow:hidden : [Attachment 72387] Followon patch v1

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Peter Kasting
<pkasting at google.com>'s request for review:
Bug 47036: Wheel scrolls and autoscrolls should not scroll overflow:hidden
https://bugs.webkit.org/show_bug.cgi?id=47036

Attachment 72387: Followon patch v1
https://bugs.webkit.org/attachment.cgi?id=72387&action=review

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=72387&action=review

> WebCore/dom/Node.cpp:2929
> +	       // scrolling the entire page.  We could fix this by trying to

Single space after periods in comments (in WebKit).

> WebCore/rendering/RenderLayer.cpp:1275
> +    if (renderer()->hasOverflowClip() && (!renderer()->parent() ||
renderer()->parent()->style()->lineClamp().isNone())) {

This long clause is repeated in two places. Should it be a method?

Also, this appears like it is unrelated to the fix. It simply changes the form
of the code to something that is less clear (to me).

> WebCore/rendering/RenderLayer.cpp:1306
> +bool RenderLayer::hasAScrollbar(bool vertical, bool horizontal)

This would be be better as two functions:


hasVerticalScrollbar
hasHorizontalScrollbar

> WebCore/rendering/RenderLayer.h:234
> +    // directions.  For renderers without an overflow clip (documents), this


Single space after .

> LayoutTests/fast/events/wheelevent-bubble.html:57
>  \ No newline at end of file

Please fix.