[webkit-reviews] review granted: [Bug 60779] Bug in rubber banding logic for scroll animators : [Attachment 93599] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun May 15 22:16:00 PDT 2011


Simon Fraser (smfr) <simon.fraser at apple.com> has granted Jon Lee
<jonlee at apple.com>'s request for review:
Bug 60779: Bug in rubber banding logic for scroll animators
https://bugs.webkit.org/show_bug.cgi?id=60779

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=93599&action=review

> Source/WebCore/platform/ScrollableArea.h:137
> +    bool isHorizontalScrollerPinnedToLeft() const { return
!horizontalScrollbar() || scrollPosition(horizontalScrollbar()) <=
minimumScrollPosition().x(); }
> +    bool isHorizontalScrollerPinnedToRight() const { return
!horizontalScrollbar() || scrollPosition(horizontalScrollbar()) >=
maximumScrollPosition().x(); }
> +    

'left' and 'right' are loaded terms in a mixed writing-direction world. I'd
rename these to isHorizontalScrollerAtMinimum() and
isHorizontalScrollerAtMaximum or something.

> Source/WebCore/platform/mac/ScrollAnimatorMac.h:149
> +    int m_aggregateScroll;

Not sure what 'aggregate' means here. Would 'cumulative' be a better term to
use?

Also, if this is only ever in X, the name should reflect that.


More information about the webkit-reviews mailing list