[webkit-reviews] review denied: [Bug 24722] Sticky Autoscroll Fix : [Attachment 28792] Fix for Sticky Autoscroll Problem

[bugzilla-daemon at webkit.org]

Jon Honeycutt <jhoneycutt at apple.com> has denied Itai <idanan at chromium.org>'s
request for review:
Bug 24722: Sticky Autoscroll Fix
https://bugs.webkit.org/show_bug.cgi?id=24722

Attachment 28792: Fix for Sticky Autoscroll Problem
https://bugs.webkit.org/attachment.cgi?id=28792&action=review

------- Additional Comments from Jon Honeycutt <jhoneycutt at apple.com>
Hi, Itai, thanks for the patch! Some comments:

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog (revision 41862)
> +++ WebCore/ChangeLog (working copy)
> @@ -1,3 +1,14 @@
> +2009-03-20  Itai Danan  <set EMAIL_ADDRESS environment variable>

Your e-mail address is missing.

> +	   Reviewed by NOBODY (OOPS!).
> +
> +	Prevent the problem of autoscroll being sticky, particularly for case
when entering the mode is accidental.
> +	Codewise, a few lines were simply moved from on function to another a
few lines below.
> +
> +	   * page/EventHandler.cpp:
> +	   (WebCore::EventHandler::handleMousePressEvent):
> +	   (WebCore::EventHandler::handleMouseReleaseEvent):
> +

This comment isn't clear to me. It would be more clear to explain the change
you're making exactly, e.g., "Stop auto-scrolling when the middle mouse button
has been released."

I am r-ing, because this change doesn't match the auto-scroll behavior of other
browsers, and I think matching other browsers is the best way to fix this.

With Firefox and IE, you have to mouse the mouse farther from the click point
before the page begins scrolling, both browsers scroll more slowly than WebKit
when the mouse is only a short distance from the click point, and neither
browser will start an auto-scroll if the area clicked isn't scrollable (e.g., a
page with no scrollbars). I think fixing these would resolve the issues that
most people are having.

Bug #21794 is similar and is about supporting another mode of auto-scrolling,
where the auto-scroll stops when the middle mouse button has been released.