[webkit-reviews] review denied: [Bug 74926] [Qt] Optionally support smooth-scrolling on all platforms : [Attachment 143037] Patch

[bugzilla-daemon at webkit.org]

Tor Arne Vestbø <vestbo at webkit.org> has denied Allan Sandfeld Jensen
<allan.jensen at nokia.com>'s request for review:
Bug 74926: [Qt] Optionally support smooth-scrolling on all platforms
https://bugs.webkit.org/show_bug.cgi?id=74926

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

------- Additional Comments from Tor Arne Vestbø <vestbo at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=143037&action=review


> Source/WebCore/Target.pri:2935
> +    HEADERS += platform/ScrollAnimatorNone.h
> +    SOURCES += platform/ScrollAnimatorNone.cpp

I'd like to see this renamed to something that reflects that it's a
runtime-configurable behavior, not a no-op file.

> Source/WebCore/platform/ScrollAnimatorNone.h:178
> +#if !PLATFORM(QT)
>      OwnPtr<ActivePlatformGestureAnimation> m_gestureAnimation;
> +#endif
>  };

Is there a better guard for this?

> Tools/Scripts/webkitperl/FeatureList.pm:306
> +    { option => "smooth-scrolling", desc => "Toggle Animated Smooth
Scrolling",
> +	 define => "ENABLE_SMOOTH_SCROLLING", default => (isAppleMacWebKit() ||
isGtk() || isChromium() || isBlackBerry()), value => \$smoothScrollingSupport
},
> +

You need to touch more build system files now that the feature is a
build-webkit-visible enable.

Some you might be able to auto-generate using the python-script that eseidel
has been hacking on (do a git log on this file for details).

> Tools/qmake/mkspecs/features/features.prf:208
> +# Animated smooth-scrolling default

features.pri (auto-generated from FeatureList.pm) should have
ENABLE_SMOOTH_SCROLLING=0