[webkit-reviews] review denied: [Bug 59753] Chromium Mac: Use ScrollAnimatorMac.mm for overlay scrollbar support : [Attachment 91589] Fixed Objective-C symbol clash error.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 28 16:59:37 PDT 2011


James Robinson <jamesr at chromium.org> has denied Sailesh Agrawal
<sail at chromium.org>'s request for review:
Bug 59753: Chromium Mac: Use ScrollAnimatorMac.mm for overlay scrollbar support
https://bugs.webkit.org/show_bug.cgi?id=59753

Attachment 91589: Fixed Objective-C symbol clash error.
https://bugs.webkit.org/attachment.cgi?id=91589&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=91589&action=review

The changes to ScrollbarOverlayUtilitiesMac.h/mm appear to be changes from
WebKit style braces to non WebKit style braces, except for one typo.  I'd
suggest not changing those files at all.

Why is the include guard on ScrollAnimatorMac changed?	Do we want to use
ScrollAnimator on chromium mac even if ENABLE(SMOOTH_SCROLLING) is off?  That
seems wrong, why not just turn SMOOTH_SCROLLING on if that's the behavior we
want?  Is there code in ScrollAnimatorMac.mm currently that is not related to
the SMOOTH_SCROLLING enable that should be refactored?

It seems you have some unintended changes in ScrollAnimatorMac.mm (like
reodering the elasticDelta/reboundDelta functions for no obvious reason).

> Source/WebCore/ChangeLog:10
> +	   No new tests. (OOPS!)

you can't do this, it will fail a presubmit check. replace this with some text
explaining why this patch does not have layout tests.


More information about the webkit-reviews mailing list