[Webkit-unassigned] [Bug 135195] Refactor EventHandler to call ScrollAnimator::handleWheelEvent for overflow scrolling
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jul 29 16:04:26 PDT 2014
https://bugs.webkit.org/show_bug.cgi?id=135195
--- Comment #28 from Beth Dakin <bdakin at apple.com> 2014-07-29 16:04:35 PST ---
(From update of attachment 235694)
View in context: https://bugs.webkit.org/attachment.cgi?id=235694&action=review
> Source/WebCore/page/EventHandler.cpp:290
> + return scrollableArea->scroll(delta < 0 ? (isVerticalAxis ? ScrollUp : ScrollLeft) : (isVerticalAxis ? ScrollDown : ScrollRight), wheelGranularityToScrollGranularity(wheelEvent->deltaMode()), delta > 0 ? delta : -delta);
This is a little hard to read. Maybe you can pull some of those ternary operator evaluations out into well-named local variable?
>> Source/WebCore/page/EventHandler.cpp:293
>> +static inline bool handleWheelEventInAppropriateEnclosingBoxForSingleAxis(Node* startNode, WheelEvent* wheelEvent, Element** stopElement, bool isVerticalAxis)
>
> I considered using an enum for this instead of a bool flag, but the only existing enum that made sense was ScrollbarOrientation, which sounded like it should be used for scrollbar-specific purposes. Meanwhile, ScrollDirection sounds more accurate, but it's specific to all 4 scrolling directions, whereas I only need the orientation (left/right vs. up/down).
I think you should consider adding a new enum. Something called ScrollGestureAxis or ScrollEventAxis or something along those lines?
> Source/WebCore/page/EventHandler.cpp:298
> + RenderBox* initialEnclosingBox = &startNode->renderer()->enclosingBox();
Is there any reason you want to use this as a pointer instead of a reference?
> Source/WebCore/page/EventHandler.cpp:306
> + if (boxLayer && (platformEvent != nullptr ? boxLayer->handleWheelEvent(isVerticalAxis ? platformEvent->copyIgnoringHorizontalDelta() : platformEvent->copyIgnoringVerticalDelta()) : didScrollInScrollableAreaForSingleAxis(boxLayer, wheelEvent, isVerticalAxis))) {
This is very difficult to read. Again, I suggest using some well-named local variables for the ternary operator evaluations.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list