[webkit-reviews] review granted: [Bug 68959] Extend DOM WheelEvent to differentiate between physical and logical scroll directions : [Attachment 109083] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jon Lee <jonlee at apple.com>'s request
for review:
Bug 68959: Extend DOM WheelEvent to differentiate between physical and logical
scroll directions
https://bugs.webkit.org/show_bug.cgi?id=68959

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=109083&action=review


OK as-is, but seems to be too conditional in the wrong way, so maybe we could
tighten it up.

> Source/WebCore/dom/WheelEvent.h:95
> +#if PLATFORM(MAC)
> +	   WheelEvent(const FloatPoint& wheelTicks, const FloatPoint& rawDelta,

> +	       Granularity, PassRefPtr<AbstractView>,
> +	       const IntPoint& screenLocation, const IntPoint& pageLocation,
> +	       bool ctrlKey, bool altKey, bool shiftKey, bool metaKey, bool
directionInvertedFromDevice);
> +#endif

Perhaps we could set the directionInvertedFromDevice flag with an explicit set
call after construction. That would eliminate the need to repeat the entire
constructor and improve the ifdefs.

> Source/WebCore/dom/WheelEvent.idl:45
> +#if defined(WTF_PLATFORM_MAC) && WTF_PLATFORM_MAC
> +	   readonly attribute boolean webkitDirectionInvertedFromDevice;
> +#endif /* PLATFORM(WTF_PLATFORM_MAC) */

It’s ugly to use the WTF prefixed macros directly in IDL files.

Ideally this would be a HAVE-type define instead of a direct platform one, like
HAVE(INVERTED_WHEEL) or something like that. The platform macros themselves are
slated for demolition.

Why wouldn’t we want this attribute to exist and be false on all platforms?