With Lion, users may choose to reverse the scrolling direction. Exposing a boolean indicating whether the user is scrolling naturally would help web authors determine a scroll event's direction. <rdar://problem/10036688>
Created attachment 109072 [details] Patch
Comment on attachment 109072 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109072&action=review > Source/WebKit2/Shared/mac/WebEventFactory.mm:1103 > +#if BUILDING_ON_LION > + bool directionInvertedFromDevice = [event isDirectionInvertedFromDevice]; > +#else > + bool directionInvertedFromDevice = false; > +#endif This is not correct.
Created attachment 109078 [details] Patch
Fixed the #if OS check.
The #if should be #if !defined(...)-- sending another patch.
Created attachment 109079 [details] Patch
Comment on attachment 109079 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109079&action=review I think we probably want to prefix the IDL attribute with webkit as well. > Source/WebCore/platform/mac/WheelEventMac.mm:104 > + , m_directionInvertedFromDevice([event isDirectionInvertedFromDevice]) I think this probably needs to be conditional on Lion or later.
(In reply to comment #7) > (From update of attachment 109079 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=109079&action=review > > I think we probably want to prefix the IDL attribute with webkit as well. OK. > > > Source/WebCore/platform/mac/WheelEventMac.mm:104 > > + , m_directionInvertedFromDevice([event isDirectionInvertedFromDevice]) > > I think this probably needs to be conditional on Lion or later. Whoops, you are right. Will add.
Created attachment 109083 [details] Patch
Updated with Sam's corrections.
Comment on attachment 109083 [details] Patch 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?
> > Why wouldn’t we want this attribute to exist and be false on all platforms? I think the answer to that is we probably do want it on all platforms. -Sam
I'll make the adjustments and send out another patch.
Created attachment 109510 [details] Patch
Comment on attachment 109510 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109510&action=review > Source/WebKit2/ChangeLog:41 > +2011-09-28 Jon Lee <jonlee@apple.com> > + > + Extend DOM WheelEvent to differentiate between physical and logical scroll directions > + https://bugs.webkit.org/show_bug.cgi?id=68959 > + <rdar://problem/10036688> > + > + Reviewed by NOBODY (OOPS!). > + > + * Shared/WebEvent.h: > + (WebKit::WebWheelEvent::directionInvertedFromDevice): > + * Shared/WebEventConversion.cpp: > + (WebKit::WebKit2PlatformWheelEvent::WebKit2PlatformWheelEvent): > + * Shared/WebWheelEvent.cpp: > + (WebKit::WebWheelEvent::WebWheelEvent): > + (WebKit::WebWheelEvent::encode): > + (WebKit::WebWheelEvent::decode): > + * Shared/mac/WebEventFactory.mm: > + (WebKit::WebEventFactory::createWebWheelEvent): get the flag from the NSEvent. > + * UIProcess/WebPageProxy.cpp: > + (WebKit::coalesce): Double changelog!
(In reply to comment #15) > Double changelog! Fixed, and thanks!
Committed r96613: <http://trac.webkit.org/changeset/96613>
This patch broke Leopard build. Fix attempted in http://trac.webkit.org/changeset/96629.
This bug also appears to have broken the gtk build. http://build.webkit.org/builders/GTK%20Linux%2032-bit%20Release/builds/17973/steps/compile-webkit/logs/stdio
n/m, false alarm on the gtk folks.
(In reply to comment #19) > This bug also appears to have broken the gtk build. > http://build.webkit.org/builders/GTK%20Linux%2032-bit%20Release/builds/17973/steps/compile-webkit/logs/stdio This is due to https://bugs.webkit.org/show_bug.cgi?id=69357.