[webkit-reviews] review denied: [Bug 32080] Make it possible to keep track of scrolls : [Attachment 44169] Make it possible to keep track of scrolls

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Dec 20 08:14:05 PST 2009


Adam Treat <treat at kde.org> has denied Kenneth Rohde Christiansen
<kenneth at webkit.org>'s request for review:
Bug 32080: Make it possible to keep track of scrolls
https://bugs.webkit.org/show_bug.cgi?id=32080

Attachment 44169: Make it possible to keep track of scrolls
https://bugs.webkit.org/attachment.cgi?id=44169&action=review

------- Additional Comments from Adam Treat <treat at kde.org>
> This patch makes sure that there is no behaviour change for Haiku.
...
> +bool ChromeClientHaiku::scroll(const IntSize& scrollDelta, const IntRect&
rectToScroll, const IntRect& clipRect, bool canBlit)
>  {
>      notImplemented();
> +    return true;

There is a behavior change here.  Haiku previously would have repainted the
entire viewport for a slow scroll.  Now it won't since it returns true
indicating it 'canBlit'.

> This patch makes sure that there is no behaviour change for Mac.
...
> -void WebChromeClient::scroll(const IntSize&, const IntRect&, const IntRect&)

> +bool WebChromeClient::scroll(const IntSize&, const IntRect&, const IntRect&,
bool)
>  {
> +    return true;
>  }

There is a behavior change here.  Mac previously would have repainted the
entire viewport for a slow scroll.  Now it won't since it returns true
indicating it 'canBlit'.

You don't need to add a bool return value to scroll IMHO.  I'd prefer you add a
new 'slowScroll' notification to the various ChromeClient's instead of doing it
this way.

r- for the behavior changes.


More information about the webkit-reviews mailing list